unrar: Add information about exe/archive files contained within downloaded .rar

Bug: 750327
Change-Id: I24dd7764a41826836b81f70c994d0a0904d82d6c
Reviewed-on: https://chromium-review.googlesource.com/1091830
Commit-Queue: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Jialiu Lin <jialiul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565770}

chrome/common/safe_browsing/BUILD.gn

@@ -41,6 +41,21 @@ if (safe_browsing_mode == 1) {
     ]
   }
 
+  source_set("download_protection_util") {
+    sources = [
+      "download_protection_util.cc",
+      "download_protection_util.h",
+    ]
+    deps = [
+      ":file_type_policies",
+      "//base",
+      "//components/safe_browsing:features",
+    ]
+    public_deps = [
+      "//components/safe_browsing:csd_proto",
+    ]
+  }
+
   source_set("rar_analyzer") {
     sources = [
       "rar_analyzer.cc",
@@ -49,8 +64,10 @@ if (safe_browsing_mode == 1) {
 
     deps = [
       ":archive_analyzer_results",
+      ":download_protection_util",
       ":file_type_policies",
       "//base",
+      "//base:i18n",
       "//third_party/unrar:unrar",
     ]
 
@@ -85,8 +102,6 @@ source_set("safe_browsing") {
       "binary_feature_extractor.h",
       "binary_feature_extractor_mac.cc",
       "binary_feature_extractor_win.cc",
-      "download_protection_util.cc",
-      "download_protection_util.h",
       "ipc_protobuf_message_macros.h",
       "ipc_protobuf_message_null_macros.h",
       "mach_o_image_reader_mac.cc",
@@ -105,6 +120,7 @@ source_set("safe_browsing") {
 
     deps += [
       ":archive_analyzer_results",
+      ":download_protection_util",
       ":rar_analyzer",
       "//components/safe_browsing:features",
     ]

chrome/common/safe_browsing/rar_analyzer.cc

@@ -7,9 +7,11 @@
 #include <memory>
 #include <string>
 #include "base/files/file_path.h"
+#include "base/i18n/streaming_utf8_validator.h"
 #include "base/metrics/histogram_macros.h"
 #include "build/build_config.h"
 #include "chrome/common/safe_browsing/archive_analyzer_results.h"
+#include "chrome/common/safe_browsing/download_protection_util.h"
 #include "chrome/common/safe_browsing/file_type_policies.h"
 #include "third_party/unrar/src/unrar_wrapper.h"
 
@@ -59,8 +61,33 @@ void AnalyzeRarFile(base::File rar_file,
 
     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.
+
+    base::FilePath basename = file_path.BaseName();
+    std::string basename_utf8(basename.AsUTF8Unsafe());
+    bool is_utf8_valid_basename =
+        base::StreamingUtf8Validator::Validate(basename_utf8);
+
     if (is_archive) {
-      archived_archive_filenames.insert(file_path.BaseName());
+      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_length(unpacked_size);
+    } else if (is_executable) {
+      ClientDownloadRequest::ArchivedBinary* archived_binary =
+          results->archived_binary.Add();
+      if (is_utf8_valid_basename)
+        archived_binary->set_file_basename(basename_utf8);
+      archived_binary->set_download_type(
+          download_protection_util::GetDownloadType(file_path));
+      archived_binary->set_length(unpacked_size);
     }
     results->archived_archive_filenames.assign(
         archived_archive_filenames.begin(), archived_archive_filenames.end());

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

@@ -11,6 +11,7 @@
 #include "base/macros.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
+#include "base/strings/utf_string_conversions.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/safe_browsing/archive_analyzer_results.h"
 #include "chrome/services/file_util/file_util_service.h"
@@ -21,7 +22,17 @@
 
 namespace {
 
+#define CDRDT(x) safe_browsing::ClientDownloadRequest_DownloadType_##x
+
 class SandboxedRarAnalyzerTest : public testing::Test {
+ protected:
+  // Constants for validating the data reported by the analyzer.
+  struct BinaryData {
+    const char* file_basename;
+    safe_browsing::ClientDownloadRequest_DownloadType download_type;
+    int64_t length;
+  };
+
  public:
   SandboxedRarAnalyzerTest()
       : browser_thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP),
@@ -47,6 +58,23 @@ class SandboxedRarAnalyzerTest : public testing::Test {
         .AppendASCII("rar")
         .AppendASCII(file_name);
   }
+  // Verifies expectations about a binary found by the analyzer.
+  void ExpectBinary(
+      const BinaryData& data,
+      const safe_browsing::ClientDownloadRequest_ArchivedBinary& binary) {
+    ASSERT_TRUE(binary.has_file_basename());
+    EXPECT_EQ(data.file_basename, binary.file_basename());
+    ASSERT_TRUE(binary.has_download_type());
+    EXPECT_EQ(data.download_type, binary.download_type());
+    ASSERT_FALSE(binary.has_digests());
+    ASSERT_TRUE(binary.has_length());
+    EXPECT_EQ(data.length, binary.length());
+    ASSERT_FALSE(binary.has_signature());
+    ASSERT_FALSE(binary.has_image_headers());
+  }
+  static const BinaryData kEmptyZip;
+  static const BinaryData kNotARar;
+  static const BinaryData kSignedExe;
 
  private:
   // A helper that provides a SandboxedRarAnalyzer::ResultCallback that will
@@ -81,6 +109,21 @@ class SandboxedRarAnalyzerTest : public testing::Test {
   std::unique_ptr<service_manager::Connector> connector_;
 };
 
+const SandboxedRarAnalyzerTest::BinaryData SandboxedRarAnalyzerTest::kEmptyZip =
+    {
+        "empty.zip", CDRDT(RAR_COMPRESSED_ARCHIVE), 22,
+};
+
+const SandboxedRarAnalyzerTest::BinaryData SandboxedRarAnalyzerTest::kNotARar =
+    {
+        "not_a_rar.rar", CDRDT(RAR_COMPRESSED_ARCHIVE), 18,
+};
+
+const SandboxedRarAnalyzerTest::BinaryData
+    SandboxedRarAnalyzerTest::kSignedExe = {
+        "signed.exe", CDRDT(WIN_EXECUTABLE), 37768,
+};
+
 TEST_F(SandboxedRarAnalyzerTest, AnalyzeBenignRar) {
   base::FilePath path;
   ASSERT_NO_FATAL_FAILURE(path = GetFilePath("small_archive.rar"));
@@ -90,8 +133,8 @@ TEST_F(SandboxedRarAnalyzerTest, AnalyzeBenignRar) {
 
   ASSERT_TRUE(results.success);
   EXPECT_FALSE(results.has_executable);
-  EXPECT_EQ(0, results.archived_binary.size());
-  EXPECT_EQ(0u, results.archived_archive_filenames.size());
+  EXPECT_TRUE(results.archived_binary.empty());
+  EXPECT_TRUE(results.archived_archive_filenames.empty());
 }
 
 TEST_F(SandboxedRarAnalyzerTest, AnalyzeRarWithPassword) {
@@ -105,13 +148,13 @@ TEST_F(SandboxedRarAnalyzerTest, AnalyzeRarWithPassword) {
 
   ASSERT_TRUE(results.success);
   EXPECT_FALSE(results.has_executable);
-  EXPECT_EQ(0, results.archived_binary.size());
-  EXPECT_EQ(0u, results.archived_archive_filenames.size());
+  EXPECT_TRUE(results.archived_binary.empty());
+  EXPECT_TRUE(results.archived_archive_filenames.empty());
 }
 
 TEST_F(SandboxedRarAnalyzerTest, AnalyzeRarContainingExecutable) {
   // Can detect when .rar contains executable files.
-  // has_exe.rar contains 1 file: Yahoo-Warez.exe
+  // has_exe.rar contains 1 file: signed.exe
   base::FilePath path;
   ASSERT_NO_FATAL_FAILURE(path = GetFilePath("has_exe.rar"));
 
@@ -120,22 +163,23 @@ TEST_F(SandboxedRarAnalyzerTest, AnalyzeRarContainingExecutable) {
 
   ASSERT_TRUE(results.success);
   EXPECT_TRUE(results.has_executable);
-  EXPECT_EQ(0, results.archived_binary.size());
-  EXPECT_EQ(0u, results.archived_archive_filenames.size());
+  EXPECT_EQ(1, results.archived_binary.size());
+  EXPECT_TRUE(results.archived_archive_filenames.empty());
+  ExpectBinary(kSignedExe, results.archived_binary.Get(0));
 }
 
 TEST_F(SandboxedRarAnalyzerTest, AnalyzeTextAsRar) {
   // Catches when a file isn't a a valid RAR file.
   base::FilePath path;
-  ASSERT_NO_FATAL_FAILURE(path = GetFilePath("not_a_rar.rar"));
+  ASSERT_NO_FATAL_FAILURE(path = GetFilePath(kNotARar.file_basename));
 
   safe_browsing::ArchiveAnalyzerResults results;
   AnalyzeFile(path, &results);
 
   ASSERT_FALSE(results.success);
   EXPECT_FALSE(results.has_executable);
-  EXPECT_EQ(0, results.archived_binary.size());
-  EXPECT_EQ(0u, results.archived_archive_filenames.size());
+  EXPECT_TRUE(results.archived_binary.empty());
+  EXPECT_TRUE(results.archived_archive_filenames.empty());
 }
 
 TEST_F(SandboxedRarAnalyzerTest, AnalyzeRarContainingArchive) {
@@ -149,13 +193,14 @@ TEST_F(SandboxedRarAnalyzerTest, AnalyzeRarContainingArchive) {
 
   ASSERT_TRUE(results.success);
   EXPECT_TRUE(results.has_executable);  // .zip is considered binary executable.
-  EXPECT_EQ(0, results.archived_binary.size());
+  EXPECT_EQ(1, results.archived_binary.size());
   EXPECT_EQ(1u, results.archived_archive_filenames.size());
+  ExpectBinary(kEmptyZip, results.archived_binary.Get(0));
 }
 
 TEST_F(SandboxedRarAnalyzerTest, AnalyzeRarContainingAssortmentOfFiles) {
   // Can detect when .rar contains a mix of different intereting types.
-  // has_exe_rar_text_zip.rar contains: content.exe, not_a_rar.rar, text.txt,
+  // has_exe_rar_text_zip.rar contains: signed.exe, not_a_rar.rar, text.txt,
   // empty.zip
   base::FilePath path;
   ASSERT_NO_FATAL_FAILURE(path = GetFilePath("has_exe_rar_text_zip.rar"));
@@ -165,8 +210,15 @@ TEST_F(SandboxedRarAnalyzerTest, AnalyzeRarContainingAssortmentOfFiles) {
 
   ASSERT_TRUE(results.success);
   EXPECT_TRUE(results.has_executable);
-  EXPECT_EQ(0, results.archived_binary.size());
+  EXPECT_EQ(3, results.archived_binary.size());
+  ExpectBinary(kSignedExe, results.archived_binary.Get(0));
+  ExpectBinary(kNotARar, results.archived_binary.Get(1));
+  ExpectBinary(kEmptyZip, results.archived_binary.Get(2));
   EXPECT_EQ(2u, results.archived_archive_filenames.size());
+  EXPECT_EQ(FILE_PATH_LITERAL("empty.zip"),
+            results.archived_archive_filenames[0].value());
+  EXPECT_EQ(FILE_PATH_LITERAL("not_a_rar.rar"),
+            results.archived_archive_filenames[1].value());
 }
 
 }  // namespace

chrome/test/data/safe_browsing/rar/text.txt

+this is a text file