Overview

Low
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactInappropriate implementation in MHTML
DescriptionInappropriate implementation in MHTML
ComponentMHTML
Bug ClassLogic Error
Tracker496628298
Fix commitc85bc6c1ecbe (chromium/src) +50/-4
CISA KEVNot listed
CreditedGoogle
Disclosed2026-05-05

Changed Functions

FunctionChangeNotes
if
base/files/file_posix.cc
modified
TEST
base/files/file_unittest.cc
modified
if
base/files/file_win.cc
modified

Files Changed

  • base/files/file_posix.cc
  • base/files/file_unittest.cc
  • base/files/file_win.cc
  • content/browser/download/mhtml_generation_manager.cc
From c85bc6c1ecbe14065777a53ae53fd7629c7d5a0c Mon Sep 17 00:00:00 2001
From: Lukasz Anforowicz <lukasza@chromium.org>
Date: Wed, 01 Apr 2026 09:30:04 -0700
Subject: [PATCH] [mhtml] Use `FLAG_APPEND` instead of `FLAG_WRITE` in `CreateMHTMLFile`,

MHTML files are always written sequentially - there is no need to seek
within the file and therefore `FLAG_APPEND` can be used.

This CL also augments the `DCHECK`s that have been introduced in
https://codereview.chromium.org/314023002/, so that they also allow
`FLAG_APPEND`.  `FLAG_APPEND` also results in `O_RDWR` or `O_WRONLY`
[1], so the original concern from this 2014 CL is still addressed.

[1] https://source.chromium.org/chromium/chromium/src/+/main:base/files/file_posix.cc;l=604-608;drc=b3a0fa5691682590fcbab672868c0b34beec7052

Fixed: 496628298
Change-Id: I46f2723a1eb7f6be43c7c40a78358bd85ea53d09
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7708242
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1608595}
---

diff --git a/base/files/file_posix.cc b/base/files/file_posix.cc
index 9a19153a..f67c179f 100644
--- a/base/files/file_posix.cc
+++ b/base/files/file_posix.cc
@@ -571,7 +571,7 @@
 
   if (flags & FLAG_CREATE_ALWAYS) {
     DCHECK(!open_flags);
-    DCHECK(flags & FLAG_WRITE);
+    DCHECK(flags & (FLAG_WRITE | FLAG_APPEND));
     open_flags = O_CREAT | O_TRUNC;
   }
 
diff --git a/base/files/file_unittest.cc b/base/files/file_unittest.cc
index 8998864..5a327c5 100644
--- a/base/files/file_unittest.cc
+++ b/base/files/file_unittest.cc
@@ -454,6 +454,49 @@
   }
 }
 
+// Test whether `FLAG_APPEND` really works - i.e. that Chromium's
+// platform-specific code correctly uses the APIs provided by the OS for locking
+// down the capabilities of a file handle.
+TEST(FileTest, AppendEffectiveness) {
+  ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+  FilePath file_path = temp_dir.GetPath().AppendASCII("append_file_cant_seek");
+  File file(file_path, File::FLAG_CREATE | File::FLAG_APPEND);
+  ASSERT_TRUE(file.IsValid());
+
+  // Write "test" to the file.
+  const std::string_view kTestData = "test";
+  std::optional<size_t> bytes_written = file.Write(0, as_byte_span(kTestData));
+  EXPECT_TRUE(bytes_written.has_value());
+  EXPECT_EQ(kTestData.size(), bytes_written.value());
+
+  // Attempt to `Seek` should fail because of `FLAG_APPEND` used above.
+  //
+  // We ignore the returned value because it is irrelevant - the real
+  // verification is whether the seek actually happened which is
+  // verified via `ReadFileToString` below.
+  std::ignore = file.Seek(File::FROM_BEGIN, /*offset=*/0);
+
+  // Write "foo" to the file.
+  const std::string_view kFooData = "foo";
+  bytes_written = file.WriteAtCurrentPos(as_byte_span(kFooData));
+  EXPECT_TRUE(bytes_written.has_value());
+  EXPECT_EQ(kFooData.size(), bytes_written.value());
+
+  // Try to write "bar" at offset 0.  We expect that the offset is ignored
+  // (explicitly in `file_posix.cc`, implicitly/by-the-OS in `file_win.cc`).
+  const std::string_view kBarData = "bar";
+  bytes_written = file.Write(/*offset=*/0, as_byte_span(kBarData));
+  EXPECT_TRUE(bytes_written.has_value());
+  EXPECT_EQ(kBarData.size(), bytes_written.value());
+
+  // Close the file and re-read the contents to verify that happened above.
+  file.Close();
+  std::string actual_contents;
+  EXPECT_TRUE(ReadFileToString(file_path, &actual_contents));
+  EXPECT_EQ("testfoobar", actual_contents);
+}
+
 TEST(FileTest, Length) {
   ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
diff --git a/base/files/file_win.cc b/base/files/file_win.cc
index d2477f8..e2df9ea 100644
--- a/base/files/file_win.cc
+++ b/base/files/file_win.cc
@@ -408,7 +408,7 @@
 
   if (flags & FLAG_CREATE_ALWAYS) {
     DCHECK(!disposition);
-    DCHECK(flags & FLAG_WRITE);
+    DCHECK(flags & (FLAG_WRITE | FLAG_APPEND));
     disposition = CREATE_ALWAYS;
   }
 
diff --git a/content/browser/download/mhtml_generation_manager.cc b/content/browser/download/mhtml_generation_manager.cc
index 4deb5bc8..61cc2eb 100644
--- a/content/browser/download/mhtml_generation_manager.cc
+++ b/content/browser/download/mhtml_generation_manager.cc
@@ -88,10 +88,13 @@
   // SECURITY NOTE: A file descriptor to the file created below will be passed
   // to multiple renderer processes which (in out-of-process iframes mode) can
   // act on behalf of separate web principals.  Therefore it is important to
-  // only allow writing to the file and forbid reading from the file (as this
+  // only allow appending to the file and forbid reading from the file (as this
   // would allow reading content generated by other renderers / other web
+  // principals) and forbid seeking/overwriting earlier file contents (as this
+  // would allow overwriting content generated by other renderers / other web
   // principals).
-  uint32_t file_flags = base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE;
+  uint32_t file_flags =
+      base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_APPEND;
 
   base::File browser_file(file_path, file_flags);
   if (!browser_file.IsValid()) {
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/base/files/file_unittest.cc b/base/files/file_unittest.cc
index 8998864..5a327c5 100644
--- a/base/files/file_unittest.cc
+++ b/base/files/file_unittest.cc
@@ -454,6 +454,49 @@
   }
 }
 
