Medium watch CVSS 4.4 chrome Logic Error 📄 Reporter bug report 🔧 Commit mapped

Overview

Medium
Severity
4.4
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactInappropriate implementation in Downloads
DescriptionInappropriate implementation in Downloads
ComponentDownloads
Sub-componentUI>Browser>Downloads
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.
Tracker429140219
Fix commitade24699eeba (chromium/src) +109/-48
CWECWE-290
CVSS vectorCVSS:3.1/AV:L/AC:L/PR:N/UI:R/S:U/C:L/I:L/A:N
CISA KEVNot listed
CreditedEric Lawrence of Microsoft
Reported2025-07-02 (per vendor advisory)
Fix landed2025-10-22 (public git)
Disclosed2025-12-02
Patch gap41 days — fix public in source before the advisory

Background

Save Page As / `SaveFileManager`
the browser-process machinery that writes a page and its parts to disk as SaveFile items. It runs on the download task runner and tracks each item by SaveItemId.
`SaveFileCreateInfo::SaveFileSource`
records where a saved item’s bytes come from. SAVE_FILE_FROM_NET items are fetched by a SimpleURLLoaderHelper. Other sources, handled by the else branch, are started “manually” because their data does not come from a network loader; the diff’s comment says the data “will come from render process”.
Quarantine / source annotation
SaveFile::AnnotateWithSourceInformation, reached through a quarantine::mojom::Quarantine remote, tags a saved file with its source URL and referrer. This is Mark-of-the-Web style metadata that OS and enterprise (DLP) protections use. It completes in SaveFileManager::OnQuarantineComplete.
`SaveFinished` vs `SaveCompleted`
after the patch, SaveFinished means “bytes are written, now quarantine”, and SaveCompleted (the old SaveFinished body) means “report the result to the UI thread via OnSaveCompleted”.

Root Cause Analysis

