Chrome · Downloads
CVE-2026-84358
Logic Error in Downloads
Overview
Medium
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Files Changed
content/browser/download/mhtml_generation_browsertest.cccontent/browser/download/mhtml_generation_manager.cc
Patch
From 68855c577a9bd5e3d4b6ea5fad41b04f1ee684f8 Mon Sep 17 00:00:00 2001
From: Min Qin <qinmin@chromium.org>
Date: Thu, 11 Jun 2026 00:01:46 -0700
Subject: [PATCH] Secure MHTML generation by introducing a Mojo Data Pipe on all platforms.
This CL secures the MHTML generation path by introducing a Mojo Data
Pipe on all platforms (including POSIX and Windows):
1. The browser process creates a Mojo Data Pipe, passing the Producer handle to the renderer and retaining the Consumer handle.
2. A new helper class, MHTMLDataPipeReader, runs on the blocking download task runner to read incoming serialized MHTML data from the Consumer handle and write it safely to disk in the browser process.
3. MHTMLHandleWriter and its subclasses are updated to use a self-destruction mechanism via std::unique_ptr (passing the unique_ptr through the asynchronous task/callback chain) to guarantee robust lifetime management during the Mojo streaming process. This completely avoids sequence lifetime concerns and potential memory leaks upon unexpected termination.
4. Legacy mojo::SimpleWatcher member and includes are removed from the browser process, as the asynchronous reading is now entirely driven by mojo::DataPipeDrainer.
Bug: 514006886
Change-Id: Icf51a14c5422e725f2a99e69ed964f166b788a6a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7883815
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1645142}
---
diff --git a/content/browser/download/mhtml_generation_browsertest.cc b/content/browser/download/mhtml_generation_browsertest.cc
index 66c6675..cfa602b 100644
--- a/content/browser/download/mhtml_generation_browsertest.cc
+++ b/content/browser/download/mhtml_generation_browsertest.cc
@@ -280,57 +280,60 @@
void SerializeAsMHTML(mojom::SerializeAsMHTMLParamsPtr params,
SerializeAsMHTMLCallback callback) override {
// Upon using the overridden mock interface implementation, this will be
- // handled by the product code as illustrated below. (1), (2), (3) depict
- // points in time when product code runs on UI thread and download sequence.
- // For the repro, the message pipe disconnection needs to happen between (1)
- // and (3).
+ // handled by the product code as illustrated below. (1), (2), (3), (4)
+ // depict points in time when product code runs on UI thread and download
+ // sequence. For the repro, the message pipe disconnection needs to happen
+ // after the browser has completed writing to disk (2) and before the job is
+ // destroyed in OnFinished (4).
//
- // Test instance UI thread download sequence
- // --------- --------- -----------
- // | | |
- // WE ARE HERE | |
- // | | |
- // | | |
- // +--------------->+ |
- // | | |
- // | | |
- // | | |
- // | | |
- // | | |
- // | | |
- // (1) | MHTMLGenerationManager::Job |
- // | ::SerializeAsMHTMLResponse |
- // | +-------------------->+
- // | | |
- // | | |
- // | | |
- // (2) | | MHTMLGenerationManager::Job
- // | | ::CloseFileOnFileThread
- // | | |
- // | | |
- // | test needs to |
- // | disconnect message pipe |
- // | HERE - between (1) and (3) |
- // | | |
- // | | |
- // | +<--------------------+
- // | | |
- // (3) | MHTMLGenerationManager |
- // | Job::OnFinished |
- // | | |
+ // Test instance UI thread download sequence
+ // --------- --------- -----------
+ // | | |
+ // WE ARE HERE | |
+ // | | |
+ // +---------------->+ |
+ // | | |
+ // | | |
+ // (1) | MHTMLGenerationManager::Job |
+ // | ::SerializeAsMHTMLResponse |
+ // | | |
+ // | | |
+ // | | [Streams MHTML data]
+ // | | +
+ // (2) | +<---------------------+
+ // | MHTMLGenerationManager::Job |
+ // | ::DoneWritingToDisk |
+ // | +-------------------->+
+ // | | |
+ // (3) | | MHTMLGenerationManager::Job
+ // | | ::FinalizeOnFileThread
+ // | | |
+ // | | |
+ // | test needs to |
+ // | disconnect message pipe |
+ // | HERE - between (2) and (4) |
+ // | | |
+ // | | |
+ // | +<---------------------+
+ // | | |
+ // (4) | MHTMLGenerationManager |
+ // | Job::OnFinished |
+ // | | |
//
- // We hope that the error handler is invoked between (1) and (3) by doing
+ // We ensure the disconnect handler is invoked between (2) and (4) by doing
// the following:
// - From here, run the callback response to the UI thread. This queues
// the response message onto the bound message pipe.
- // - After running the callback response, immediately unbind the message
- // pipe in order to queue a message onto the bound message pipe to notify
- // the Browser the connection was closed and invoke the error handler.
- // - Upon resuming operation, the FIFO ordering property of associated
- // interfaces guarantees the execution of (1) before the error handler.
- // (1) posts (2) to the download sequence and terminates. The client end
- // then accepts the error notification and invokes the connection error
- // handler, guaranteeing its execution before (3).
+ // - After running the callback response, we post a sequence of tasks to
+ // the download thread (DelayDisconnectionStep1 -> Step2 -> Step3 ->
+ // DisconnectMojoReceiver) to delay resetting the Mojo connection until
+ // after the data pipe streaming has completed and DoneWritingToDisk (2)
+ // has run on the UI thread.
+ // - Upon resuming operation, the FIFO ordering property of Mojo interfaces
+ // guarantees that (1) executes before the error handler. In parallel,
+ // the data pipe is drained, posting (2) which then posts (3). The client
+ // end then accepts the error notification and invokes the connection
+ // error handler, guaranteeing its execution before (4).
bool compute_contents_hash = params->output_handle->is_producer_handle();
@@ -359,31 +362,39 @@
// we must ensure the write complete notification arrives before the
// connection error notification, otherwise the Browser will report
// an MhtmlSaveStatus != kSuccess. We can guarantee this by potentially
- // running tasks after each watcher invocation to send notifications that
- // it has been completed. We need at least two tasks to guarantee this,
- // as there can be at most two watcher invocations to write a block of
- // data smaller than the data pipe buffer to file.
+ // running tasks sequentially on the download thread to delay the Mojo
+ // connection reset. We need three task hops on the download sequence to
+ // ensure that the reader has completed reading the EOF, and the completion
+ // task has been posted to the UI thread (DoneWritingToDisk) before the
+ // receiver is reset.
download::GetDownloadTaskRunner()->PostTask(
FROM_HERE,
- base::BindOnce(&RespondAndDisconnectMockWriter::TaskX,
+ base::BindOnce(&RespondAndDisconnectMockWriter::DelayDisconnectionStep1,
scoped_refptr<RespondAndDisconnectMockWriter>(this)));
}
- void TaskX() {
+ void DelayDisconnectionStep1() {
download::GetDownloadTaskRunner()->PostTask(
FROM_HERE,
- base::BindOnce(&RespondAndDisconnectMockWriter::TaskY,
+ base::BindOnce(&RespondAndDisconnectMockWriter::DelayDisconnectionStep2,
scoped_refptr<RespondAndDisconnectMockWriter>(this)));
}
- void TaskY() {
+ void DelayDisconnectionStep2() {
+ download::GetDownloadTaskRunner()->PostTask(
+ FROM_HERE,
+ base::BindOnce(&RespondAndDisconnectMockWriter::DelayDisconnectionStep3,
+ scoped_refptr<RespondAndDisconnectMockWriter>(this)));
+ }
+
+ void DelayDisconnectionStep3() {
GetUIThreadTaskRunner({})->PostTask(
FROM_HERE,
- base::BindOnce(&RespondAndDisconnectMockWriter::TaskZ,
+ base::BindOnce(&RespondAndDisconnectMockWriter::DisconnectMojoReceiver,
scoped_refptr<RespondAndDisconnectMockWriter>(this)));
}
- void TaskZ() { receiver_.reset(); }
+ void DisconnectMojoReceiver() { receiver_.reset(); }
private:
friend base::RefCountedThreadSafe<RespondAndDisconnectMockWriter>;
diff --git a/content/browser/download/mhtml_generation_manager.cc b/content/browser/download/mhtml_generation_manager.cc
index 511dd0f4..e089638 100644
--- a/content/browser/download/mhtml_generation_manager.cc
+++ b/content/browser/download/mhtml_generation_manager.cc
@@ -18,6 +18,8 @@
#include "base/strings/string_util.h"
#include "base/strings/string_view_util.h"
#include "base/strings/stringprintf.h"
+#include "base/task/sequenced_task_runner.h"
+#include "base/threading/sequence_bound.h"
#include "base/time/time.h"
#include "base/trace_event/trace_event.h"
#include "base/types/optional_util.h"
@@ -37,6 +39,8 @@
#include "content/public/common/mhtml_generation_params.h"
#include "mojo/core/embedder/embedder.h"
#include "mojo/public/cpp/bindings/associated_remote.h"
+#include "mojo/public/cpp/system/data_pipe.h"
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/content/browser/download/mhtml_generation_browsertest.cc b/content/browser/download/mhtml_generation_browsertest.cc
index 66c6675..cfa602b 100644
--- a/content/browser/download/mhtml_generation_browsertest.cc
+++ b/content/browser/download/mhtml_generation_browsertest.cc
@@ -280,57 +280,60 @@
void SerializeAsMHTML(mojom::SerializeAsMHTMLParamsPtr params,
SerializeAsMHTMLCallback callback) override {
// Upon using the overridden mock interface implementation, this will be
- // handled by the product code as illustrated below. (1), (2), (3) depict
- // points in time when product code runs on UI thread and download sequence.
- // For the repro, the message pipe disconnection needs to happen between (1)
- // and (3).
+ // handled by the product code as illustrated below. (1), (2), (3), (4)
+ // depict points in time when product code runs on UI thread and download
+ // sequence. For the repro, the message pipe disconnection needs to happen
+ // after the browser has completed writing to disk (2) and before the job is
+ // destroyed in OnFinished (4).
//
- // Test instance UI thread download sequence
- // --------- --------- -----------
- // | | |
- // WE ARE HERE | |
- // | | |
- // | | |
- // +--------------->+ |
- // | | |
- // | | |
- // | | |
- // | | |
- // | | |
- // | | |
- // (1) | MHTMLGenerationManager::Job |
- // | ::SerializeAsMHTMLResponse |
- // | +-------------------->+
- // | | |
- // | | |
- // | | |
- // (2) | | MHTMLGenerationManager::Job
- // | | ::CloseFileOnFileThread
- // | | |
- // | | |
- // | test needs to |
- // | disconnect message pipe |
- // | HERE - between (1) and (3) |
- // | | |
- // | | |
- // | +<--------------------+
- // | | |
- // (3) | MHTMLGenerationManager |
- // | Job::OnFinished |
- // | | |
+ // Test instance UI thread download sequence
+ // --------- --------- -----------
+ // | | |
+ // WE ARE HERE | |
+ // | | |
+ // +---------------->+ |
+ // | | |
+ // | | |
+ // (1) | MHTMLGenerationManager::Job |
+ // | ::SerializeAsMHTMLResponse |
+ // | | |
+ // | | |
+ // | | [Streams MHTML data]
+ // | | +
+ // (2) | +<---------------------+
+ // | MHTMLGenerationManager::Job |
+ // | ::DoneWritingToDisk |
+ // | +-------------------->+
+ // | | |
+ // (3) | | MHTMLGenerationManager::Job
+ // | | ::FinalizeOnFileThread
+ // | | |
+ // | | |
+ // | test needs to |
+ // | disconnect message pipe |
+ // | HERE - between (2) and (4) |
+ // | | |
+ // | | |
+ // | +<---------------------+
+ // | | |
+ // (4) | MHTMLGenerationManager |
+ // | Job::OnFinished |
+ // | | |
//
- // We hope that the error handler is invoked between (1) and (3) by doing
+ // We ensure the disconnect handler is invoked between (2) and (4) by doing
// the following:
// - From here, run the callback response to the UI thread. This queues
// the response message onto the bound message pipe.
- // - After running the callback response, immediately unbind the message
- // pipe in order to queue a message onto the bound message pipe to notify
- // the Browser the connection was closed and invoke the error handler.
- // - Upon resuming operation, the FIFO ordering property of associated
- // interfaces guarantees the execution of (1) before the error handler.
- // (1) posts (2) to the download sequence and terminates. The client end
- // then accepts the error notification and invokes the connection error
- // handler, guaranteeing its execution before (3).
+ // - After running the callback response, we post a sequence of tasks to
+ // the download thread (DelayDisconnectionStep1 -> Step2 -> Step3 ->
+ // DisconnectMojoReceiver) to delay resetting the Mojo connection until
+ // after the data pipe streaming has completed and DoneWritingToDisk (2)
+ // has run on the UI thread.
+ // - Upon resuming operation, the FIFO ordering property of Mojo interfaces
+ // guarantees that (1) executes before the error handler. In parallel,
+ // the data pipe is drained, posting (2) which then posts (3). The client
+ // end then accepts the error notification and invokes the connection
+ // error handler, guaranteeing its execution before (4).
bool compute_contents_hash = params->output_handle->is_producer_handle();
@@ -359,31 +362,39 @@
// we must ensure the write complete notification arrives before the
// connection error notification, otherwise the Browser will report
// an MhtmlSaveStatus != kSuccess. We can guarantee this by potentially
- // running tasks after each watcher invocation to send notifications that
- // it has been completed. We need at least two tasks to guarantee this,
- // as there can be at most two watcher invocations to write a block of
- // data smaller than the data pipe buffer to file.
+ // running tasks sequentially on the download thread to delay the Mojo
+ // connection reset. We need three task hops on the download sequence to
+ // ensure that the reader has completed reading the EOF, and the completion
+ // task has been posted to the UI thread (DoneWritingToDisk) before the
+ // receiver is reset.
download::GetDownloadTaskRunner()->PostTask(
FROM_HERE,
- base::BindOnce(&RespondAndDisconnectMockWriter::TaskX,
+ base::BindOnce(&RespondAndDisconnectMockWriter::DelayDisconnectionStep1,
scoped_refptr<RespondAndDisconnectMockWriter>(this)));
}
- void TaskX() {
+ void DelayDisconnectionStep1() {
download::GetDownloadTaskRunner()->PostTask(
FROM_HERE,
- base::BindOnce(&RespondAndDisconnectMockWriter::TaskY,
+ base::BindOnce(&RespondAndDisconnectMockWriter::DelayDisconnectionStep2,
scoped_refptr<RespondAndDisconnectMockWriter>(this)));
}
- void TaskY() {
+ void DelayDisconnectionStep2() {
+ download::GetDownloadTaskRunner()->PostTask(
+ FROM_HERE,
+ base::BindOnce(&RespondAndDisconnectMockWriter::DelayDisconnectionStep3,
+ scoped_refptr<RespondAndDisconnectMockWriter>(this)));
+ }
+
+ void DelayDisconnectionStep3() {
GetUIThreadTaskRunner({})->PostTask(
FROM_HERE,
- base::BindOnce(&RespondAndDisconnectMockWriter::TaskZ,
+ base::BindOnce(&RespondAndDisconnectMockWriter::DisconnectMojoReceiver,
scoped_refptr<RespondAndDisconnectMockWriter>(this)));
}
- void TaskZ() { receiver_.reset(); }
+ void DisconnectMojoReceiver() { receiver_.reset(); }
private:
friend base::RefCountedThreadSafe<RespondAndDisconnectMockWriter>;
Loading diff…
Original Bug Report
reported by vm...@google.com
Cross-origin MHTML overwrite and URL spoofing on POSIX due to shared O_APPEND flag
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: MHTML generation on POSIX platforms shares an Open File Description (OFD) between the browser and multiple renderers. A compromised renderer can potentially clear the shared O_APPEND flag via fcntl(F_SETFL), allowing it to overwrite data from other origins and spoof the final archive. This is particularly impactful on Android Offline Pages, where internal archives skip integrity checks, leading to persistent Omnibox URL spoofing.
Affected files:
content/browser/download/mhtml_generation_manager.ccbase/files/file_posix.ccsandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions.ccchrome/browser/offline_pages/offline_page_mhtml_archiver.ccchrome/browser/offline_pages/offline_page_request_handler.cc
Estimated timestamp from git blame: 2026-04-01
Summary
On POSIX platforms (Linux, Android, macOS), the O_APPEND flag on an open file description (OFD) is shared between all file descriptors referring to it. In Chrome’s MHTML generation, the browser process opens an output file with O_APPEND and then provides each renderer with a duplicated file descriptor via dup(). Because these FDs share the same OFD, a compromised renderer can clear the O_APPEND flag for the entire OFD using fcntl(F_SETFL, ...). This allows the renderer to use lseek and write to overwrite content written by other origins during the shared archival process.
Root Cause Analysis
In content/browser/download/mhtml_generation_manager.cc, the browser opens the MHTML output file using base::File::FLAG_APPEND. On POSIX, this results in the O_APPEND status flag being set on the Open File Description.
When the browser calls browser_file_.Duplicate() to send the handle to a renderer, it calls dup(), creating a new FD pointing to the same OFD. On Linux and Android, the renderer’s seccomp-bpf sandbox permits the modification of O_APPEND via fcntl:
// sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions.cc
const uint64_t kAllowedMask = O_ACCMODE | O_APPEND | O_NONBLOCK | O_SYNC | ...;
// ...
.Case(F_SETFL,
If((long_arg & ~kAllowedMask) == 0, Allow()).Else(CrashSIGSYS()))
A compromised renderer can potentially call fcntl(fd, F_SETFL, 0) to clear O_APPEND. It can then use lseek(fd, 0, SEEK_SET) to move the shared file offset to the beginning and overwrite the archive with malicious content. The Android renderer sandbox also permits ftruncate(), which can be used to erase subsequent legitimate data.
Potential Impact: Android Offline Pages URL Spoofing
This vulnerability is most critical for the Android Offline Pages feature. When a page is archived, the browser records the main frame’s URL and the file’s SHA256 digest in a local database. Because archives in internal directories are considered trusted, OfflinePageRequestHandler skips the integrity validation (digest check) when serving them later.
By overwriting the archive during the generation phase, a compromised renderer for a cross-origin subframe can inject malicious HTML that will be associated with the victim main frame’s URL. When the user later opens the offline page, Chrome will display the victim’s URL in the Omnibox while rendering the attacker’s content.
Suggested Reproductions Steps (Potential)
- Compromise a renderer process for a cross-origin iframe (e.g., via a V8 exploit).
- Trigger a page save (e.g., ‘Download’ in Android Chrome) for a site embedding that iframe.
- In the compromised renderer, wait for the MHTML file handle.
- Execute:
fcntl(fd, F_SETFL, 0); lseek(fd, 0, SEEK_SET); write(fd, malicious_data, len); ftruncate(fd, len);. - Wait for the browser to finalize the archive.
- Access the saved offline page to observe the URL spoof.
Suggested Fix
To prevent this escalation, the browser should avoid sharing the same Open File Description with multiple renderers if O_APPEND is the only intended protection. Possible solutions include:
- Platform-specific hardening: On Windows, the kernel enforces
FILE_APPEND_DATApermissions. On POSIX, consider using a pipe to receive data from renderers and having the browser perform the writes. - Verify file integrity: Ensure that even internal archives undergo mandatory digest validation before being served to the user in
OfflinePageRequestHandler.
Evaluated with Chrome root at commit: 1a8d40fc44df2088d5945c0bf53584038aa1614a
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.
References
On This Page