Chrome · Downloads
CVE-2026-87549
Logic Error in Downloads
Overview
Medium
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifcomponents/download/internal/common/base_file.cc |
modified | |
TEST_Fcomponents/download/internal/common/base_file_unittest.cc |
modified |
Files Changed
components/download/internal/common/base_file.cccomponents/download/internal/common/base_file_unittest.cccomponents/download/internal/common/download_file_impl.cccomponents/download/public/common/base_file.h
Patch
From 64a4680b515734720bbd8fab271b0be5f306da52 Mon Sep 17 00:00:00 2001 From: Yaw Frempong <[email protected]> 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 <[email protected]> Reviewed-by: Min Qin <[email protected]> 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 [email protected]
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.cccomponents/download/internal/common/download_file_impl.cccomponents/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.
- Parallel Download and High-Offset Writes: During a parallel download,
ParallelDownloadJobspans multiple worker requests to download slices. A high-offset range worker writes data at offsetH(writingBbytes up toH+B) directly to the intermediate.crdownloadfile. - Metadata Batching Window: Intermediate database updates for in-progress downloads are batched in
DownloadDBCache::AddOrReplaceEntrywith a timer interval ofkUpdateDBIntervalMs = 10000(10 seconds) (seecomponents/download/internal/common/download_db_cache.cc). - 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. - Resumption: Upon restart, the database entry loads the stale slice metadata. The download is resumed with the same ETag validator.
- No Truncation on Resume:
DownloadFileImpl::Initializeinitializes the file as sparse and callsBaseFile::Open(). Because the file is sparse,Open()skips truncation, leaving the physical file size atH+Bbytes while the internal tracking state (bytes_so_far_) is set to 0 (or the stale metadata state). - Completion and Hash Bypass: On resumption, the server completes the remaining download slices up to logical length
L(whereL < H+B) and completes successfully.BaseFile::Finish()is called, which computes the SHA-256 hash of only the prefix[0..L)viaCalculatePartialHash(). - 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 lengthL, 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.
References
On This Page