Before the patch, quarantine was attached only to the network fetch path. In the save-registration code, remote_quarantine, url, referrer.url and client_guid were bound into save_finished_cb (SaveFileManager::OnURLLoaderComplete) inside the if (save_source == SaveFileCreateInfo::SAVE_FILE_FROM_NET) branch. Only OnURLLoaderComplete called save_file->AnnotateWithSourceInformation. Items started in the else branch (// We manually start the save job.) carried no quarantine information. When they finished, they went straight to the old SaveFinished, which posted OnSaveFinished to the UI thread, so in this code path these files were written to disk without source annotation. The broken rule is that every file Save Page writes should be quarantined before it is reported as complete, whatever its SaveFileSource.

The fix builds a single quarantine_callback (SaveFileManager::QuarantineItem, with URLs blanked when IsOffTheRecord()) before the branch. It stores the callback in SaveFileCreateInfo::quarantine_callback on both branches; on the network branch the callback goes through SimpleURLLoaderHelper and is placed into info there. SaveFinished now runs SaveFile::RunQuarantineCallback() on success. QuarantineItem → OnQuarantineComplete → SaveCompleted then finishes the item, so all sources share one quarantine step.

Key insight
Quarantine was wired into the network loader’s completion callback, not into the save item, so saved files that did not come from SAVE_FILE_FROM_NET skipped source annotation. The fix stores the quarantine closure on each item’s SaveFileCreateInfo and runs it from the common SaveFinished path.

Attack Path

  1. Victim saves an attacker page a user saves an attacker-controlled page with Save Page As, for example SAVE_PAGE_TYPE_AS_COMPLETE_HTML as in the updated SaveDlp-style browser test.
  2. Item uses a non-network source part of the save, such as content supplied by the renderer, goes through the else branch, which before the patch got no remote_quarantine binding.
  3. File written without annotation on completion, the old SaveFinished reported success directly, so AnnotateWithSourceInformation never ran and the file had no source URL or referrer metadata.
  4. Downstream protections skipped OS reputation and Mark-of-the-Web checks, or ChromeOS DLP source-URL rules, would treat the saved file as locally created rather than downloaded from the attacker’s origin (inferred; the patch only shows that annotation was missing).

Impact Assessment

An attacker who gets a user to save a crafted page can cause files from that page to land on disk without the quarantine or source-URL metadata Chrome normally applies to downloads. This weakens the OS and DLP protections that rely on that metadata. It is a protection-bypass logic bug in the browser process and needs user interaction; it is not memory corruption. The vendor rates it medium (“Inappropriate implementation in Downloads”), with no in-the-wild exploitation reported. Any code execution that follows from running such a file is (inferred) and depends on further user action.

Changed Functions

FunctionChangeNotes
if
content/browser/download/save_file_manager.cc
modified
save_source
content/browser/download/save_types.cc
modified
for
chrome/browser/download/save_page_browsertest.cc
modified
IN_PROC_BROWSER_TEST_F
chrome/browser/download/save_page_browsertest.cc
modified

Files Changed

  • chrome/browser/download/save_page_browsertest.cc
  • content/browser/download/save_file.cc
  • content/browser/download/save_file.h
  • content/browser/download/save_file_manager.cc
  • content/browser/download/save_file_manager.h
  • content/browser/download/save_types.cc
  • content/browser/download/save_types.h

Audit Directions

  • Security steps bound to one source branch
    look for quarantine, Safe Browsing or DLP hooks attached to one producer (network loader) instead of the shared sink (file completion). Examples include other save and download sources, MHTML, PDF save, FileSystemAccess writes, and drag-out downloads.
  • Completion paths that skip post-processing
    for each item state machine, check that every success path, including failure-then-retry and renderer-driven completion, goes through annotation before “complete” is reported to the UI.
  • Off-the-record source data
    confirm that blanking url and referrer.url for IsOffTheRecord() still produces a quarantine mark, and that the source information is not dropped entirely.
diff --git a/content/browser/download/save_file.cc b/content/browser/download/save_file.cc
index 6eed86d5..89dfc43 100644
--- a/content/browser/download/save_file.cc
+++ b/content/browser/download/save_file.cc
@@ -91,4 +91,10 @@
   return file_.DebugString();
 }
 
+void SaveFile::RunQuarantineCallback() {
+  if (!info_->quarantine_callback.is_null()) {
+    std::move(info_->quarantine_callback).Run();
+  }
+}
+
 }  // namespace content
diff --git a/content/browser/download/save_file.h b/content/browser/download/save_file.h
index 77dde846..71497ef 100644
--- a/content/browser/download/save_file.h
+++ b/content/browser/download/save_file.h
@@ -56,6 +56,8 @@
   }
   const SaveFileCreateInfo& create_info() const { return *info_; }
 
+  void RunQuarantineCallback();
+
  private:
   download::BaseFile file_;
   std::unique_ptr<SaveFileCreateInfo> info_;
diff --git a/content/browser/download/save_file_manager.cc b/content/browser/download/save_file_manager.cc
index 41f27959..5e1057c 100644
--- a/content/browser/download/save_file_manager.cc
+++ b/content/browser/download/save_file_manager.cc
@@ -71,11 +71,13 @@
       const net::NetworkTrafficAnnotationTag& annotation_tag,
       network::mojom::URLLoaderFactory* url_loader_factory,
       SaveFileManager* save_file_manager,
+      base::OnceClosure quarantine_callback,
       URLLoaderCompleteCallback on_complete_cb) {
     return std::unique_ptr<SimpleURLLoaderHelper>(new SimpleURLLoaderHelper(
         std::move(resource_request), save_item_id, save_package_id,
         render_process_id, render_frame_routing_id, annotation_tag,
-        url_loader_factory, save_file_manager, std::move(on_complete_cb)));
+        url_loader_factory, save_file_manager, std::move(quarantine_callback),
+        std::move(on_complete_cb)));
   }
 
   SimpleURLLoaderHelper(const SimpleURLLoaderHelper&) = delete;
