Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
[SafeBrowsing] Send pings for Zip files that contain other archives.
Browse files Browse the repository at this point in the history 
BUG=515216

Review URL: https://codereview.chromium.org/1262753002

Cr-Commit-Position: refs/heads/master@{#341475}
  • Loading branch information
@asankah
asankah authored and Commit bot committed Aug 2, 2015
1 parent 6c3cda9 commit 04a4eb9
Show file tree
Hide file tree
Showing 15 changed files with 280 additions and 56 deletions.
55 changes: 44 additions & 11 deletions chrome/browser/safe_browsing/download_protection_service.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,26 +120,53 @@ const base::FilePath::CharType* const kDangerousFileTypes[] = {
FILE_PATH_LITERAL(".scf"),
FILE_PATH_LITERAL(".sct"),
FILE_PATH_LITERAL(".wsf"),
FILE_PATH_LITERAL(".7z"),
FILE_PATH_LITERAL(".xz"),
FILE_PATH_LITERAL(".gz"),
FILE_PATH_LITERAL(".tgz"),
FILE_PATH_LITERAL(".bz2"),
FILE_PATH_LITERAL(".tar"),
FILE_PATH_LITERAL(".arj"),
FILE_PATH_LITERAL(".lzh"),
FILE_PATH_LITERAL(".lha"),
FILE_PATH_LITERAL(".wim"),
FILE_PATH_LITERAL(".z"),
FILE_PATH_LITERAL(".lzma"),
FILE_PATH_LITERAL(".cpio"),
};

// UMA enumeration value for unrecognized file types. This is the array index of
// the "Other" bucket in kDangerousFileTypes.
const int EXTENSION_OTHER = 18;

void RecordFileExtensionType(const base::FilePath& file) {
// Maximum extension ID returned by GetExtensionTypeForUMA() + 1.
const int EXTENSION_MAX = arraysize(kDangerousFileTypes);

int GetExtensionTypeForUMA(const base::FilePath::StringType& extension) {
DCHECK_EQ(static_cast<base::FilePath::CharType*>(nullptr),
kDangerousFileTypes[EXTENSION_OTHER]);

int extension_type = EXTENSION_OTHER;
for (const auto& extension : kDangerousFileTypes) {
if (extension && file.MatchesExtension(extension)) {
extension_type = &extension - kDangerousFileTypes;
break;
}
DCHECK(extension.find(base::FilePath::kExtensionSeparator) == 0 ||
extension.empty());
DCHECK_EQ(extension, base::FilePath(extension).FinalExtension());

for (const auto& dangerous_extension : kDangerousFileTypes) {
if (dangerous_extension &&
base::FilePath::CompareEqualIgnoreCase(dangerous_extension, extension))
return &dangerous_extension - kDangerousFileTypes;
}
return EXTENSION_OTHER;
}

void RecordFileExtensionType(const base::FilePath& file) {
UMA_HISTOGRAM_ENUMERATION("SBClientDownload.DownloadExtensions",
extension_type, arraysize(kDangerousFileTypes));
GetExtensionTypeForUMA(file.FinalExtension()),
EXTENSION_MAX);
}

void RecordArchivedArchiveFileExtensionType(
const base::FilePath::StringType& extension) {
UMA_HISTOGRAM_ENUMERATION("SBClientDownload.ArchivedArchiveExtensions",
GetExtensionTypeForUMA(extension), EXTENSION_MAX);
}

// Enumerate for histogramming purposes.
Expand Down Expand Up @@ -502,7 +529,7 @@ class DownloadProtectionService::CheckClientDownloadRequest
*reason = REASON_INVALID_URL;
return false;
}
if (!download_protection_util::IsBinaryFile(target_path)) {
if (!download_protection_util::IsSupportedBinaryFile(target_path)) {
*reason = REASON_NOT_BINARY_FILE;
return false;
}
Expand Down Expand Up @@ -601,6 +628,7 @@ class DownloadProtectionService::CheckClientDownloadRequest

void OnZipAnalysisFinished(const zip_analyzer::Results& results) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
DCHECK_EQ(ClientDownloadRequest::ZIPPED_EXECUTABLE, type_);
if (!service_)
return;
if (results.success) {
Expand All @@ -618,11 +646,16 @@ class DownloadProtectionService::CheckClientDownloadRequest
results.has_archive && !zipped_executable_);
UMA_HISTOGRAM_TIMES("SBClientDownload.ExtractZipFeaturesTime",
base::TimeTicks::Now() - zip_analysis_start_time_);
for (const auto& file_extension : results.archived_archive_filetypes)
RecordArchivedArchiveFileExtensionType(file_extension);

if (!zipped_executable_) {
if (!zipped_executable_ && !results.has_archive) {
PostFinishTask(UNKNOWN, REASON_ARCHIVE_WITHOUT_BINARIES);
return;
}

if (!zipped_executable_ && results.has_archive)
type_ = ClientDownloadRequest::ZIPPED_ARCHIVE;
OnFileFeatureExtractionDone();
}

Expand Down
56 changes: 56 additions & 0 deletions chrome/browser/safe_browsing/download_protection_service_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1074,6 +1074,9 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadZip) {
EXPECT_TRUE(IsResult(DownloadProtectionService::SAFE));
EXPECT_TRUE(HasClientDownloadRequest());
const ClientDownloadRequest& request = *GetClientDownloadRequest();
EXPECT_TRUE(request.has_download_type());
EXPECT_EQ(ClientDownloadRequest_DownloadType_ZIPPED_EXECUTABLE,
request.download_type());
EXPECT_EQ(1, request.archived_binary_size());
const ClientDownloadRequest_ArchivedBinary* archived_binary =
GetRequestArchivedBinary(request, "file.exe");
Expand Down Expand Up @@ -1112,6 +1115,59 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadZip) {
#else
EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
EXPECT_FALSE(HasClientDownloadRequest());
#endif
Mock::VerifyAndClearExpectations(binary_feature_extractor_.get());

// Repeat the test with an archive inside the zip file in addition to the
// executable.
ASSERT_EQ(static_cast<int>(file_contents.size()),
base::WriteFile(
zip_source_dir.path().Append(FILE_PATH_LITERAL("file.rar")),
file_contents.data(), file_contents.size()));
ASSERT_TRUE(zip::Zip(zip_source_dir.path(), a_tmp, false));

download_service_->CheckClientDownload(
&item, base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
base::Unretained(this)));
MessageLoop::current()->Run();

#if defined(OS_WIN) || defined(OS_MACOSX)
ASSERT_TRUE(HasClientDownloadRequest());
EXPECT_EQ(1, GetClientDownloadRequest()->archived_binary_size());
EXPECT_TRUE(GetClientDownloadRequest()->has_download_type());
EXPECT_EQ(ClientDownloadRequest_DownloadType_ZIPPED_EXECUTABLE,
GetClientDownloadRequest()->download_type());
ClearClientDownloadRequest();
#else
// For !(OS_WIN || OS_MACOSX), no file types are currently supported. Hence
// the resulting verdict is UNKNOWN.
EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
EXPECT_FALSE(HasClientDownloadRequest());
#endif
Mock::VerifyAndClearExpectations(binary_feature_extractor_.get());

// Repeat the test with just the archive inside the zip file.
ASSERT_TRUE(
base::DeleteFile(zip_source_dir.path().AppendASCII("file.exe"), false));
ASSERT_TRUE(zip::Zip(zip_source_dir.path(), a_tmp, false));

download_service_->CheckClientDownload(
&item, base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
base::Unretained(this)));
MessageLoop::current()->Run();

