PatchGap
Chrome · Safebrowsing › Safe_browsing

CVE-2026-87535

Logic Error in Safebrowsing (Safe_browsing)
Medium watch CVSS 6.5 chrome Logic Error 📄 Reporter bug report 🔧 Commit mapped

Overview

Medium
Severity
6.5
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactInformation loss or omission in Safebrowsing
DescriptionInformation loss or omission in Safebrowsing
ComponentSafebrowsing
Sub-componentSafe_browsing
Bug ClassLogic Error
Action tierwatch — logic/policy bug — security-relevant, no memory-unsafety
Computed by PatchGap's deterministic exploitability ruler from the fixing commit. patch-now = RCE-class & attacker-reachable in a default config, or exploited in the wild; watch = real security signal below that bar; noise = no security relevance.
Tracker520572550
Fix commit04f80b813454 (chromium/src) +106/-38
CWECWE-221
CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:U/C:N/I:H/A:N
CISA KEVNot listed
CreditedGoogle
Reported2026-06-06 (per vendor advisory)
Fix landed2026-07-29 (public git)
Disclosed2026-09-08
Patch gap41 days — fix public in source before the advisory

Background

DMG analyzer
Safe Browsing’s macOS component that walks the entries of a downloaded .dmg disk image and copies interesting files into a temp file so they can be inspected and reported.
`archived_binary`
the repeated field in the download-protection request (ClientDownloadRequest) that lists the files found inside an archive; entries absent from it are never seen by Safe Browsing’s backend.
`UpdateResultsForEntry`
the shared routine each archive analyzer calls per entry; per the patch’s own comment it “handles nested archives vs plain files itself,” i.e. it, not the caller, is responsible for classifying an entry.
`DownloadFileType_InspectionType`
an enum (ZIP, RAR, DMG, SEVEN_ZIP, etc.) returned by GetFileType describing how a file extension should be treated.

Root Cause Analysis

In the DMG extraction loop (in DMGAnalyzer::ResumeExtraction per the commit message; the @@ header only shows @@ -213,28 +213,21 @@), a non-Mach-O entry was gated behind a file_type check: it was copied to temp_file_ and passed to UpdateResultsForEntry only when GetFileType(base::FilePath(path)) returned DownloadFileType::ZIP, RAR, DMG, or SEVEN_ZIP. Every other non-Mach-O entry — the removed code’s comment and the commit message cite .pkg, .command, and .tar — fell through and was silently dropped, so it never reached archived_binary. This violated the invariant, upheld by the ZIP/RAR/7z analyzers, that every non-Mach-O entry is forwarded to UpdateResultsForEntry, which itself decides how to classify it.

The fix simply deletes the if (file_type == ... ) wrapper so the copy-to-temp-file and UpdateResultsForEntry call now run for all non-Mach-O entries. Because the omitted files were never reported, this is an information-loss/omission defect in Safe Browsing rather than a memory-safety bug; the diff shows only a filtering condition being removed, not a new sink being reached.

Key insight
The core mistake was letting the caller pre-filter entries by extension-derived DownloadFileType_InspectionType when classification is properly UpdateResultsForEntry’s job, causing every non-archive, non-Mach-O file to be dropped; removing the check restores parity with the other archive analyzers and forwards all entries.

Attack Path

  1. Craft a DMG with a hidden payload An attacker packages a malicious but non-Mach-O, non-nested-archive file (e.g. a .pkg installer, a .command script, or a .tar) inside a .dmg disk image and gets a victim to download it.
  2. Extension misses the allow-list During analysis, GetFileType returns an DownloadFileType_InspectionType that is not ZIP/RAR/DMG/SEVEN_ZIP, so the pre-patch guard skips CopyStreamToFile and UpdateResultsForEntry for that entry.
  3. Entry omitted from the report The file never gets added to archived_binary, so the ClientDownloadRequest sent to Safe Browsing omits it entirely.
  4. Detection is evaded Because Safe Browsing’s backend never sees the payload’s digest/metadata, it cannot flag or block the download on that basis (inferred; the diff proves the omission but not a specific backend verdict change).

Impact Assessment

An attacker gains the ability to smuggle a class of files (non-Mach-O, non-nested-archive entries such as .pkg, .command, .tar) inside a .dmg past Safe Browsing’s download-protection reporting on macOS, since those entries were never placed in archived_binary. This is an information loss/omission in Safe Browsing, matching the advisory’s stated impact and LogicError class; the precondition is that the victim downloads an attacker-supplied DMG containing such an entry. The vendor advisory rates this medium severity and does not report in-the-wild exploitation; NVD scores it CVSS 6.5. The advisory does not report exploitation in the wild or a severity rating, so none is asserted here.