@@ -93,10 +95,12 @@
       const net::NetworkTrafficAnnotationTag& annotation_tag,
       network::mojom::URLLoaderFactory* url_loader_factory,
       SaveFileManager* save_file_manager,
+      base::OnceClosure quarantine_callback,
       URLLoaderCompleteCallback on_complete_cb)
       : save_file_manager_(save_file_manager),
         save_item_id_(save_item_id),
         save_package_id_(save_package_id),
+        quarantine_callback_(std::move(quarantine_callback)),
         on_complete_cb_(std::move(on_complete_cb)) {
     GURL url = resource_request->url;
     url_loader_ = network::SimpleURLLoader::Create(std::move(resource_request),
@@ -124,6 +128,7 @@
     auto info = std::make_unique<SaveFileCreateInfo>(
         url, final_url, save_item_id_, save_package_id_, render_process_id,
         render_frame_routing_id, content_disposition);
+    info->quarantine_callback = std::move(quarantine_callback_);
     download::GetDownloadTaskRunner()->PostTask(
         FROM_HERE, base::BindOnce(&SaveFileManager::StartSave,
                                   save_file_manager_, std::move(info)));
@@ -155,6 +160,7 @@
   SaveItemId save_item_id_;
   SavePackageId save_package_id_;
   std::unique_ptr<network::SimpleURLLoader> url_loader_;
+  base::OnceClosure quarantine_callback_;
   URLLoaderCompleteCallback on_complete_cb_;
 };
 
@@ -227,6 +233,12 @@
   DCHECK(!base::Contains(packages_, save_item_id));
   packages_[save_item_id] = save_package;
 
+  base::OnceClosure quarantine_callback = base::BindOnce(
+      &SaveFileManager::QuarantineItem, this, save_item_id, save_package->id(),
+      context->IsOffTheRecord() ? GURL() : url,
+      context->IsOffTheRecord() ? GURL() : referrer.url, client_guid,
+      std::move(remote_quarantine));
+
   // Register a saving job.
   if (save_source == SaveFileCreateInfo::SAVE_FILE_FROM_NET) {
     DCHECK(url.is_valid());
@@ -317,22 +329,20 @@
     }
 
     base::OnceCallback<void(bool /*success*/)> save_finished_cb =
-        base::BindOnce(&SaveFileManager::OnURLLoaderComplete, this,
-                       save_item_id, save_package->id(),
-                       context->IsOffTheRecord() ? GURL() : url,
-                       context->IsOffTheRecord() ? GURL() : referrer.url,
-                       client_guid, std::move(remote_quarantine));
-
+        base::BindOnce(&SaveFileManager::SaveFinished, this, save_item_id,
+                       save_package->id());
     url_loader_helpers_[save_item_id] =
         SimpleURLLoaderHelper::CreateAndStartDownload(
             std::move(request), save_item_id, save_package->id(),
             render_process_host_id, render_frame_routing_id, traffic_annotation,
-            factory, this, std::move(save_finished_cb));
+            factory, this, std::move(quarantine_callback),
+            std::move(save_finished_cb));
   } else {
     // We manually start the save job.
     auto info = std::make_unique<SaveFileCreateInfo>(
         file_full_path, url, save_item_id, save_package->id(),
         render_process_host_id, render_frame_routing_id, save_source);
+    info->quarantine_callback = std::move(quarantine_callback);
 
     // Since the data will come from render process, so we need to start
     // this kind of save job by ourself.
@@ -382,20 +392,16 @@
       base::BindOnce(&SaveFileManager::CancelSave, this, save_item_id));
 }
 
