PatchGap
Chrome · Downloads

CVE-2026-87549

Logic Error in Downloads
Medium chrome Logic Error 📄 Reporter bug report 🔧 Commit mapped

Overview

Medium
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactIncomplete cleanup in Downloads
DescriptionIncomplete cleanup in Downloads
ComponentDownloads
Bug ClassLogic Error
Tracker516534546
Fix commit64a4680b5157 (chromium/src) +55/-6
CISA KEVNot listed
CreditedGoogle
Disclosed2026-09-08

Changed Functions

FunctionChangeNotes
if
components/download/internal/common/base_file.cc
 modified
TEST_F
components/download/internal/common/base_file_unittest.cc
 modified

Files Changed

  • components/download/internal/common/base_file.cc
  • components/download/internal/common/base_file_unittest.cc
  • components/download/internal/common/download_file_impl.cc
  • components/download/public/common/base_file.h

Patch

view commit
From 64a4680b515734720bbd8fab271b0be5f306da52 Mon Sep 17 00:00:00 2001
From: Yaw Frempong <yawfrempong@google.com>
Date: Fri, 31 Jul 2026 05:27:41 -0700
Subject: [PATCH] [Downloads] Truncate trailing data when finishing sparse downloads

When a parallel download is resumed from an intermediate file that is
longer than the persisted received-slice total (e.g. after an unclean
browser shutdown), the trailing bytes were left on disk and excluded
from the SHA-256 returned by BaseFile::Finish(), so the renamed file no
longer matched the hash reported to download consumers.

Truncate the sparse file to bytes_so_far_ in Finish() before computing
the hash, mirroring the truncation that the non-sparse Open() branch
already performs.

Regression test added to base_file_unittest.cc.

Reviewed in https://crrev.com/i/9589657

Bug: 516534546
Change-Id: I4d25e4d650353ffdad488ddbe8293f3ee3bcd07a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8177084
Commit-Queue: Yaw Frempong <yawfrempong@google.com>
Reviewed-by: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1671770}
---

diff --git a/components/download/internal/common/base_file.cc b/components/download/internal/common/base_file.cc
index 185b2b8..162cc3d 100644
--- a/components/download/internal/common/base_file.cc
+++ b/components/download/internal/common/base_file.cc
@@ -348,12 +348,24 @@
   Detach();
 }
 
-std::unique_ptr<crypto::SecureHash> BaseFile::Finish() {
+std::unique_ptr<crypto::SecureHash> BaseFile::Finish(int64_t expected_size) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   // TODO(qinmin): verify that all the holes have been filled.
-  if (is_sparse_file_)
+  if (is_sparse_file_) {
+    // Determine the target physical size to truncate to.
+    // If expected_size is provided (> 0), use that; otherwise fall back to
+    // bytes_so_far_.
+    int64_t target_size = (expected_size > 0) ? expected_size : bytes_so_far_;
+
+    // Calculate hash over the logical prefix
     CalculatePartialHash(std::string());
+
+    // Truncate trailing unverified bytes past the target size
+    if (file_.IsValid() && file_.GetLength() > target_size) {
+      file_.SetLength(target_size);
+    }
+  }
   Close();
   return std::move(secure_hash_);
 }
diff --git a/components/download/internal/common/base_file_unittest.cc b/components/download/internal/common/base_file_unittest.cc
index 5ed6bf95..7c4dd6b 100644
--- a/components/download/internal/common/base_file_unittest.cc
+++ b/components/download/internal/common/base_file_unittest.cc
@@ -830,6 +830,37 @@
   ExpectHashValue(kHashOfTestData1To3, base_file_->Finish());
 }
 
+// Open an existing file as a sparse file. The size on disk is larger than the
+// total number of bytes written by the time the download finishes. The
+// trailing bytes should be discarded so that the renamed file matches the
+// reported hash.
+TEST_F(BaseFileTest, ExistingSparseFileTooLong) {
+  base::FilePath file_path = temp_dir_.GetPath().AppendASCII("existing");
+  std::string contents;
+  contents.append(kTestData1);
+  contents.resize(kTestData1.size() + kTestData2.size() + kTestData3.size() +
+                      kTestData4.size(),
+                  'x');
+  ASSERT_TRUE(base::WriteFile(file_path, contents));
+
+  EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_NONE,
+            base_file_->Initialize(file_path, base::FilePath(), base::File(),
+                                   kTestData1.size(), std::string(),
+                                   std::unique_ptr<crypto::SecureHash>(), true,
+                                   &kTestDataBytesWasted));
+  base_file_->WriteDataToFile(kTestData1.size(),
+                              base::as_byte_span(kTestData2));
+  base_file_->WriteDataToFile(kTestData1.size() + kTestData2.size(),
+                              base::as_byte_span(kTestData3));
+  ExpectHashValue(kHashOfTestData1To3, base_file_->Finish());
+
+  base::FilePath new_path(temp_dir_.GetPath().AppendASCII("NewFile"));
+  EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_NONE, base_file_->Rename(new_path));
+  set_expected_data(base::JoinString({kTestData1, kTestData2, kTestData3}, ""));
+  base_file_->Detach();
+  expect_file_survives_ = true;
+}
+
 // Test that validating data in a file works.
 TEST_F(BaseFileTest, ValidateDataInFile) {
   ASSERT_TRUE(InitializeFile());
diff --git a/components/download/internal/common/download_file_impl.cc b/components/download/internal/common/download_file_impl.cc
index b1d6a9e..1fcaccb 100644
--- a/components/download/internal/common/download_file_impl.cc
+++ b/components/download/internal/common/download_file_impl.cc
@@ -785,9 +785,11 @@
   }
 
   std::unique_ptr<crypto::SecureHash> hash_state =