Changed Functions

FunctionChangeNotes
for
chrome/services/file_util/public/cpp/sandboxed_dmg_analyzer_mac_unittest.cc
 modified
TEST
chrome/utility/safe_browsing/mac/dmg_analyzer_unittest.cc
 modified

Files Changed

  • chrome/browser/safe_browsing/download_protection/download_protection_service_unittest.cc
  • chrome/services/file_util/public/cpp/sandboxed_dmg_analyzer_mac_unittest.cc
  • chrome/utility/safe_browsing/mac/dmg_analyzer_unittest.cc
  • chrome/utility/safe_browsing/mac/dmg_analyzer.cc

Audit Directions

  • Caller-side pre-filtering before a shared classifier
    Look for other archive analyzers or extraction loops that gate entries on GetFileType/DownloadFileType_InspectionType before calling a shared per-entry routine like UpdateResultsForEntry, since that routine is designed to classify entries itself and any upstream filter can silently drop data.
  • Silent `continue`/skip on the non-matching branch
    Audit loops where entries that fail an allow-list fall through with no reporting; a missing entry in a security telemetry field (e.g. archived_binary) is an omission bug that produces no error and no test failure absent an explicit count check.
  • Divergence between per-format analyzers
    Compare the DMG, ZIP, RAR, and 7z analyzers for behavioral parity; a rule (like “forward every non-Mach-O/non-nested entry”) enforced in some analyzers but not others is a strong signal of an information-loss gap.

Patch

view commit
From 04f80b813454196b720af0b1c3fefa963c643fef Mon Sep 17 00:00:00 2001
From: Tiffany Song <tiffanysong@google.com>
Date: Wed, 29 Jul 2026 10:43:05 -0700
Subject: [PATCH] safe_browsing: record all non-Mach-O DMG entries

DMGAnalyzer::ResumeExtraction only forwarded a non-Mach-O entry to
UpdateResultsForEntry when its extension mapped to one of the
nested-archive inspection types (ZIP/RAR/DMG/7z), so files like .pkg,
.command and .tar inside a DMG were dropped and never appeared in
archived_binary. The ZIP/RAR/7z analyzers already forward every entry;
UpdateResultsForEntry handles nested archives vs plain files itself.

Remove the inspection-type check so every non-Mach-O entry is copied to
the temp file and reported, matching the other archive analyzers. Relax
the SandboxedDMGAnalyzer and DownloadProtectionService DMG end-to-end
tests, which previously asserted that the mach_o_in_dmg.dmg fixture
produced exactly two archived_binary entries, to tolerate the additional
non-Mach-O entries now reported. Add DMGAnalyzer unit tests covering
.pkg and .tar entries.

Internal review: https://chrome-internal-review.googlesource.com/c/chrome/experimental/chromium/src/+/9604128

Bug: 520572550
Change-Id: I547f9cf2a9d18319084efbc471aead8dcf230833
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8163316
Reviewed-by: Yaw Frempong <yawfrempong@google.com>
Commit-Queue: Tiffany Song <tiffanysong@google.com>
Cr-Commit-Position: refs/heads/main@{#1670393}
---
diff --git a/chrome/utility/safe_browsing/mac/dmg_analyzer.cc b/chrome/utility/safe_browsing/mac/dmg_analyzer.cc
index 894f4f8..06187d7 100644
--- a/chrome/utility/safe_browsing/mac/dmg_analyzer.cc
+++ b/chrome/utility/safe_browsing/mac/dmg_analyzer.cc
@@ -213,28 +213,21 @@
     } else {
       // Get a new `stream` because it was moved from in previous branches.
       stream = iterator_->GetReadStream();
-      DownloadFileType_InspectionType file_type =
-          GetFileType(base::FilePath(path));
-      if (file_type == DownloadFileType::ZIP ||
-          file_type == DownloadFileType::RAR ||
-          file_type == DownloadFileType::DMG ||
-          file_type == DownloadFileType::SEVEN_ZIP) {
-        if (!CopyStreamToFile(*stream, temp_file_)) {
-          continue;
-        }
+      if (!CopyStreamToFile(*stream, temp_file_)) {
+        continue;
+      }
 
-        if (!temp_file_.IsValid()) {
-          continue;
-        }
+      if (!temp_file_.IsValid()) {
+        continue;
+      }
 
-        // TODO(crbug.com/40871873): Support file length here.
-        if (!UpdateResultsForEntry(
-                temp_file_.Duplicate(), GetRootPath().Append(path),
-                /*file_length=*/0,
-                /*is_encrypted=*/false, /*is_directory=*/false,
-                /*contents_valid=*/true)) {
-          return false;
-        }
+      // TODO(crbug.com/40871873): Support file length here.
+      if (!UpdateResultsForEntry(temp_file_.Duplicate(),
+                                 GetRootPath().Append(path),
+                                 /*file_length=*/0,
+                                 /*is_encrypted=*/false, /*is_directory=*/false,
+                                 /*contents_valid=*/true)) {
+        return false;
       }
     }
   }