-void SaveFileManager::OnURLLoaderComplete(
+void SaveFileManager::QuarantineItem(
     SaveItemId save_item_id,
     SavePackageId save_package_id,
     const GURL& url,
     const GURL& referrer_url,
     const std::string& client_guid,
-    mojo::PendingRemote<quarantine::mojom::Quarantine> remote_quarantine,
-    bool is_success) {
+    mojo::PendingRemote<quarantine::mojom::Quarantine> remote_quarantine) {
   DCHECK(download::GetDownloadTaskRunner()->RunsTasksInCurrentSequence());
   SaveFile* save_file = LookupSaveFile(save_item_id);
-  if (!is_success || !save_file) {
-    SaveFinished(save_item_id, save_package_id, is_success);
-    return;
-  }
+  CHECK(save_file);
 
   save_file->AnnotateWithSourceInformation(
       client_guid, url, referrer_url, std::move(remote_quarantine),
@@ -408,8 +414,8 @@
     SavePackageId save_package_id,
     download::DownloadInterruptReason result) {
   DCHECK(download::GetDownloadTaskRunner()->RunsTasksInCurrentSequence());
-  SaveFinished(save_item_id, save_package_id,
-               result == download::DOWNLOAD_INTERRUPT_REASON_NONE);
+  SaveCompleted(save_item_id, save_package_id,
+                result == download::DOWNLOAD_INTERRUPT_REASON_NONE);
 }
 
 // Notifications sent from the IO thread and run on the file thread:
@@ -457,11 +463,23 @@
   }
 }
 
-// The IO thread will call this when saving is completed or it got error when
-// fetching data. We forward the message to OnSaveFinished in UI thread.
 void SaveFileManager::SaveFinished(SaveItemId save_item_id,
                                    SavePackageId save_package_id,
                                    bool is_success) {
+  DCHECK(download::GetDownloadTaskRunner()->RunsTasksInCurrentSequence());
+
+  SaveFile* save_file = LookupSaveFile(save_item_id);
+  if (!is_success || !save_file) {
+    SaveCompleted(save_item_id, save_package_id, is_success);
+    return;
+  }
+
+  save_file->RunQuarantineCallback();
+}
+
+void SaveFileManager::SaveCompleted(SaveItemId save_item_id,
+                                    SavePackageId save_package_id,
+                                    bool is_success) {
   DVLOG(20) << __func__ << "() save_item_id = " << save_item_id
             << " save_package_id = " << save_package_id
             << " is_success = " << is_success;
@@ -481,12 +499,11 @@
   }
 
   GetUIThreadTaskRunner({})->PostTask(
-      FROM_HERE, base::BindOnce(&SaveFileManager::OnSaveFinished, this,
+      FROM_HERE, base::BindOnce(&SaveFileManager::OnSaveCompleted, this,
                                 save_item_id, bytes_so_far, is_success));
 }
 
 // Notifications sent from the file thread and run on the UI thread.
-
 void SaveFileManager::OnStartSave(const SaveFileCreateInfo& info) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   SavePackage* save_package = GetSavePackageFromRenderIds(
@@ -512,9 +529,9 @@
     SendCancelRequest(save_item_id);
 }
 