#if defined(OS_WIN) || defined(OS_MACOSX)
ASSERT_TRUE(HasClientDownloadRequest());
EXPECT_EQ(0, GetClientDownloadRequest()->archived_binary_size());
EXPECT_TRUE(GetClientDownloadRequest()->has_download_type());
EXPECT_EQ(ClientDownloadRequest_DownloadType_ZIPPED_ARCHIVE,
GetClientDownloadRequest()->download_type());
ClearClientDownloadRequest();
#else
// For !(OS_WIN || OS_MACOSX), no file types are currently supported. Hence
// the resulting verdict is UNKNOWN.
EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
EXPECT_FALSE(HasClientDownloadRequest());
#endif
Mock::VerifyAndClearExpectations(binary_feature_extractor_.get());
}
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/safe_browsing/incident_reporting/last_download_finder.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ int64 GetEndTime(const ClientIncidentReport_DownloadDetails& details) {
bool IsBinaryDownload(const history::DownloadRow& row) {
// TODO(grt): Peek into archives to see if they contain binaries;
// http://crbug.com/386915.
return (download_protection_util::IsBinaryFile(row.target_path) &&
return (download_protection_util::IsSupportedBinaryFile(row.target_path) &&
!download_protection_util::IsArchiveFile(row.target_path));
}

Expand Down
113 changes: 92 additions & 21 deletions chrome/browser/safe_browsing/sandboxed_zip_analyzer_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,23 @@ class SandboxedZipAnalyzerTest : public ::testing::Test {
run_loop.Run();
}

#if defined(OS_WIN)
void ExpectPEHeaders(const BinaryData& data,
const ClientDownloadRequest_ArchivedBinary& binary) {
ASSERT_EQ(data.is_signed, binary.has_signature());
if (data.is_signed) {
ASSERT_LT(0, binary.signature().signed_data_size());
ASSERT_NE(0U, binary.signature().signed_data(0).size());
}
ASSERT_TRUE(binary.has_image_headers());
ASSERT_TRUE(binary.image_headers().has_pe_headers());
EXPECT_TRUE(binary.image_headers().pe_headers().has_dos_header());
EXPECT_TRUE(binary.image_headers().pe_headers().has_file_header());
EXPECT_TRUE(binary.image_headers().pe_headers().has_optional_headers32());
EXPECT_FALSE(binary.image_headers().pe_headers().has_optional_headers64());
}
#endif

// Verifies expectations about a binary found by the analyzer.
void ExpectBinary(const BinaryData& data,
const ClientDownloadRequest_ArchivedBinary& binary) {
Expand All @@ -97,28 +114,24 @@ class SandboxedZipAnalyzerTest : public ::testing::Test {
EXPECT_FALSE(binary.digests().has_md5());
ASSERT_TRUE(binary.has_length());
EXPECT_EQ(data.length, binary.length());
#if defined(OS_WIN) // ExtractImageFeatures is only implemented for Win.
ASSERT_EQ(data.is_signed, binary.has_signature());
if (data.is_signed) {
ASSERT_LT(0, binary.signature().signed_data_size());
ASSERT_NE(0U, binary.signature().signed_data(0).size());
#if defined(OS_WIN)
// ExtractImageFeatures only implemented for Windows, and only works on PE
// files.
if (binary.file_basename().find(".exe") != std::string::npos) {
ExpectPEHeaders(data, binary);
return;
}
ASSERT_TRUE(binary.has_image_headers());
ASSERT_TRUE(binary.image_headers().has_pe_headers());
EXPECT_TRUE(binary.image_headers().pe_headers().has_dos_header());
EXPECT_TRUE(binary.image_headers().pe_headers().has_file_header());
EXPECT_TRUE(binary.image_headers().pe_headers().has_optional_headers32());
EXPECT_FALSE(binary.image_headers().pe_headers().has_optional_headers64());
#else // OS_WIN
#endif // OS_WIN
ASSERT_FALSE(binary.has_signature());
ASSERT_FALSE(binary.has_image_headers());
#endif // !OS_WIN
}

static const uint8_t kUnsignedDigest[];
static const uint8_t kSignedDigest[];
static const uint8_t kJSEFileDigest[];
static const BinaryData kUnsignedExe;
static const BinaryData kSignedExe;
static const BinaryData kJSEFile;

base::FilePath dir_test_data_;
content::TestBrowserThreadBundle browser_thread_bundle_;
Expand All @@ -127,15 +140,17 @@ class SandboxedZipAnalyzerTest : public ::testing::Test {

// static
const uint8_t SandboxedZipAnalyzerTest::kUnsignedDigest[] = {
0x1e, 0x95, 0x4d, 0x9c, 0xe0, 0x38, 0x9e, 0x2b, 0xa7, 0x44, 0x72, 0x16,
0xf2, 0x17, 0x61, 0xf9, 0x8d, 0x1e, 0x65, 0x40, 0xc2, 0xab, 0xec, 0xdb,
0xec, 0xff, 0x57, 0x0e, 0x36, 0xc4, 0x93, 0xdb
};
0x1e, 0x95, 0x4d, 0x9c, 0xe0, 0x38, 0x9e, 0x2b, 0xa7, 0x44, 0x72,
0x16, 0xf2, 0x17, 0x61, 0xf9, 0x8d, 0x1e, 0x65, 0x40, 0xc2, 0xab,
0xec, 0xdb, 0xec, 0xff, 0x57, 0x0e, 0x36, 0xc4, 0x93, 0xdb};
const uint8_t SandboxedZipAnalyzerTest::kSignedDigest[] = {
0xe1, 0x1f, 0xfa, 0x0c, 0x9f, 0x25, 0x23, 0x44, 0x53, 0xa9, 0xed, 0xd1,
0xcb, 0x25, 0x1d, 0x46, 0x10, 0x7f, 0x34, 0xb5, 0x36, 0xad, 0x74, 0x64,
0x2a, 0x85, 0x84, 0xac, 0xa8, 0xc1, 0xa8, 0xce
};
0xe1, 0x1f, 0xfa, 0x0c, 0x9f, 0x25, 0x23, 0x44, 0x53, 0xa9, 0xed,
0xd1, 0xcb, 0x25, 0x1d, 0x46, 0x10, 0x7f, 0x34, 0xb5, 0x36, 0xad,
0x74, 0x64, 0x2a, 0x85, 0x84, 0xac, 0xa8, 0xc1, 0xa8, 0xce};
const uint8_t SandboxedZipAnalyzerTest::kJSEFileDigest[] = {
0x58, 0x91, 0xb5, 0xb5, 0x22, 0xd5, 0xdf, 0x08, 0x6d, 0x0f, 0xf0,
0xb1, 0x10, 0xfb, 0xd9, 0xd2, 0x1b, 0xb4, 0xfc, 0x71, 0x63, 0xaf,
0x34, 0xd0, 0x82, 0x86, 0xa2, 0xe8, 0x46, 0xf6, 0xbe, 0x03};
const SandboxedZipAnalyzerTest::BinaryData
SandboxedZipAnalyzerTest::kUnsignedExe = {
"unsigned.exe",
Expand All @@ -152,6 +167,14 @@ const SandboxedZipAnalyzerTest::BinaryData
37768,
true, // is_signed
};
const SandboxedZipAnalyzerTest::BinaryData SandboxedZipAnalyzerTest::kJSEFile =
{
"hello.jse",
ClientDownloadRequest_DownloadType_WIN_EXECUTABLE,
&kJSEFileDigest[0],
6,
false, // is_signed
};

TEST_F(SandboxedZipAnalyzerTest, NoBinaries) {
zip_analyzer::Results results;
Expand Down Expand Up @@ -185,4 +208,52 @@ TEST_F(SandboxedZipAnalyzerTest, TwoBinariesOneSigned) {
ExpectBinary(kSignedExe, results.archived_binary.Get(1));
}

TEST_F(SandboxedZipAnalyzerTest, ZippedArchiveNoBinaries) {
zip_analyzer::Results results;
RunAnalyzer(dir_test_data_.AppendASCII("zipfile_archive_no_binaries.zip"),
&results);
ASSERT_TRUE(results.success);
EXPECT_FALSE(results.has_executable);
EXPECT_TRUE(results.has_archive);
EXPECT_EQ(0, results.archived_binary.size());
ASSERT_EQ(1u, results.archived_archive_filetypes.size());
EXPECT_EQ(FILE_PATH_LITERAL(".zip"), results.archived_archive_filetypes[0]);
}

TEST_F(SandboxedZipAnalyzerTest, ZippedRarArchiveNoBinaries) {
zip_analyzer::Results results;
RunAnalyzer(dir_test_data_.AppendASCII("zipfile_rar_archive_no_binaries.zip"),
&results);
ASSERT_TRUE(results.success);
EXPECT_FALSE(results.has_executable);
EXPECT_TRUE(results.has_archive);
EXPECT_EQ(0, results.archived_binary.size());
ASSERT_EQ(1u, results.archived_archive_filetypes.size());
EXPECT_EQ(FILE_PATH_LITERAL(".rar"), results.archived_archive_filetypes[0]);
}

TEST_F(SandboxedZipAnalyzerTest, ZippedArchiveAndBinaries) {
zip_analyzer::Results results;
RunAnalyzer(dir_test_data_.AppendASCII("zipfile_archive_and_binaries.zip"),
&results);
ASSERT_TRUE(results.success);
EXPECT_TRUE(results.has_executable);
EXPECT_TRUE(results.has_archive);
ASSERT_EQ(1, results.archived_binary.size());
ExpectBinary(kSignedExe, results.archived_binary.Get(0));
ASSERT_EQ(1u, results.archived_archive_filetypes.size());
EXPECT_EQ(FILE_PATH_LITERAL(".7z"), results.archived_archive_filetypes[0]);
}

TEST_F(SandboxedZipAnalyzerTest, ZippedJSEFile) {
zip_analyzer::Results results;
RunAnalyzer(dir_test_data_.AppendASCII("zipfile_one_jse_file.zip"), &results);
ASSERT_TRUE(results.success);
EXPECT_TRUE(results.has_executable);
EXPECT_FALSE(results.has_archive);
ASSERT_EQ(1, results.archived_binary.size());
ExpectBinary(kJSEFile, results.archived_binary.Get(0));
EXPECT_TRUE(results.archived_archive_filetypes.empty());
}

} // namespace safe_browsing
1 change: 1 addition & 0 deletions chrome/common/chrome_utility_messages.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ IPC_STRUCT_TRAITS_BEGIN(safe_browsing::zip_analyzer::Results)
IPC_STRUCT_TRAITS_MEMBER(has_executable)
IPC_STRUCT_TRAITS_MEMBER(has_archive)
IPC_STRUCT_TRAITS_MEMBER(archived_binary)
IPC_STRUCT_TRAITS_MEMBER(archived_archive_filetypes)
IPC_STRUCT_TRAITS_END()
#endif // FULL_SAFE_BROWSING

Expand Down
3 changes: 2 additions & 1 deletion chrome/common/safe_browsing/csd.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,8 @@ message ClientDownloadRequest {
ANDROID_APK = 2; // .apk files.
// .zip files containing one of the other executable types.
ZIPPED_EXECUTABLE = 3;
MAC_EXECUTABLE = 4; // .dmg, .pkg, etc.
MAC_EXECUTABLE = 4; // .dmg, .pkg, etc.
ZIPPED_ARCHIVE = 5; // .zip file containing another archive.
}
optional DownloadType download_type = 10 [default = WIN_EXECUTABLE];

Expand Down
Loading

0 comments on commit 04a4eb9

Please sign in to comment.