-      obfuscator_ ? obfuscator_->GetUnobfuscatedHash() : file_.Finish();
+      obfuscator_ ? obfuscator_->GetUnobfuscatedHash()
+                  : file_.Finish(potential_file_length_);
 #else
-  std::unique_ptr<crypto::SecureHash> hash_state = file_.Finish();
+  std::unique_ptr<crypto::SecureHash> hash_state =
+      file_.Finish(potential_file_length_);
 #endif
 
   update_timer_.reset();
@@ -937,7 +939,8 @@
   weak_factory_.InvalidateWeakPtrs();
 
   // TODO(b/367257039): Maintain obfuscated file hash for interrupted downloads.
-  std::unique_ptr<crypto::SecureHash> hash_state = file_.Finish();
+  std::unique_ptr<crypto::SecureHash> hash_state =
+      file_.Finish(potential_file_length_);
   main_task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&DownloadDestinationObserver::DestinationError, observer_,
diff --git a/components/download/public/common/base_file.h b/components/download/public/common/base_file.h
index d46c5c57..d3b58e0c 100644
--- a/components/download/public/common/base_file.h
+++ b/components/download/public/common/base_file.h
@@ -153,7 +153,10 @@
   // Returns the SecureHash object representing the state of the hash function
   // at the end of the operation. If |is_sparse_file_| is true, calling this
   // will cause |secure_hash_| to get calculated.
-  std::unique_ptr<crypto::SecureHash> Finish();
+  //
+  // |expected_size|: The expected final size of the file in bytes. If non-zero,
+  //     BaseFile will verify that the file size matches this value.
+  std::unique_ptr<crypto::SecureHash> Finish(int64_t expected_size = 0);
 
   // Callback used with AnnotateWithSourceInformation.
   // Created by DownloadFileImpl::RenameWithRetryInternal
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/components/download/internal/common/base_file_unittest.cc b/components/download/internal/common/base_file_unittest.cc
index 5ed6bf95..7c4dd6b 100644
--- a/components/download/internal/common/base_file_unittest.cc
+++ b/components/download/internal/common/base_file_unittest.cc
@@ -830,6 +830,37 @@
   ExpectHashValue(kHashOfTestData1To3, base_file_->Finish());
 }
 