-void SaveFileManager::OnSaveFinished(SaveItemId save_item_id,
-                                     int64_t bytes_so_far,
-                                     bool is_success) {
+void SaveFileManager::OnSaveCompleted(SaveItemId save_item_id,
+                                      int64_t bytes_so_far,
+                                      bool is_success) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   ClearURLLoader(save_item_id);
   SavePackage* package = LookupPackage(save_item_id);
diff --git a/content/browser/download/save_file_manager.h b/content/browser/download/save_file_manager.h
index 9b6e7be2..8ea30f6 100644
--- a/content/browser/download/save_file_manager.h
+++ b/content/browser/download/save_file_manager.h
@@ -119,9 +119,18 @@
   // Notifications sent from the IO thread and run on the file thread:
   void StartSave(std::unique_ptr<SaveFileCreateInfo> info);
   void UpdateSaveProgress(SaveItemId save_item_id, const std::string& data);
+
+  // Called on the download TaskRunner when all the data is saved or the
+  // download failed. The saved item will go through quarantine before it is
+  // considered comple.
   void SaveFinished(SaveItemId save_item_id,
                     SavePackageId save_package_id,
                     bool is_success);
+  // The file saving is completed. We forward the message to OnSaveFinished in
+  // UI thread.
+  void SaveCompleted(SaveItemId save_item_id,
+                     SavePackageId save_package_id,
+                     bool is_success);
 
   // Notifications sent from the UI thread and run on the file thread.
   // Cancel a SaveFile instance which has specified save item id.
@@ -181,17 +190,17 @@
   // Help function for sending notification of canceling specific request.
   void SendCancelRequest(SaveItemId save_item_id);
 
-  // Called on the file thread when the URLLoader completes saving a SaveItem.
-  void OnURLLoaderComplete(
+  // Called on the download TaskRunner to start quarantine a saved file.
+  void QuarantineItem(
       SaveItemId save_item_id,
       SavePackageId save_package_id,
       const GURL& url,
       const GURL& referrer_url,
       const std::string& client_guid,
-      mojo::PendingRemote<quarantine::mojom::Quarantine> remote_quarantine,
-      bool is_success);
+      mojo::PendingRemote<quarantine::mojom::Quarantine> remote_quarantine);
 
-  // Called on the file thread when file quarantine finishes on a SaveItem.
+  // Called on the download TaskRunner when file quarantine finishes on a
+  // SaveItem.
   void OnQuarantineComplete(SaveItemId save_item_id,
                             SavePackageId save_package_id,
                             download::DownloadInterruptReason result);
@@ -208,9 +217,9 @@
                             bool write_success);
   // Update the SavePackage with the finish state, and remove the request
   // tracking entries.
-  void OnSaveFinished(SaveItemId save_item_id,
-                      int64_t bytes_so_far,
-                      bool is_success);
+  void OnSaveCompleted(SaveItemId save_item_id,
+                       int64_t bytes_so_far,
+                       bool is_success);
   // Notifies SavePackage that the whole page saving job is finished.
   void OnFinishSavePageJob(int render_process_id,
                            int render_frame_routing_id,
diff --git a/content/browser/download/save_types.cc b/content/browser/download/save_types.cc
index 0d93a8f..1fdcb7a8 100644
--- a/content/browser/download/save_types.cc
+++ b/content/browser/download/save_types.cc
@@ -39,8 +39,18 @@
       content_disposition(content_disposition),
       save_source(SaveFileCreateInfo::SAVE_FILE_FROM_NET) {}
 
-SaveFileCreateInfo::SaveFileCreateInfo(const SaveFileCreateInfo& other) =
-    default;
+SaveFileCreateInfo::SaveFileCreateInfo(const SaveFileCreateInfo& other)
+    : path(other.path),
+      url(other.url),
+      final_url(other.final_url),
+      save_item_id(other.save_item_id),
+      save_package_id(other.save_package_id),
+      render_process_id(other.render_process_id),
+      render_frame_routing_id(other.render_frame_routing_id),
+      content_disposition(other.content_disposition),
+      save_source(other.save_source) {
+  // The quarantine_callback is intentionally not copied.
+}
 
 SaveFileCreateInfo::~SaveFileCreateInfo() {}
 
diff --git a/content/browser/download/save_types.h b/content/browser/download/save_types.h
index da246fc..d1ce26c0 100644
--- a/content/browser/download/save_types.h
+++ b/content/browser/download/save_types.h
@@ -13,6 +13,7 @@
 #include <vector>
 
 #include "base/files/file_path.h"
+#include "base/functional/callback.h"
 #include "base/types/id_type.h"
 #include "url/gurl.h"
 
@@ -80,6 +81,8 @@
   std::string content_disposition;
   // Source type of saved file.
   SaveFileSource save_source;
+  // Callback to run to quarantine the file;
+  base::OnceClosure quarantine_callback;
 };
 
 }  // namespace content