diff --git a/chrome/browser/safe_browsing/download_protection/download_protection_service_unittest.cc b/chrome/browser/safe_browsing/download_protection/download_protection_service_unittest.cc
index 8b96419c..20c3d138 100644
--- a/chrome/browser/safe_browsing/download_protection/download_protection_service_unittest.cc
+++ b/chrome/browser/safe_browsing/download_protection/download_protection_service_unittest.cc
@@ -2460,21 +2460,26 @@
   EXPECT_EQ(ClientDownloadRequest_DownloadType_MAC_EXECUTABLE,
             request->download_type());
 
-  ASSERT_EQ(2, request->archived_binary().size());
-  for (const auto& binary : request->archived_binary()) {
+  EXPECT_GT(request->archived_binary_size(), 0);
+  bool found_mach_o = false;
+  for (int i = 0; i < request->archived_binary_size(); ++i) {
+    const auto& binary = request->archived_binary(i);
     EXPECT_FALSE(binary.file_path().empty());
-    EXPECT_EQ(ClientDownloadRequest_DownloadType_MAC_EXECUTABLE,
-              binary.download_type());
     ASSERT_TRUE(binary.has_digests());
     EXPECT_TRUE(binary.digests().has_sha256());
     EXPECT_TRUE(binary.has_length());
-    ASSERT_TRUE(binary.has_image_headers());
-    ASSERT_FALSE(binary.image_headers().mach_o_headers().empty());
+    if (binary.image_headers().mach_o_headers().empty()) {
+      continue;
+    }
+    found_mach_o = true;
+    EXPECT_EQ(ClientDownloadRequest_DownloadType_MAC_EXECUTABLE,
+              binary.download_type());
     EXPECT_FALSE(
         binary.image_headers().mach_o_headers().Get(0).mach_header().empty());
     EXPECT_FALSE(
         binary.image_headers().mach_o_headers().Get(0).load_commands().empty());
   }
+  EXPECT_TRUE(found_mach_o);
 
   ASSERT_EQ(1, request->detached_code_signature().size());
   EXPECT_FALSE(request->detached_code_signature().Get(0).file_name().empty());
diff --git a/chrome/services/file_util/public/cpp/sandboxed_dmg_analyzer_mac_unittest.cc b/chrome/services/file_util/public/cpp/sandboxed_dmg_analyzer_mac_unittest.cc
index fdc298d..4b1628c 100644
--- a/chrome/services/file_util/public/cpp/sandboxed_dmg_analyzer_mac_unittest.cc
+++ b/chrome/services/file_util/public/cpp/sandboxed_dmg_analyzer_mac_unittest.cc
@@ -98,7 +98,6 @@
 
   EXPECT_TRUE(results.success);
   EXPECT_TRUE(results.has_executable);
-  EXPECT_EQ(2, results.archived_binary.size());
 
   bool got_executable = false, got_dylib = false;
   for (const auto& binary : results.archived_binary) {
@@ -107,11 +106,11 @@
         safe_browsing::ClientDownloadRequest_MachOHeaders>& headers =
         binary.image_headers().mach_o_headers();
 
-    EXPECT_EQ(safe_browsing::ClientDownloadRequest_DownloadType_MAC_EXECUTABLE,
-              binary.download_type());
-
     if (file_name.find("executablefat") != std::string::npos) {
       got_executable = true;
+      EXPECT_EQ(
+          safe_browsing::ClientDownloadRequest_DownloadType_MAC_EXECUTABLE,
+          binary.download_type());
       ASSERT_EQ(2, headers.size());
 
       const safe_browsing::ClientDownloadRequest_MachOHeaders& arch32 =
@@ -133,6 +132,9 @@
           actual_sha256);
     } else if (file_name.find("lib64.dylib") != std::string::npos) {
       got_dylib = true;
+      EXPECT_EQ(
+          safe_browsing::ClientDownloadRequest_DownloadType_MAC_EXECUTABLE,
+          binary.download_type());
       ASSERT_EQ(1, headers.size());
 
       const safe_browsing::ClientDownloadRequest_MachOHeaders& arch =
@@ -146,8 +148,6 @@
       EXPECT_EQ(
           "2012CE4987B0FA4A5D285DF7E810560E841CFAB3054BC19E1AAB345F862A6C4E",
           actual_sha256);
-    } else {
-      ADD_FAILURE() << "Unexpected result file " << binary.file_path();
     }
   }
 
