[s] Do not mark contained archives as executables.

This mimics the behavior of zip_analyzer.

Also, mark contained archives as ARCHIVE instead of
ZIPPED_ARCHIVE or RAR_COMPRESSED_ARCHIVE.

Bug: 853922, 853971
Change-Id: I159126518c1de29029c0a3a0fd435fb997cb2952
Reviewed-on: https://chromium-review.googlesource.com/1105195Reviewed-by: Jialiu Lin <jialiul@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Varun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568299}

chrome/common/safe_browsing/rar_analyzer.cc

@@ -57,11 +57,7 @@ void AnalyzeRarFile(base::File rar_file,
 
     bool is_executable =
         FileTypePolicies::GetInstance()->IsCheckedBinaryFile(file_path);
-    results->has_executable |= is_executable;
-
     bool is_archive = FileTypePolicies::GetInstance()->IsArchiveFile(file_path);
-    results->has_archive |= is_archive;
-
     int64 unpacked_size =
         archive->FileHead.UnpSize;  // Read from header, may not be accurate.
     // TODO(vakh): Log UMA if |unpacked_size| < 0.
@@ -72,15 +68,16 @@ void AnalyzeRarFile(base::File rar_file,
         base::StreamingUtf8Validator::Validate(basename_utf8);
 
     if (is_archive) {
+      results->has_archive = true;
       archived_archive_filenames.insert(basename);
       ClientDownloadRequest::ArchivedBinary* archived_archive =
           results->archived_binary.Add();
       if (is_utf8_valid_basename)
         archived_archive->set_file_basename(basename_utf8);
-      archived_archive->set_download_type(
-          ClientDownloadRequest::RAR_COMPRESSED_ARCHIVE);
+      archived_archive->set_download_type(ClientDownloadRequest::ARCHIVE);
       archived_archive->set_length(unpacked_size);
     } else if (is_executable) {
+      results->has_executable = true;
       ClientDownloadRequest::ArchivedBinary* archived_binary =
           results->archived_binary.Add();
       if (is_utf8_valid_basename)

chrome/common/safe_browsing/zip_analyzer.cc

@@ -162,8 +162,7 @@ void AnalyzeZipFile(base::File zip_file,
       std::string file_basename_utf8(file.BaseName().AsUTF8Unsafe());
       if (base::StreamingUtf8Validator::Validate(file_basename_utf8))
         archived_archive->set_file_basename(file_basename_utf8);
-      archived_archive->set_download_type(
-          ClientDownloadRequest::ZIPPED_ARCHIVE);
+      archived_archive->set_download_type(ClientDownloadRequest::ARCHIVE);
     } else if (current_entry_is_executable) {
 #if defined(OS_MACOSX)
       // This check prevents running analysis on .app files since they are

chrome/services/file_util/public/cpp/sandboxed_rar_analyzer_unittest.cc

@@ -111,12 +111,12 @@ class SandboxedRarAnalyzerTest : public testing::Test {
 
 const SandboxedRarAnalyzerTest::BinaryData SandboxedRarAnalyzerTest::kEmptyZip =
     {
-        "empty.zip", CDRDT(RAR_COMPRESSED_ARCHIVE), 22,
+        "empty.zip", CDRDT(ARCHIVE), 22,
 };
 
 const SandboxedRarAnalyzerTest::BinaryData SandboxedRarAnalyzerTest::kNotARar =
     {
-        "not_a_rar.rar", CDRDT(RAR_COMPRESSED_ARCHIVE), 18,
+        "not_a_rar.rar", CDRDT(ARCHIVE), 18,
 };
 
 const SandboxedRarAnalyzerTest::BinaryData
@@ -183,7 +183,7 @@ TEST_F(SandboxedRarAnalyzerTest, AnalyzeTextAsRar) {
 }
 
 TEST_F(SandboxedRarAnalyzerTest, AnalyzeRarContainingArchive) {
-  // Can detect when .rar contains executable files.
+  // Can detect when .rar contains other archive files.
   // has_archive.rar contains 1 file: empty.zip
   base::FilePath path;
   ASSERT_NO_FATAL_FAILURE(path = GetFilePath("has_archive.rar"));
@@ -192,7 +192,7 @@ TEST_F(SandboxedRarAnalyzerTest, AnalyzeRarContainingArchive) {
   AnalyzeFile(path, &results);
 
   ASSERT_TRUE(results.success);
-  EXPECT_TRUE(results.has_executable);  // .zip is considered binary executable.
+  EXPECT_FALSE(results.has_executable);
   EXPECT_EQ(1, results.archived_binary.size());
   EXPECT_EQ(1u, results.archived_archive_filenames.size());
   ExpectBinary(kEmptyZip, results.archived_binary.Get(0));