Nested Rar Analyzer

This CL contains the implementation for unpacking nested rar file by the
safe archive analyzer. Unpacking nested rar files allows the Chrome
to combat cookie theft.

Bug: 1373671
Change-Id: I951a66adee5b3daafdd5c4dc7cb00a87528e51dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4350049
Reviewed-by: Daniel Rubery <drubery@chromium.org>
Commit-Queue: Adam Psarouthakis <psarouthakis@google.com>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Brendon Tiszka <tiszka@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1120655}

chrome/browser/safe_browsing/download_protection/file_analyzer.cc

@@ -185,7 +185,7 @@ void FileAnalyzer::OnZipAnalysisFinished(
 void FileAnalyzer::StartExtractRarFeatures() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  // We give the rar analyzer a weak pointer to this object.  Since the
+  // We give the rar analyzer a weak pointer to this object. Since the
   // analyzer is refcounted, it might outlive the request.
   rar_analyzer_ = SandboxedRarAnalyzer::CreateAnalyzer(
       tmp_path_,

chrome/common/safe_browsing/BUILD.gn

@@ -58,25 +58,6 @@ if (safe_browsing_mode == 1) {
     public_deps = [ "//components/safe_browsing/core/common/proto:csd_proto" ]
   }
 
-  source_set("rar_analyzer") {
-    sources = [
-      "rar_analyzer.cc",
-      "rar_analyzer.h",
-    ]
-
-    deps = [
-      ":archive_analyzer_results",
-      ":download_type_util",
-      "//base",
-      "//base:i18n",
-      "//components/safe_browsing/content/common:file_type_policies",
-      "//components/safe_browsing/core/common",
-      "//third_party/unrar:unrar",
-    ]
-
-    public_deps = [ "//components/safe_browsing/core/common/proto:csd_proto" ]
-  }
-
   if (is_linux || is_win) {
     source_set("document_analyzer") {
       sources = [
@@ -162,6 +143,8 @@ source_set("safe_browsing") {
       "protobuf_message_log_macros.h",
       "protobuf_message_read_macros.h",
       "protobuf_message_write_macros.h",
+      "rar_analyzer.cc",
+      "rar_analyzer.h",
       "seven_zip_analyzer.cc",
       "seven_zip_analyzer.h",
       "zip_analyzer.cc",
@@ -172,9 +155,12 @@ source_set("safe_browsing") {
       ":archive_analyzer_results",
       ":binary_feature_extractor",
       ":download_type_util",
-      ":rar_analyzer",
+      "//base",
+      "//base:i18n",
+      "//components/safe_browsing/content/common:file_type_policies",
       "//components/safe_browsing/core/common",
       "//third_party/lzma_sdk/google:seven_zip_reader",
+      "//third_party/unrar:unrar",
     ]
 
     if (is_linux) {

chrome/common/safe_browsing/archive_analyzer_results.cc

@@ -143,15 +143,11 @@ void UpdateArchiveAnalyzerResultsWithFile(base::FilePath path,
   }
 }
 
-bool IsArchivePath(base::FilePath path) {
-  // TODO(crbug.com/1373671): Add support for SevenZip, Rar, and Dmg
-  // archives.
-  if (FileTypePolicies::GetInstance()
-          ->PolicyForFile(path, GURL{}, nullptr)
-          .inspection_type() == DownloadFileType::ZIP) {
-    return true;
-  }
-  return false;
+safe_browsing::DownloadFileType_InspectionType GetFileType(
+    base::FilePath path) {
+  return FileTypePolicies::GetInstance()
+      ->PolicyForFile(path, GURL{}, nullptr)
+      .inspection_type();
 }
 
 void SetNameForContainedFile(

chrome/common/safe_browsing/archive_analyzer_results.h

@@ -12,6 +12,7 @@
 
 #include "base/files/file_path.h"
 #include "build/build_config.h"
+#include "components/safe_browsing/content/common/proto/download_file_types.pb.h"
 #include "components/safe_browsing/core/common/proto/csd.pb.h"
 
 namespace base {
@@ -71,8 +72,8 @@ void UpdateArchiveAnalyzerResultsWithFile(base::FilePath path,
                                           bool is_encrypted,
                                           ArchiveAnalyzerResults* results);
 
-// Returns true if the nested archive should be unpacked by an analyzer.
-bool IsArchivePath(base::FilePath path);
+// Returns the `DownloadFileType_InspectionType` of the file path.
+safe_browsing::DownloadFileType_InspectionType GetFileType(base::FilePath path);
 
 // Update the `archived_binary` with the string value path name.
 void SetNameForContainedFile(

chrome/common/safe_browsing/rar_analyzer.cc

@@ -16,66 +16,129 @@
 #include "build/build_config.h"
 #include "chrome/common/safe_browsing/archive_analyzer_results.h"
 #include "chrome/common/safe_browsing/download_type_util.h"
+#include "chrome/common/safe_browsing/zip_analyzer.h"
 #include "components/safe_browsing/content/common/file_type_policies.h"
 #include "components/safe_browsing/core/common/features.h"
 #include "third_party/unrar/google/unrar_wrapper.h"
 
 namespace safe_browsing {
-namespace rar_analyzer {
 
-namespace {
+RarAnalyzer::~RarAnalyzer() = default;
 
-// The maximum duration of RAR analysis, in milliseconds.
-const int kRarAnalysisTimeoutMs = 10000;
+RarAnalyzer::RarAnalyzer() = default;
 
-}  // namespace
-
-void AnalyzeRarFile(base::File rar_file,
-                    base::File temp_file,
-                    ArchiveAnalyzerResults* results) {
-  base::Time start_time = base::Time::Now();
-  results->success = false;
-  results->file_count = 0;
-  results->directory_count = 0;
+void RarAnalyzer::Init(base::File rar_file,
+                       base::FilePath root_rar_path,
+                       FinishedAnalysisCallback finished_analysis_callback,
+                       GetTempFileCallback get_temp_file_callback,
+                       ArchiveAnalyzerResults* results) {
+  results_ = results;
+  root_rar_path_ = root_rar_path;
+  finished_analysis_callback_ = std::move(finished_analysis_callback);
+  get_temp_file_callback_ = get_temp_file_callback;
+  rar_file_ = std::move(rar_file);
+  get_temp_file_callback_.Run(
+      base::BindOnce(&RarAnalyzer::FilePreChecks, weak_factory_.GetWeakPtr()));
+}
 
+void RarAnalyzer::FilePreChecks(base::File temp_file) {
+  if (!temp_file.IsValid()) {
+    results_->success = false;
+    results_->analysis_result = ArchiveAnalysisResult::kFailedToOpenTempFile;
+    std::move(finished_analysis_callback_).Run();
+    return;
+  }
+  temp_file_ = std::move(temp_file);
   // If the file is too big to unpack, return failure. This will still send a
   // ping as an "invalid" RAR.
   bool too_big_to_unpack =
-      base::checked_cast<uint64_t>(rar_file.GetLength()) >
+      base::checked_cast<uint64_t>(rar_file_.GetLength()) >
       FileTypePolicies::GetInstance()->GetMaxFileSizeToAnalyze("rar");
   if (too_big_to_unpack) {
-    results->analysis_result = ArchiveAnalysisResult::kTooLarge;
+    results_->success = false;
+    results_->analysis_result = ArchiveAnalysisResult::kTooLarge;
+    std::move(finished_analysis_callback_).Run();
     return;
   }
-
-  third_party_unrar::RarReader reader;
-  if (!reader.Open(std::move(rar_file), temp_file.Duplicate())) {
-    results->analysis_result = ArchiveAnalysisResult::kUnknown;
+  // `rar_file_` is consumed by the reader and cannot be used after
+  // this point.
+  if (!reader_.Open(std::move(rar_file_), temp_file_.Duplicate())) {
+    results_->success = false;
+    results_->analysis_result = ArchiveAnalysisResult::kUnknown;
+    std::move(finished_analysis_callback_).Run();
     return;
   }
+  AnalyzeRarFile();
+}
 
-  bool timeout = false;
-  while (reader.ExtractNextEntry()) {
-    if (base::Time::Now() - start_time >
-        base::Milliseconds(kRarAnalysisTimeoutMs)) {
-      timeout = true;
-      break;
-    }
+void RarAnalyzer::AnalyzeRarFile() {
+  results_->success = false;
+  while (reader_.ExtractNextEntry()) {
     const third_party_unrar::RarReader::EntryInfo& entry =
-        reader.current_entry();
-    UpdateArchiveAnalyzerResultsWithFile(entry.file_path, &temp_file,
-                                         entry.file_size, entry.is_encrypted,
-                                         results);
-    if (entry.is_directory)
-      results->directory_count++;
-    else
-      results->file_count++;
+        reader_.current_entry();
+    if (entry.is_directory) {
+      results_->directory_count++;
+    } else {
+      results_->file_count++;
+    }
+    has_encrypted_ |= entry.is_encrypted;
+    if (base::FeatureList::IsEnabled(kNestedArchives) && !entry.is_encrypted &&
+        AnalyzeNestedArchive(GetFileType(entry.file_path), entry.file_path)) {
+      return;
+    } else {
+      UpdateArchiveAnalyzerResultsWithFile(
+          root_rar_path_.Append(entry.file_path), &temp_file_, entry.file_size,
+          entry.is_encrypted, results_);
+    }
   }
+  results_->success = true;
+  results_->analysis_result = ArchiveAnalysisResult::kValid;
+  std::move(finished_analysis_callback_).Run();
+}
 
-  results->analysis_result =
-      timeout ? ArchiveAnalysisResult::kTimeout : ArchiveAnalysisResult::kValid;
-  results->success = !timeout;
+bool RarAnalyzer::AnalyzeNestedArchive(
+    safe_browsing::DownloadFileType_InspectionType file_type,
+    base::FilePath path) {
+  FinishedAnalysisCallback nested_analysis_finished_callback =
+      base::BindOnce(&RarAnalyzer::NestedAnalysisFinished,
+                     weak_factory_.GetWeakPtr(), root_rar_path_.Append(path));
+  if (file_type == DownloadFileType::ZIP) {
+    nested_zip_analyzer_ = std::make_unique<safe_browsing::ZipAnalyzer>();
+    nested_zip_analyzer_->Init(temp_file_.Duplicate(),
+                               root_rar_path_.Append(path),
+                               std::move(nested_analysis_finished_callback),
+                               get_temp_file_callback_, results_);
+    return true;
+  } else if (file_type == DownloadFileType::RAR) {
+    nested_rar_analyzer_ = std::make_unique<safe_browsing::RarAnalyzer>();
+    nested_rar_analyzer_->Init(temp_file_.Duplicate(),
+                               root_rar_path_.Append(path),
+                               std::move(nested_analysis_finished_callback),
+                               get_temp_file_callback_, results_);
+    return true;
+  }
+  return false;
+}
+
+void RarAnalyzer::NestedAnalysisFinished(base::FilePath path) {
+  // `results_->success` will contain the latest analyzer's success
+  // status and can be used to determine if the nester archive unpacked
+  // successfully.
+  // TODO(crbug.com/1373671): Add support for SevenZip, Rar, and Dmg
+  // archives.
+  if (!results_->success) {
+    results_->has_archive = true;
+    results_->archived_archive_filenames.push_back(path.BaseName());
+    ClientDownloadRequest::ArchivedBinary* archived_archive =
+        results_->archived_binary.Add();
+    archived_archive->set_download_type(ClientDownloadRequest::ARCHIVE);
+    archived_archive->set_is_encrypted(false);
+    archived_archive->set_is_archive(true);
+    SetNameForContainedFile(path, archived_archive);
+    SetLengthAndDigestForContainedFile(&temp_file_, temp_file_.GetLength(),
+                                       archived_archive);
+  }
+  AnalyzeRarFile();
 }
 
-}  // namespace rar_analyzer
 }  // namespace safe_browsing

chrome/common/safe_browsing/rar_analyzer.h

@@ -22,26 +22,80 @@
 #define CHROME_COMMON_SAFE_BROWSING_RAR_ANALYZER_H_
 
 #include "base/files/file.h"
+#include "base/functional/callback.h"
+#include "components/safe_browsing/content/common/proto/download_file_types.pb.h"
+#include "third_party/unrar/google/unrar_wrapper.h"
 
 namespace safe_browsing {
 
+class ZipAnalyzer;
+
 struct ArchiveAnalyzerResults;
+using FinishedAnalysisCallback = base::OnceCallback<void()>;
+using GetTempFileCallback =
+    base::RepeatingCallback<void(base::OnceCallback<void(base::File)>)>;
+
+class RarAnalyzer {
+ public:
+  RarAnalyzer();
+
+  virtual ~RarAnalyzer();
+
+  RarAnalyzer(const RarAnalyzer&) = delete;
+  RarAnalyzer& operator=(const RarAnalyzer&) = delete;
+
+  // Loads variables and fetches the needed `temp_file` from the
+  // `temp_file_getter`.
+  void Init(base::File rar_file,
+            base::FilePath root_rar_path,
+            FinishedAnalysisCallback finished_analysis_callback,
+            GetTempFileCallback get_temp_file_callback,
+            ArchiveAnalyzerResults* results);
+
+ private:
+  // Ensures that the `rar_file` and `temp_file` are both valid and should
+  // be analyzed.
+  void FilePreChecks(base::File temp_file);
+
+  // Analyzes the `rar_file_`. Creates a `nested_rar_analyzer_` when a
+  // nested archive is found. Waits for that archive to be analyzed
+  // before continuing.
+  void AnalyzeRarFile();
+
+  // Checks the `file_type` and creates a new analyzer if the file is a
+  // nested archive. Returns true when a new analyzer is created, and
+  // false when one is not.
+  bool AnalyzeNestedArchive(
+      safe_browsing::DownloadFileType_InspectionType file_type,
+      base::FilePath path);
+
+  // Called from `nested_rar_analyzer_` using
+  // `finished_analysis_callback_`. If unsuccessful, records unpacked
+  // archive in results
+  void NestedAnalysisFinished(base::FilePath path);
+
+  bool has_encrypted_ = false;
+
+  // Tracks overall file path while unpacking nested archives.
+  base::FilePath root_rar_path_;
+
+  base::File rar_file_;
+  base::File temp_file_;
+  third_party_unrar::RarReader reader_;
+  ArchiveAnalyzerResults* results_;
+
+  FinishedAnalysisCallback finished_analysis_callback_;
+  GetTempFileCallback get_temp_file_callback_;
+
+  // The below analyzers are used to unpack nested archives using
+  // DFS to unpacked nested archives.
+  // TODO(crbug.com/1426164) Create a common class to hold all analyzers.
+  std::unique_ptr<safe_browsing::RarAnalyzer> nested_rar_analyzer_;
+  std::unique_ptr<safe_browsing::ZipAnalyzer> nested_zip_analyzer_;
+
+  base::WeakPtrFactory<RarAnalyzer> weak_factory_{this};
+};
 
-namespace rar_analyzer {
-
-// |rar_file| is a platform-agnostic handle to the file, and |temp_file| is a
-// handle for a temporary file the sandbox can write to. Since |AnalyzeRarFile|
-// runs inside a sandbox, it isn't allowed to open file handles. So both files
-// are opened in |SandboxedRarAnalyzer|, which runs in the browser process, and
-// the handles are passed here. The function populates the various fields in
-// |results| based on the results of parsing the rar file. If the parsing fails
-// for any reason, including crashing the sandbox process, the browser process
-// considers the file safe.
-void AnalyzeRarFile(base::File rar_file,
-                    base::File temp_file,
-                    ArchiveAnalyzerResults* results);
-
-}  // namespace rar_analyzer
 }  // namespace safe_browsing
 
 #endif  // CHROME_COMMON_SAFE_BROWSING_RAR_ANALYZER_H_