+// Test whether `FLAG_APPEND` really works - i.e. that Chromium's
+// platform-specific code correctly uses the APIs provided by the OS for locking
+// down the capabilities of a file handle.
+TEST(FileTest, AppendEffectiveness) {
+  ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+  FilePath file_path = temp_dir.GetPath().AppendASCII("append_file_cant_seek");
+  File file(file_path, File::FLAG_CREATE | File::FLAG_APPEND);
+  ASSERT_TRUE(file.IsValid());
+
+  // Write "test" to the file.
+  const std::string_view kTestData = "test";
+  std::optional<size_t> bytes_written = file.Write(0, as_byte_span(kTestData));
+  EXPECT_TRUE(bytes_written.has_value());
+  EXPECT_EQ(kTestData.size(), bytes_written.value());
+
+  // Attempt to `Seek` should fail because of `FLAG_APPEND` used above.
+  //
+  // We ignore the returned value because it is irrelevant - the real
+  // verification is whether the seek actually happened which is
+  // verified via `ReadFileToString` below.
+  std::ignore = file.Seek(File::FROM_BEGIN, /*offset=*/0);
+
+  // Write "foo" to the file.
+  const std::string_view kFooData = "foo";
+  bytes_written = file.WriteAtCurrentPos(as_byte_span(kFooData));
+  EXPECT_TRUE(bytes_written.has_value());
+  EXPECT_EQ(kFooData.size(), bytes_written.value());
+
+  // Try to write "bar" at offset 0.  We expect that the offset is ignored
+  // (explicitly in `file_posix.cc`, implicitly/by-the-OS in `file_win.cc`).
+  const std::string_view kBarData = "bar";
+  bytes_written = file.Write(/*offset=*/0, as_byte_span(kBarData));
+  EXPECT_TRUE(bytes_written.has_value());
+  EXPECT_EQ(kBarData.size(), bytes_written.value());
+
+  // Close the file and re-read the contents to verify that happened above.
+  file.Close();
+  std::string actual_contents;
+  EXPECT_TRUE(ReadFileToString(file_path, &actual_contents));
+  EXPECT_EQ("testfoobar", actual_contents);
+}
+
 TEST(FileTest, Length) {
   ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
Loading diff…

Original Bug Report

reported by vm...@google.com

Cross-Origin MHTML Content Overwrite via Shared File Descriptor

Project Fortify, an experimental security project, has identified the following potential security issue.

Overview: MHTML generation shares a single file handle across all renderer processes in a frame tree without enforcing append-only writes. A compromised cross-origin subframe can use its duplicated handle to truncate the file and rewind the shared seek offset, erasing the main frame’s content. This allows the attacker to forge the entire MHTML archive, leading to a persistent Site Isolation bypass and Universal Cross-Site Scripting (UXSS), especially critical on Android’s automated Offline Pages.

Affected files:

  • content/browser/download/mhtml_generation_manager.cc
  • content/renderer/mhtml_handle_writer.cc
  • chrome/browser/offline_pages/offline_page_mhtml_archiver.cc
  • chrome/browser/offline_pages/recent_tab_helper.cc

Estimated timestamp from git blame: 2019-04-04

Description

There is a potential vulnerability in Chrome’s MHTML generation logic where a single file handle is shared across all renderers in a frame tree. In content/browser/download/mhtml_generation_manager.cc, CreateMHTMLFile opens a file with base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE. It does not use base::File::FLAG_APPEND.

To serialize each frame, the browser process calls mojom::MhtmlOutputHandle::NewFileHandle(browser_file_.Duplicate()) and sends this duplicated handle to the corresponding renderer process via the SerializeAsMHTML Mojo method.

On Windows and POSIX systems (excluding Fuchsia), base::File::Duplicate() uses DuplicateHandle() or dup(). These OS-level functions create a new descriptor that points to the exact same underlying kernel open-file description. Consequently, all renderer processes and the browser process share the same global file seek offset.

Because the main frame is processed first, it writes the top-level MHTML headers and its content to the file. However, when a cross-origin subframe receives its duplicated handle, it has full write access to the entire file. A compromised renderer can issue ftruncate(fd, 0) and lseek(fd, 0, SEEK_SET)—both of which are explicitly permitted by the Linux/Android seccomp-bpf sandbox for legitimately obtained file descriptors (sandbox/linux/seccomp-bpf-helpers/syscall_sets.cc). The attacker can then overwrite the file with a forged MHTML archive containing malicious scripts, attributing them to the main frame’s origin.

Impact

This is a Site Isolation bypass resulting in persistent cross-origin spoofing and UXSS. The impact is particularly severe on Android, where RecentTabHelper automatically triggers background MHTML saves for ‘Offline Pages’ when tabs are hidden. An attacker can silently poison the offline cache of a trusted victim site without any user interaction.

Potential Exploitation Steps

Note: These are suggested steps based on code analysis. Our setup does not execute code to verify these steps with a working proof of concept.

  1. Setup: An attacker embeds a cross-origin subframe (attacker.com) inside a trusted page (victim.com). The attacker uses a separate vulnerability (e.g., a V8 bug) to compromise the sandboxed renderer process hosting attacker.com.
  2. Trigger: An MHTML save is initiated (e.g., automatically by Android’s Offline Pages when the user switches tabs, or manually via ‘Save Page As’).
  3. Main Frame Serialization: The browser creates the file and sends a duplicated handle to the victim.com renderer, which writes legitimate MHTML headers and content. The shared file offset advances.
  4. Subframe Interception: The browser duplicates the handle again and sends it to the attacker.com renderer, along with the mhtml_boundary_marker inside mojom::SerializeAsMHTMLParams.
  5. Truncate and Rewind: The compromised attacker.com renderer intercepts the Mojo call, extracts the file descriptor (fd), and executes ftruncate(fd, 0) followed by lseek(fd, 0, SEEK_SET).
  6. Payload Injection: Using the provided boundary marker, the attacker writes a fully forged MHTML payload to fd. The forged payload claims to be victim.com and contains malicious scripts.
  7. Finalization: The browser appends the MHTML footer. The resulting file is a well-formed archive entirely controlled by the attacker.
  8. Execution: When the user opens the file or views the offline page, the attacker’s scripts execute in the context of victim.com.

Suggested Fix

There are two primary ways to fix this:

  1. Enforce Append-Only Writes: Modify the file creation in MHTMLGenerationManager::CreateMHTMLFile to use base::File::FLAG_APPEND instead of base::File::FLAG_WRITE. On POSIX, this maps to O_APPEND, ensuring that all writes automatically occur at the end of the file, regardless of any lseek() calls by a compromised renderer.
  2. Use Data Pipes: Transition the architecture to use Mojo Data Pipes instead of raw file descriptors. The MhtmlOutputHandle union already supports a producer_handle (mojo::ScopedDataPipeProducerHandle), and the renderer side has an implementation (MHTMLProducerHandleWriter). Updating the browser side to use this pipe would eliminate the shared kernel file description entirely.

Evaluated with Chrome root at commit: 0eb4855bda702feaaa8b899336664f97e3df88b8


Results so far have been promising, but there can be wrong deductions. If this proves to be a false positive, please close as WAI; data from false positives will be used to improve accuracy over time. Please feel free to reach out to me if you have concerns or feedback.

View on issue tracker