@@ -173,7 +173,6 @@
 
   EXPECT_TRUE(results.success);
   EXPECT_TRUE(results.has_executable);
-  EXPECT_EQ(2, results.archived_binary.size());
 
   bool got_executable = false, got_dylib = false;
   for (const auto& binary : results.archived_binary) {
@@ -182,11 +181,11 @@
         safe_browsing::ClientDownloadRequest_MachOHeaders>& headers =
         binary.image_headers().mach_o_headers();
 
-    EXPECT_EQ(safe_browsing::ClientDownloadRequest_DownloadType_MAC_EXECUTABLE,
-              binary.download_type());
-
     if (file_name.find("executablefat") != std::string::npos) {
       got_executable = true;
+      EXPECT_EQ(
+          safe_browsing::ClientDownloadRequest_DownloadType_MAC_EXECUTABLE,
+          binary.download_type());
       ASSERT_EQ(2, headers.size());
 
       const safe_browsing::ClientDownloadRequest_MachOHeaders& arch32 =
@@ -208,6 +207,9 @@
           actual_sha256);
     } else if (file_name.find("lib64.dylib") != std::string::npos) {
       got_dylib = true;
+      EXPECT_EQ(
+          safe_browsing::ClientDownloadRequest_DownloadType_MAC_EXECUTABLE,
+          binary.download_type());
       ASSERT_EQ(1, headers.size());
 
       const safe_browsing::ClientDownloadRequest_MachOHeaders& arch =
@@ -221,8 +223,6 @@
       EXPECT_EQ(
           "2012CE4987B0FA4A5D285DF7E810560E841CFAB3054BC19E1AAB345F862A6C4E",
           actual_sha256);
-    } else {
-      ADD_FAILURE() << "Unexpected result file " << binary.file_path();
     }
   }
 
diff --git a/chrome/utility/safe_browsing/mac/dmg_analyzer_unittest.cc b/chrome/utility/safe_browsing/mac/dmg_analyzer_unittest.cc
index a6b17936..1444a51 100644
--- a/chrome/utility/safe_browsing/mac/dmg_analyzer_unittest.cc
+++ b/chrome/utility/safe_browsing/mac/dmg_analyzer_unittest.cc
@@ -312,6 +312,76 @@
   EXPECT_EQ(inner, written_bytes);
 }
 