+// Open an existing file as a sparse file. The size on disk is larger than the
+// total number of bytes written by the time the download finishes. The
+// trailing bytes should be discarded so that the renamed file matches the
+// reported hash.
+TEST_F(BaseFileTest, ExistingSparseFileTooLong) {
+  base::FilePath file_path = temp_dir_.GetPath().AppendASCII("existing");
+  std::string contents;
+  contents.append(kTestData1);
+  contents.resize(kTestData1.size() + kTestData2.size() + kTestData3.size() +
+                      kTestData4.size(),
+                  'x');
+  ASSERT_TRUE(base::WriteFile(file_path, contents));
+
+  EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_NONE,
+            base_file_->Initialize(file_path, base::FilePath(), base::File(),
+                                   kTestData1.size(), std::string(),
+                                   std::unique_ptr<crypto::SecureHash>(), true,
+                                   &kTestDataBytesWasted));
+  base_file_->WriteDataToFile(kTestData1.size(),
+                              base::as_byte_span(kTestData2));
+  base_file_->WriteDataToFile(kTestData1.size() + kTestData2.size(),
+                              base::as_byte_span(kTestData3));
+  ExpectHashValue(kHashOfTestData1To3, base_file_->Finish());
+
+  base::FilePath new_path(temp_dir_.GetPath().AppendASCII("NewFile"));
+  EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_NONE, base_file_->Rename(new_path));
+  set_expected_data(base::JoinString({kTestData1, kTestData2, kTestData3}, ""));
+  base_file_->Detach();
+  expect_file_survives_ = true;
+}
+
 // Test that validating data in a file works.
 TEST_F(BaseFileTest, ValidateDataInFile) {
   ASSERT_TRUE(InitializeFile());
Loading diff…

Original Bug Report

reported by vm...@google.com

Potential Safe Browsing hash bypass via lack of sparse file truncation during download resume

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: A potential vulnerability in Chrome’s download manager allows trailing on-disk payload bytes to escape Safe Browsing hash calculation during parallel downloads. When a parallel download is resumed using a stale database state (for example, after a browser crash within the 10-second DB commit window), the sparse-file branch of BaseFile::Open fails to truncate the intermediate file. This can result in a completed file retaining an untruncated malicious suffix, while Safe Browsing only receives and validates the hash of the benign prefix.

Affected files:

  • components/download/internal/common/base_file.cc
  • components/download/internal/common/download_file_impl.cc
  • components/download/internal/common/download_db_cache.cc

Estimated timestamp from git blame: Unknown (Google3 checkout)

Root Cause

In Chrome’s parallel download implementation, BaseFile::Open (in components/download/internal/common/base_file.cc) handles intermediate file opening and state validation. When reopening an intermediate file during download resumption, the non-sparse branch truncates the file via file_.SetLength(bytes_so_far_) whenever the on-disk file size exceeds the logged progress (bytes_so_far_).

However, the sparse branch (is_sparse_file_ == true) lacks this truncation, returning early if the file length is at least bytes_so_far_:

// components/download/internal/common/base_file.cc
if (is_sparse_file_) {
  if (file_.GetLength() < bytes_so_far_) {
    *bytes_wasted = bytes_so_far_;
    ClearFile();
    return LogInterruptReason("File has fewer written bytes than expected", 0,
                              DOWNLOAD_INTERRUPT_REASON_FILE_TOO_SHORT);
  }
  return DOWNLOAD_INTERRUPT_REASON_NONE;   // No truncation occurs here
}

When completion occurs, BaseFile::Finish() for sparse files invokes CalculatePartialHash() to hash the downloaded file, but only hashes exactly bytes_so_far_ bytes starting from offset 0. Since the physical file is never truncated and CalculatePartialHash only reads up to bytes_so_far_ (which is determined by the server-provided length of the final version of the download), trailing bytes from previous high-offset writes remain on disk. The file is eventually renamed and moved to the final path, retaining the untruncated trailing data.

Potential Trigger Path

Note: These are suggested/potential steps; our tooling agent does not currently have the capability to run code to verify this interactively.

  1. Parallel Download and High-Offset Writes: During a parallel download, ParallelDownloadJob spans multiple worker requests to download slices. A high-offset range worker writes data at offset H (writing B bytes up to H+B) directly to the intermediate .crdownload file.
  2. Metadata Batching Window: Intermediate database updates for in-progress downloads are batched in DownloadDBCache::AddOrReplaceEntry with a timer interval of kUpdateDBIntervalMs = 10000 (10 seconds) (see components/download/internal/common/download_db_cache.cc).
  3. Unclean Exit: If the browser process terminates abruptly (e.g. due to Android OOM-kill or a force stop) within this 10-second window, the physical file on disk retains the high-offset slice data (extending to length H+B), but the persisted database metadata (received_slices_) is stale, lacking this high-offset slice information.
  4. Resumption: Upon restart, the database entry loads the stale slice metadata. The download is resumed with the same ETag validator.
  5. No Truncation on Resume: DownloadFileImpl::Initialize initializes the file as sparse and calls BaseFile::Open(). Because the file is sparse, Open() skips truncation, leaving the physical file size at H+B bytes while the internal tracking state (bytes_so_far_) is set to 0 (or the stale metadata state).
  6. Completion and Hash Bypass: On resumption, the server completes the remaining download slices up to logical length L (where L < H+B) and completes successfully. BaseFile::Finish() is called, which computes the SHA-256 hash of only the prefix [0..L) via CalculatePartialHash().
  7. Bypass: The final file on disk is renamed to its destination but retains the trailing [L..H+B) bytes of attacker-controlled data. Safe Browsing checks are populated using the computed prefix hash and length L, allowing the file to bypass reputation checks if the prefix matches a known benign binary.

Suggested Fix

To remediate this issue, the intermediate file should be physically truncated to its logical size upon download completion. Inside BaseFile::Finish() (in components/download/internal/common/base_file.cc), truncate the file to bytes_so_far_ before closing and renaming it:

std::unique_ptr<crypto::SecureHash> BaseFile::Finish() {
  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

  if (is_sparse_file_) {
    CalculatePartialHash(std::string());
    // Truncate the sparse file to its final completed logical length
    if (file_.IsValid()) {
      file_.SetLength(bytes_so_far_);
    }
  }
  Close();
  return std::move(secure_hash_);
}

Evaluated with Chrome root at commit: a2bea94528f4bd6cc57739c43fa3bb890b8367d3

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