diff --git a/chrome/browser/download/save_page_browsertest.cc b/chrome/browser/download/save_page_browsertest.cc
index a2385a2..e038e24 100644
--- a/chrome/browser/download/save_page_browsertest.cc
+++ b/chrome/browser/download/save_page_browsertest.cc
@@ -69,6 +69,7 @@
 #include "ui/shell_dialogs/fake_select_file_dialog.h"
 
 #if BUILDFLAG(IS_CHROMEOS)
+#include "base/test/test_future.h"
 #include "chromeos/dbus/dlp/dlp_client.h"
 #endif  // BUILDFLAG(IS_CHROMEOS)
 
@@ -1712,27 +1713,40 @@
 
   chromeos::DlpClient::Shutdown();
   chromeos::DlpClient::InitializeFake();
-  base::test::RepeatingTestFuture<
-      dlp::AddFilesRequest, base::OnceCallback<void(dlp::AddFilesResponse)>>
-      add_file_cb;
-  chromeos::DlpClient::Get()->GetTestInterface()->SetAddFilesMock(
-      add_file_cb.GetCallback());
 
-  url = NavigateToMockURL("a");
+  // Use page "b" which has subresources (1.png, 1.css). Total 3 files.
+  url = NavigateToMockURL("b");
 
-  SaveCurrentTab(url, content::SAVE_PAGE_TYPE_AS_COMPLETE_HTML, "a", 1, &dir,
+  SaveCurrentTab(url, content::SAVE_PAGE_TYPE_AS_COMPLETE_HTML, "b", 3, &dir,
                  &full_file_name);
 
   ASSERT_FALSE(HasFailure());
 
-  auto request = std::get<0>(add_file_cb.Take());
-  ASSERT_EQ(1, request.add_file_requests().size());
-  EXPECT_EQ(full_file_name.value(), request.add_file_requests(0).file_path());
-  EXPECT_EQ(request.add_file_requests(0).source_url(), url.spec());
+  // Asynchronously get the recorded requests from the fake client.
+  base::test::TestFuture<const dlp::GetDatabaseEntriesResponse> future;
+  chromeos::DlpClient::Get()->GetDatabaseEntries(future.GetCallback());
+  const auto& response = future.Get();
+  const auto& requests = response.files_entries();
 
-  base::ScopedAllowBlockingForTesting allow_blocking;
-  EXPECT_TRUE(base::PathExists(full_file_name));
-  EXPECT_FALSE(base::PathExists(dir));
+  // There is a total of 6 requests, 3 for temporary files and 3 for final
+  // destination.
+  ASSERT_EQ(6, requests.size());
+
+  // The order of subresource saving is not guaranteed, so we use a set
+  // to verify the presence of each expected file path.
+  std::set<std::string> expected_paths;
+  expected_paths.insert(full_file_name.value());
+  expected_paths.insert(dir.AppendASCII("1.png").value());
+  expected_paths.insert(dir.AppendASCII("1.css").value());
+
+  std::set<std::string> actual_paths;
+  for (const auto& request : requests) {
+    actual_paths.insert(request.path());
+  }
+
+  for (const auto& expected_path : expected_paths) {
+    EXPECT_TRUE(base::Contains(actual_paths, expected_path));
+  }
 }
 
 IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveMHTMLWithDlp) {
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/chrome/browser/download/save_page_browsertest.cc b/chrome/browser/download/save_page_browsertest.cc
index a2385a2..e038e24 100644
--- a/chrome/browser/download/save_page_browsertest.cc
+++ b/chrome/browser/download/save_page_browsertest.cc
@@ -69,6 +69,7 @@
 #include "ui/shell_dialogs/fake_select_file_dialog.h"
 
 #if BUILDFLAG(IS_CHROMEOS)
+#include "base/test/test_future.h"
 #include "chromeos/dbus/dlp/dlp_client.h"
 #endif  // BUILDFLAG(IS_CHROMEOS)
 
@@ -1712,27 +1713,40 @@
 
   chromeos::DlpClient::Shutdown();
   chromeos::DlpClient::InitializeFake();
-  base::test::RepeatingTestFuture<
-      dlp::AddFilesRequest, base::OnceCallback<void(dlp::AddFilesResponse)>>
-      add_file_cb;
-  chromeos::DlpClient::Get()->GetTestInterface()->SetAddFilesMock(
-      add_file_cb.GetCallback());
 
-  url = NavigateToMockURL("a");
+  // Use page "b" which has subresources (1.png, 1.css). Total 3 files.
+  url = NavigateToMockURL("b");
 
-  SaveCurrentTab(url, content::SAVE_PAGE_TYPE_AS_COMPLETE_HTML, "a", 1, &dir,
+  SaveCurrentTab(url, content::SAVE_PAGE_TYPE_AS_COMPLETE_HTML, "b", 3, &dir,
                  &full_file_name);
 
   ASSERT_FALSE(HasFailure());
 
-  auto request = std::get<0>(add_file_cb.Take());
-  ASSERT_EQ(1, request.add_file_requests().size());
-  EXPECT_EQ(full_file_name.value(), request.add_file_requests(0).file_path());
-  EXPECT_EQ(request.add_file_requests(0).source_url(), url.spec());
+  // Asynchronously get the recorded requests from the fake client.
+  base::test::TestFuture<const dlp::GetDatabaseEntriesResponse> future;
+  chromeos::DlpClient::Get()->GetDatabaseEntries(future.GetCallback());
+  const auto& response = future.Get();
+  const auto& requests = response.files_entries();
 
-  base::ScopedAllowBlockingForTesting allow_blocking;
-  EXPECT_TRUE(base::PathExists(full_file_name));
-  EXPECT_FALSE(base::PathExists(dir));
+  // There is a total of 6 requests, 3 for temporary files and 3 for final
+  // destination.
+  ASSERT_EQ(6, requests.size());
+
+  // The order of subresource saving is not guaranteed, so we use a set
+  // to verify the presence of each expected file path.
+  std::set<std::string> expected_paths;
+  expected_paths.insert(full_file_name.value());
+  expected_paths.insert(dir.AppendASCII("1.png").value());
+  expected_paths.insert(dir.AppendASCII("1.css").value());
+
+  std::set<std::string> actual_paths;
+  for (const auto& request : requests) {
+    actual_paths.insert(request.path());
+  }
+
+  for (const auto& expected_path : expected_paths) {
+    EXPECT_TRUE(base::Contains(actual_paths, expected_path));
+  }
 }
 
 IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveMHTMLWithDlp) {
Loading diff…

Original Bug Report

reported by [email protected]

Zone.Identifier MotW is missing for files saved with Ctrl+S

Steps to reproduce the problem

  1. Visit e.g. https://bayden.com/echo.aspx
  2. Hit CTRL+S to save the file
  3. Save the file using “Web Page, Complete” format.

OBSERVE: File does not have a Mark-of-the-Web alternate data stream.

Problem Description

The lack of a Zone.identifier based MotW was documented by https://mrd0x.com/filefix-part-2/ in an attack chain that involves tricking the user into saving a HTA file.

This appears to be a regression since https://issues.chromium.org/issues/40082670#comment6

If the user chooses “Web Page, HTML Only” the Zone.Identifier MotW is attached correctly. It’s only “Web Page, Complete” that doesn’t attach one.

Summary

Zone.Identifier MotW is missing for files saved with Ctrl+S

Additional Data

Category: Security
Chrome Channel: Not sure
Regression: Yes \

View on issue tracker