+// Non-Mach-O entries with extensions that are checked binaries (e.g. .pkg)
+// should be recorded in `archived_binary` so they are included in the download
+// protection request, matching the behavior of the other archive analyzers.
+TEST(DMGAnalyzerTest, ContainedPkgRecorded) {
+  base::test::TaskEnvironment task_environment;
+  DMGAnalyzer analyzer_;
+  base::FilePath temp_path;
+  base::File temp_file;
+  base::CreateTemporaryFile(&temp_path);
+  temp_file.Initialize(
+      temp_path, (base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_READ |
+                  base::File::FLAG_WRITE | base::File::FLAG_WIN_TEMPORARY |
+                  base::File::FLAG_DELETE_ON_CLOSE));
+
+  MockDMGIterator::FileList file_list{
+      {"DMG/Install.pkg", {'x', 'a', 'r', '!', 0x00, 0x1c, 0x00, 0x01}},
+  };
+
+  std::unique_ptr<MockDMGIterator> iterator =
+      std::make_unique<MockDMGIterator>(true, file_list);
+  safe_browsing::ArchiveAnalyzerResults results;
+  base::RunLoop run_loop;
+  analyzer_.AnalyzeDMGFileForTesting(std::move(iterator), &results,
+                                     std::move(temp_file),
+                                     run_loop.QuitClosure());
+  run_loop.Run();
+
+  EXPECT_TRUE(results.success);
+  EXPECT_TRUE(results.has_executable);
+  ASSERT_EQ(1, results.archived_binary.size());
+  EXPECT_EQ("DMG/Install.pkg", results.archived_binary.Get(0).file_path());
+  EXPECT_TRUE(results.archived_binary.Get(0).is_executable());
+  EXPECT_TRUE(results.archived_binary.Get(0).has_digests());
+}
+
+// Non-Mach-O entries with extensions that are archives (e.g. .tar) should be
+// recorded in `archived_binary`, matching the behavior of the other archive
+// analyzers.
+TEST(DMGAnalyzerTest, ContainedTarRecorded) {
+  base::test::TaskEnvironment task_environment;
+  DMGAnalyzer analyzer_;
+  base::FilePath temp_path;
+  base::File temp_file;
+  base::CreateTemporaryFile(&temp_path);
+  temp_file.Initialize(
+      temp_path, (base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_READ |
+                  base::File::FLAG_WRITE | base::File::FLAG_WIN_TEMPORARY |
+                  base::File::FLAG_DELETE_ON_CLOSE));
+
+  MockDMGIterator::FileList file_list{
+      {"DMG/payload.tar", {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08}},
+  };
+
+  std::unique_ptr<MockDMGIterator> iterator =
+      std::make_unique<MockDMGIterator>(true, file_list);
+  safe_browsing::ArchiveAnalyzerResults results;
+  base::RunLoop run_loop;
+  analyzer_.AnalyzeDMGFileForTesting(std::move(iterator), &results,
+                                     std::move(temp_file),
+                                     run_loop.QuitClosure());
+  run_loop.Run();
+
+  EXPECT_TRUE(results.success);
+  EXPECT_TRUE(results.has_archive);
+  ASSERT_EQ(1, results.archived_binary.size());
+  EXPECT_EQ("DMG/payload.tar", results.archived_binary.Get(0).file_path());
+  EXPECT_TRUE(results.archived_binary.Get(0).is_archive());
+  EXPECT_TRUE(results.archived_binary.Get(0).has_digests());
+}
+
 }  // namespace
 }  // namespace dmg
 }  // namespace safe_browsing
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/chrome/browser/safe_browsing/download_protection/download_protection_service_unittest.cc b/chrome/browser/safe_browsing/download_protection/download_protection_service_unittest.cc
index 8b96419c..20c3d138 100644
--- a/chrome/browser/safe_browsing/download_protection/download_protection_service_unittest.cc
+++ b/chrome/browser/safe_browsing/download_protection/download_protection_service_unittest.cc
@@ -2460,21 +2460,26 @@
   EXPECT_EQ(ClientDownloadRequest_DownloadType_MAC_EXECUTABLE,
             request->download_type());
 
-  ASSERT_EQ(2, request->archived_binary().size());
-  for (const auto& binary : request->archived_binary()) {
+  EXPECT_GT(request->archived_binary_size(), 0);
+  bool found_mach_o = false;
+  for (int i = 0; i < request->archived_binary_size(); ++i) {
+    const auto& binary = request->archived_binary(i);
     EXPECT_FALSE(binary.file_path().empty());
-    EXPECT_EQ(ClientDownloadRequest_DownloadType_MAC_EXECUTABLE,
-              binary.download_type());
     ASSERT_TRUE(binary.has_digests());
     EXPECT_TRUE(binary.digests().has_sha256());
     EXPECT_TRUE(binary.has_length());
-    ASSERT_TRUE(binary.has_image_headers());
-    ASSERT_FALSE(binary.image_headers().mach_o_headers().empty());
+    if (binary.image_headers().mach_o_headers().empty()) {
+      continue;
+    }
+    found_mach_o = true;
+    EXPECT_EQ(ClientDownloadRequest_DownloadType_MAC_EXECUTABLE,
+              binary.download_type());
     EXPECT_FALSE(
         binary.image_headers().mach_o_headers().Get(0).mach_header().empty());
     EXPECT_FALSE(
         binary.image_headers().mach_o_headers().Get(0).load_commands().empty());
   }
+  EXPECT_TRUE(found_mach_o);
 
   ASSERT_EQ(1, request->detached_code_signature().size());
   EXPECT_FALSE(request->detached_code_signature().Get(0).file_name().empty());
diff --git a/chrome/services/file_util/public/cpp/sandboxed_dmg_analyzer_mac_unittest.cc b/chrome/services/file_util/public/cpp/sandboxed_dmg_analyzer_mac_unittest.cc
index fdc298d..4b1628c 100644
--- a/chrome/services/file_util/public/cpp/sandboxed_dmg_analyzer_mac_unittest.cc
+++ b/chrome/services/file_util/public/cpp/sandboxed_dmg_analyzer_mac_unittest.cc
@@ -98,7 +98,6 @@
 
   EXPECT_TRUE(results.success);
   EXPECT_TRUE(results.has_executable);
-  EXPECT_EQ(2, results.archived_binary.size());
 
   bool got_executable = false, got_dylib = false;
   for (const auto& binary : results.archived_binary) {
@@ -107,11 +106,11 @@
         safe_browsing::ClientDownloadRequest_MachOHeaders>& headers =
         binary.image_headers().mach_o_headers();
 
-    EXPECT_EQ(safe_browsing::ClientDownloadRequest_DownloadType_MAC_EXECUTABLE,
-              binary.download_type());
-
     if (file_name.find("executablefat") != std::string::npos) {
       got_executable = true;
+      EXPECT_EQ(
+          safe_browsing::ClientDownloadRequest_DownloadType_MAC_EXECUTABLE,
+          binary.download_type());
       ASSERT_EQ(2, headers.size());
 
       const safe_browsing::ClientDownloadRequest_MachOHeaders& arch32 =
@@ -133,6 +132,9 @@
           actual_sha256);
     } else if (file_name.find("lib64.dylib") != std::string::npos) {
       got_dylib = true;
+      EXPECT_EQ(
+          safe_browsing::ClientDownloadRequest_DownloadType_MAC_EXECUTABLE,
+          binary.download_type());
       ASSERT_EQ(1, headers.size());
 
       const safe_browsing::ClientDownloadRequest_MachOHeaders& arch =
@@ -146,8 +148,6 @@
       EXPECT_EQ(
           "2012CE4987B0FA4A5D285DF7E810560E841CFAB3054BC19E1AAB345F862A6C4E",
           actual_sha256);
-    } else {
-      ADD_FAILURE() << "Unexpected result file " << binary.file_path();
     }
   }
 
@@ -173,7 +173,6 @@
 
   EXPECT_TRUE(results.success);
   EXPECT_TRUE(results.has_executable);
-  EXPECT_EQ(2, results.archived_binary.size());
 
   bool got_executable = false, got_dylib = false;
   for (const auto& binary : results.archived_binary) {
@@ -182,11 +181,11 @@
         safe_browsing::ClientDownloadRequest_MachOHeaders>& headers =
         binary.image_headers().mach_o_headers();
 
-    EXPECT_EQ(safe_browsing::ClientDownloadRequest_DownloadType_MAC_EXECUTABLE,
-              binary.download_type());
-
     if (file_name.find("executablefat") != std::string::npos) {
       got_executable = true;
+      EXPECT_EQ(
+          safe_browsing::ClientDownloadRequest_DownloadType_MAC_EXECUTABLE,
+          binary.download_type());
       ASSERT_EQ(2, headers.size());
 
       const safe_browsing::ClientDownloadRequest_MachOHeaders& arch32 =
@@ -208,6 +207,9 @@
           actual_sha256);
     } else if (file_name.find("lib64.dylib") != std::string::npos) {
       got_dylib = true;
+      EXPECT_EQ(
+          safe_browsing::ClientDownloadRequest_DownloadType_MAC_EXECUTABLE,
+          binary.download_type());
       ASSERT_EQ(1, headers.size());
 
       const safe_browsing::ClientDownloadRequest_MachOHeaders& arch =
@@ -221,8 +223,6 @@
       EXPECT_EQ(
           "2012CE4987B0FA4A5D285DF7E810560E841CFAB3054BC19E1AAB345F862A6C4E",
           actual_sha256);
-    } else {
-      ADD_FAILURE() << "Unexpected result file " << binary.file_path();
     }
   }
diff --git a/chrome/utility/safe_browsing/mac/dmg_analyzer_unittest.cc b/chrome/utility/safe_browsing/mac/dmg_analyzer_unittest.cc
index a6b17936..1444a51 100644
--- a/chrome/utility/safe_browsing/mac/dmg_analyzer_unittest.cc
+++ b/chrome/utility/safe_browsing/mac/dmg_analyzer_unittest.cc
@@ -312,6 +312,76 @@
   EXPECT_EQ(inner, written_bytes);
 }
 
+// Non-Mach-O entries with extensions that are checked binaries (e.g. .pkg)
+// should be recorded in `archived_binary` so they are included in the download
+// protection request, matching the behavior of the other archive analyzers.
+TEST(DMGAnalyzerTest, ContainedPkgRecorded) {
+  base::test::TaskEnvironment task_environment;
+  DMGAnalyzer analyzer_;
+  base::FilePath temp_path;
+  base::File temp_file;
+  base::CreateTemporaryFile(&temp_path);
+  temp_file.Initialize(
+      temp_path, (base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_READ |
+                  base::File::FLAG_WRITE | base::File::FLAG_WIN_TEMPORARY |
+                  base::File::FLAG_DELETE_ON_CLOSE));
+
+  MockDMGIterator::FileList file_list{
+      {"DMG/Install.pkg", {'x', 'a', 'r', '!', 0x00, 0x1c, 0x00, 0x01}},
+  };
+
+  std::unique_ptr<MockDMGIterator> iterator =
+      std::make_unique<MockDMGIterator>(true, file_list);
+  safe_browsing::ArchiveAnalyzerResults results;
+  base::RunLoop run_loop;
+  analyzer_.AnalyzeDMGFileForTesting(std::move(iterator), &results,
+                                     std::move(temp_file),
+                                     run_loop.QuitClosure());
+  run_loop.Run();
+
+  EXPECT_TRUE(results.success);
+  EXPECT_TRUE(results.has_executable);
+  ASSERT_EQ(1, results.archived_binary.size());
+  EXPECT_EQ("DMG/Install.pkg", results.archived_binary.Get(0).file_path());
+  EXPECT_TRUE(results.archived_binary.Get(0).is_executable());
+  EXPECT_TRUE(results.archived_binary.Get(0).has_digests());
+}
+
+// Non-Mach-O entries with extensions that are archives (e.g. .tar) should be
+// recorded in `archived_binary`, matching the behavior of the other archive
+// analyzers.
+TEST(DMGAnalyzerTest, ContainedTarRecorded) {
+  base::test::TaskEnvironment task_environment;
+  DMGAnalyzer analyzer_;
+  base::FilePath temp_path;
+  base::File temp_file;
+  base::CreateTemporaryFile(&temp_path);
+  temp_file.Initialize(
+      temp_path, (base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_READ |
+                  base::File::FLAG_WRITE | base::File::FLAG_WIN_TEMPORARY |
+                  base::File::FLAG_DELETE_ON_CLOSE));
+
+  MockDMGIterator::FileList file_list{
+      {"DMG/payload.tar", {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08}},
+  };
+
+  std::unique_ptr<MockDMGIterator> iterator =
+      std::make_unique<MockDMGIterator>(true, file_list);
+  safe_browsing::ArchiveAnalyzerResults results;
+  base::RunLoop run_loop;
+  analyzer_.AnalyzeDMGFileForTesting(std::move(iterator), &results,
+                                     std::move(temp_file),
+                                     run_loop.QuitClosure());
+  run_loop.Run();
+
+  EXPECT_TRUE(results.success);
+  EXPECT_TRUE(results.has_archive);
+  ASSERT_EQ(1, results.archived_binary.size());
+  EXPECT_EQ("DMG/payload.tar", results.archived_binary.Get(0).file_path());
+  EXPECT_TRUE(results.archived_binary.Get(0).is_archive());
+  EXPECT_TRUE(results.archived_binary.Get(0).has_digests());
+}
+
 }  // namespace
 }  // namespace dmg
 }  // namespace safe_browsing
Loading diff…

Original Bug Report

reported by vm...@google.com

Potential Safe Browsing nested-content reporting bypass in DMGAnalyzer for checked binaries

Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.

Overview: The DMGAnalyzer potentially silently drops nested non-Mach-O entries whose file types are not ZIP, RAR, DMG, or 7z (such as .pkg, .command, and .tar files), failing to record them as archived binaries. While sibling analyzers (ZIP, RAR, 7z) unconditionally record these entries, DMGAnalyzer lacks a fallthrough execution path for them. This causes a potential detection asymmetry where malicious payloads packaged inside a DMG can bypass Safe Browsing telemetry.

Affected files:

  • chrome/utility/safe_browsing/mac/dmg_analyzer.cc

Estimated timestamp from git blame: 2023-04-26

Root Cause

In chrome/utility/safe_browsing/mac/dmg_analyzer.cc (lines 200-226), DMGAnalyzer::ResumeExtraction() is responsible for iterating over and analyzing files within a DMG. However, non-Mach-O entries are handled in an else block that only processes files of four specific inspection types (ZIP, RAR, DMG, or SEVEN_ZIP):

} else {
  // Get a new `stream` because it was read from in previous branches.
  stream = iterator_->GetReadStream();
  DownloadFileType_InspectionType file_type =
      GetFileType(base::FilePath(path));
  if (file_type == DownloadFileType::ZIP ||
      file_type == DownloadFileType::RAR ||
      file_type == DownloadFileType::DMG ||
      file_type == DownloadFileType::SEVEN_ZIP) {
    if (!CopyStreamToFile(*stream, temp_file_)) {
      continue;
    }
    ...
    if (!UpdateResultsForEntry(temp_file_.Duplicate(), ...)) {
      return false;
    }
  }
  // No else: non-{ZIP, RAR, DMG, 7z} and non-Mach-O files are silently dropped.
}

By comparison, sibling analyzers (ZIP, RAR, 7z) call UpdateResultsForEntry() unconditionally for every entry in the archive.

UpdateResultsForEntry() subsequently calls UpdateArchiveAnalyzerResultsWithFile(), which checks if the file is a checked binary (IsCheckedBinaryFile()) or an archive (IsArchiveFile()). If so, it computes its SHA256 and appends it to results->archived_binary so that it is included in the Safe Browsing telemetry request.

Because the DMGAnalyzer does not invoke UpdateResultsForEntry() for other file types, common macOS payloads such as flat-package installer files (.pkg, .mpkg), executable scripts (.command), or compressed archives (.tar, .gz, .bz2) inside a DMG are silently dropped and never recorded.

Potential Trigger Path

Note: The following steps are a potential vector, as our tooling cannot execute code.

  1. An attacker packages a malicious installer file Install.pkg inside a DMG volume (evil.dmg) and hosts it on a server.
  2. A user downloads evil.dmg on macOS using Google Chrome.
  3. Chrome triggers the out-of-process SandboxedDMGAnalyzer within the utility process.
  4. DMGAnalyzer::ResumeExtraction() iterates over the DMG’s files. For Install.pkg, the parser determines it is not a Mach-O file (since its magic bytes are xar!), entering the else block.
  5. The analyzer calls GetFileType("Install.pkg"), which evaluates to DownloadFileType::NONE because .pkg has no special inspection_type in download_file_types.asciipb.
  6. The if check on lines 205-208 evaluates to false, so the file is skipped.
  7. The browser process receives the results with an empty archived_binary list. The Safe Browsing backend receives a telemetry ping with no record of Install.pkg or its hash, allowing the download to potentially bypass blocklist and reputation checks.

Suggested Fix

Modify the check in chrome/utility/safe_browsing/mac/dmg_analyzer.cc to also extract and process the file if it is a checked binary or archive file:

} else {
  stream = iterator_->GetReadStream();
  base::FilePath file_path(path);
  DownloadFileType_InspectionType file_type = GetFileType(file_path);
  
  bool should_analyze = 
      file_type == DownloadFileType::ZIP ||
      file_type == DownloadFileType::RAR ||
      file_type == DownloadFileType::DMG ||
      file_type == DownloadFileType::SEVEN_ZIP ||
      safe_browsing::FileTypePolicies::GetInstance()->IsCheckedBinaryFile(file_path) ||
      safe_browsing::FileTypePolicies::GetInstance()->IsArchiveFile(file_path);

  if (should_analyze) {
    if (!CopyStreamToFile(*stream, temp_file_)) {
      continue;
    }
    if (!temp_file_.IsValid()) {
      continue;
    }
    if (!UpdateResultsForEntry(
            temp_file_.Duplicate(), GetRootPath().Append(path),
            /*file_length=*/0,
            /*is_encrypted=*/false, /*is_directory=*/false,
            /*contents_valid=*/true)) {
      return false;
    }
  }
}

Evaluated with Chrome root at commit: e9507a33bb4148ee071aaaf8a7e9ad68770359bf

Results so far have been promising, but there can be wrong deductions. Feel free to adjust as follows:

  • If you are familiar with the severity guidelines, you may adjust the severity.
  • If this is a false positive, and there’s no work to be done, please close as WAI.
  • If there is work to do here but not a vulnerability, please change the issue type to Task/Bug/FR.

Data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.

View on issue tracker
On This Page