Chrome · Extensions › Extensions
CVE-2026-79263
Race in Extensions (Extensions)
Overview
Low
Severity
8.1
CVSS
No
Exploited ITW
Fixed
Fix Status
Background
- CRX
- The signed archive format for a Chrome extension, whose signature is verified before the archive’s ZIP contents are unpacked and installed.
- `SandboxedUnpacker::StartWithCrx`
- The entry point that verifies a CRX’s signature and then unzips it into a temporary working directory during (force-)installation.
- `ValidateSignature`
- The routine that extracts the public key and validates the package at a given file path, reporting an error and returning false on failure.
- TOCTOU (time-of-check to time-of-use)
- A race in which a file is validated at one moment but a different (attacker-swapped) file is consumed at a later moment.
Root Cause Analysis
Before the patch, SandboxedUnpacker::StartWithCrx in sandboxed_unpacker.cc called ValidateSignature(crx_info.path, ...) directly on the caller-supplied source path, and only after validation succeeded did it copy that source file (base::CopyFile) into the temporary working directory for unpacking. Because validation and the subsequent copy/unzip both read from the same external crx_info.path, the bytes that were signature-checked and the bytes that were later copied and unzipped were not guaranteed to be identical: an actor able to write the source path could replace the validated signed CRX with different content in the window between the check and the copy. The invariant violated is that the bytes whose signature is verified must be exactly the bytes that are installed.
The fix reorders the steps: it emits OnStageChanged(InstallationStage::kCopying), copies the source into temp_crx_path, and only then emits OnStageChanged(InstallationStage::kVerification) and calls ValidateSignature(temp_crx_path, ...) on the isolated working copy—so validation and unpacking operate on the same immutable copy. Supporting changes swap the ordinal values of kCopying and kVerification in InstallationStage, reorder the corresponding assignments in ReportCRXInstallationStage, and rename the force-install timing histograms to match the new stage order.
Key insight
The core mistake was validating the caller-supplied source CRX in place and copying it afterward, leaving a TOCTOU window; the fix copies the CRX into the working directory first and validates that private copy, so the verified bytes and the unpacked bytes are guaranteed identical.
Attack Path
- Point installation at a writable source CRX
An extension (force-)install is initiated with a
CRXFileInfowhosepathrefers to a file location an attacker can modify, such as a cached CRX on disk. - Wait for the check-then-copy window
Pre-patch,
StartWithCrxverifies the signature on the source path and then, on thekCopyingstage, copies that source into the working directory; the regression testSandboxedUnpackerTest.SourceCrxReplacedDuringInstallswaps the file exactly at thekCopyingcallback. - Swap in unsigned content The attacker overwrites the source file with different bytes (e.g. a plain ZIP with no valid CRX header) after validation but before/at the copy.
- Unpack unverified bytes
Pre-patch, the copied and unzipped bytes are the swapped content that never passed signature validation, so unsigned extension content could be installed; post-patch, validation runs on the already-copied
temp_crx_pathand fails withCRX_HEADER_INVALID, blocking installation. - Installation of attacker content (inferred)
Successful installation of unsigned/attacker-chosen extension content is the intended consequence; any further escalation is
(inferred)and not demonstrated by the patch.
Impact Assessment
An attacker able to modify the source CRX file during installation could win a TOCTOU race so that content differing from the signature-validated bytes is unpacked and installed, defeating the CRX signature guarantee for the installed extension. The advisory classifies this as a
Race condition in the Extensions component (“Race condition in Extensions”); it requires the ability to write the install source path within the check-to-use window and does not state exploitation in the wild. Any privilege gained from running the swapped extension content is (inferred).Changed Functions
| Function | Change | Notes |
|---|---|---|
ifchrome/browser/extensions/forced_extensions/force_installed_metrics.cc |
modified | |
ifextensions/browser/forced_extensions/install_stage_tracker.cc |
modified | |
ifextensions/browser/sandboxed_unpacker_unittest.cc |
modified | |
SandboxedUnpackerTestextensions/browser/sandboxed_unpacker_unittest.cc |
modified | |
TEST_Fextensions/browser/sandboxed_unpacker_unittest.cc |
modified |
Files Changed
chrome/browser/extensions/forced_extensions/force_installed_metrics_unittest.ccchrome/browser/extensions/updater/extension_updater_unittest.ccextensions/browser/sandboxed_unpacker_unittest.ccchrome/browser/extensions/forced_extensions/force_installed_metrics.ccextensions/browser/install_stage.hextensions/browser/sandboxed_unpacker.ccextensions/browser/forced_extensions/install_stage_tracker.cctools/metrics/histograms/metadata/extensions/histograms.xml
Audit Directions
- Validate-then-copy orderingSearch for install/update flows that validate a signature, hash, or integrity of a caller-supplied path and then re-read or copy that same external path, and confirm validation targets an isolated copy.
- Externally writable inputs consumed twiceAudit code that both checks and later uses a file whose location is attacker-influenced (caches, downloads, temp dirs) for TOCTOU windows.
- Stage/enum reordering side effectsWhen installation stage enums or metrics encode an implicit ordering, verify that reordering security-relevant steps also updates every consumer that relies on the old sequence.
Patch
From 07433eb1f903b534f350d41bd1d42db78af01bd5 Mon Sep 17 00:00:00 2001 From: Achuith Bhandarkar <[email protected]> Date: Wed, 22 Jul 2026 17:10:43 -0700 Subject: [PATCH] [extensions] Validate CRX signature on working copy SandboxedUnpacker::StartWithCrx() now copies the CRX archive into the working directory prior to running signature validation. Update the force installation metrics stage tracking order accordingly. BUG=497256260 TAG=agy CONV=5cdfb5a3-3edb-4d9c-9279-368a749b3673 Test: SandboxedUnpackerTest.SourceCrxReplacedDuringInstall in extensions_unittests Change-Id: I929ebb3fb99ad4973f01b694e0f29f71ac32d524 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8127239 Reviewed-by: Devlin Cronin <[email protected]> Reviewed-by: Kelvin Jiang <[email protected]> Commit-Queue: Achuith Bhandarkar <[email protected]> Cr-Commit-Position: refs/heads/main@{#1666728} --- diff --git a/chrome/browser/extensions/forced_extensions/force_installed_metrics.cc b/chrome/browser/extensions/forced_extensions/force_installed_metrics.cc index 098516f..3e9b2dd 100644 --- a/chrome/browser/extensions/forced_extensions/force_installed_metrics.cc +++ b/chrome/browser/extensions/forced_extensions/force_installed_metrics.cc @@ -164,20 +164,20 @@ installation.download_CRX_finish_time.value() - installation.download_CRX_started_time.value()); } - if (installation.copying_started_time) { - DCHECK(installation.verification_started_time); - base::UmaHistogramLongTimes( - "Extensions.ForceInstalledTime.VerificationStartTo.CopyingStart", - installation.copying_started_time.value() - - installation.verification_started_time.value()); - } - if (installation.unpacking_started_time && + if (installation.verification_started_time && installation.copying_started_time) { base::UmaHistogramLongTimes( - "Extensions.ForceInstalledTime.CopyingStartTo.UnpackingStart", - installation.unpacking_started_time.value() - + "Extensions.ForceInstalledTime.CopyingStartTo.VerificationStart", + installation.verification_started_time.value() - installation.copying_started_time.value()); } + if (installation.unpacking_started_time && + installation.verification_started_time) { + base::UmaHistogramLongTimes( + "Extensions.ForceInstalledTime.VerificationStartTo.UnpackingStart", + installation.unpacking_started_time.value() - + installation.verification_started_time.value()); + } if (installation.checking_expectations_started_time && installation.unpacking_started_time) { base::UmaHistogramLongTimes( diff --git a/extensions/browser/forced_extensions/install_stage_tracker.cc b/extensions/browser/forced_extensions/install_stage_tracker.cc index f42b462..0be8d3b 100644 --- a/extensions/browser/forced_extensions/install_stage_tracker.cc +++ b/extensions/browser/forced_extensions/install_stage_tracker.cc @@ -200,10 +200,10 @@ InstallationData& data = installation_data_map_[id]; data.installation_stage = stage; const base::TimeTicks current_time = base::TimeTicks::Now(); - if (stage == InstallationStage::kVerification) { - data.verification_started_time = current_time; - } else if (stage == InstallationStage::kCopying) { + if (stage == InstallationStage::kCopying) { data.copying_started_time = current_time; + } else if (stage == InstallationStage::kVerification) { + data.verification_started_time = current_time; } else if (stage == InstallationStage::kUnpacking) { data.unpacking_started_time = current_time; } else if (stage == InstallationStage::kCheckingExpectations) { diff --git a/extensions/browser/install_stage.h b/extensions/browser/install_stage.h index b6be1bdd..a85aabaa 100644 --- a/extensions/browser/install_stage.h +++ b/extensions/browser/install_stage.h @@ -9,10 +9,10 @@ // The different stages of the extension installation process. enum class InstallationStage { - // The validation of signature of the extensions is about to be started. - kVerification = 0, // Extension archive is about to be copied into the working directory. - kCopying = 1, + kCopying = 0, + // The validation of signature of the extensions is about to be started. + kVerification = 1, // Extension archive is about to be unpacked. kUnpacking = 2, // Performing the expectation checks before the installation can be started. diff --git a/extensions/browser/sandboxed_unpacker.cc b/extensions/browser/sandboxed_unpacker.cc index e820e61..f590c5f 100644 --- a/extensions/browser/sandboxed_unpacker.cc +++ b/extensions/browser/sandboxed_unpacker.cc @@ -287,7 +287,6 @@ // We assume that we are started on the thread that the client wants us // to do file IO on. DCHECK(unpacker_io_task_runner_->RunsTasksInCurrentSequence()); - client_->OnStageChanged(InstallationStage::kVerification); std::string expected_hash; if (!crx_info.expected_hash.empty() && base::CommandLine::ForCurrentProcess()->HasSwitch( @@ -302,15 +301,15 @@ // Initialize the path that will eventually contain the unpacked extension. extension_root_ = temp_dir_.GetPath().AppendASCII(kTempExtensionName); - // Extract the public key and validate the package. - if (!ValidateSignature( - crx_info.path, expected_hash, - format_verifier_override_.value_or(crx_info.required_format))) { - return; // ValidateSignature() already reported the error. - } - client_->OnStageChanged(InstallationStage::kCopying); - // Copy the crx file into our working directory. + // Copy the crx file into our working directory before validating its + // signature so that the bytes that are validated are the same bytes that + // are subsequently unzipped, even if the source path is modified + // concurrently. + // Note: Copying prior to signature validation incurs a file copy for invalid + // or corrupt CRX files, but guarantees that validation operates on the + // isolated working copy. Since verification failing is an uncommon case, + // this isn't a major performance concern. base::FilePath temp_crx_path = temp_dir_.GetPath().Append(crx_info.path.BaseName()); @@ -324,6 +323,14 @@ return; } + client_->OnStageChanged(InstallationStage::kVerification); + // Extract the public key and validate the package. + if (!ValidateSignature( + temp_crx_path, expected_hash, + format_verifier_override_.value_or(crx_info.required_format))) { + return; // ValidateSignature() already reported the error. + } + base::FilePath normalized_crx_path = NormalizeFilePath(temp_crx_path); client_->OnStageChanged(InstallationStage::kUnpacking); // Make sure to create the directory where the extension will be unzipped, as diff --git a/tools/metrics/histograms/metadata/extensions/histograms.xml b/tools/metrics/histograms/metadata/extensions/histograms.xml index 31e6a43..6a20937 100644 --- a/tools/metrics/histograms/metadata/extensions/histograms.xml +++ b/tools/metrics/histograms/metadata/extensions/histograms.xml @@ -2509,6 +2509,10 @@ summary="Time taken to perform the expectations checks to confirm that the extension can be installed."/> <variant name=".CopyingStartTo.UnpackingStart" + summary="(OBSOLETE: Replaced by VerificationStartTo.UnpackingStart) + Time taken to complete copying of extension archive into the + working directory"/> + <variant name=".CopyingStartTo.VerificationStart" summary="Time taken to complete copying of extension archive into the working directory"/> <variant name=".DownloadingStartTo.ManifestDownloadComplete" @@ -2522,6 +2526,9 @@ summary="Time taken to complete the unpacking of the extension archive."/> <variant name=".VerificationStartTo.CopyingStart" + summary="(OBSOLETE: Replaced by CopyingStartTo.VerificationStart) + Time taken to complete signature verification of CRX"/> + <variant name=".VerificationStartTo.UnpackingStart" summary="Time taken to complete signature verification of CRX"/> </token> </histogram> diff --git a/chrome/browser/extensions/forced_extensions/force_installed_metrics_unittest.cc b/chrome/browser/extensions/forced_extensions/force_installed_metrics_unittest.cc index d1849b68..b5330b6 100644 --- a/chrome/browser/extensions/forced_extensions/force_installed_metrics_unittest.cc +++ b/chrome/browser/extensions/forced_extensions/force_installed_metrics_unittest.cc @@ -150,10 +150,10 @@ constexpr char kCRXDownloadTimeStats[] = "Extensions.ForceInstalledTime.ManifestDownloadCompleteTo." "CRXDownloadComplete"; -constexpr char kVerificationTimeStats[] = - "Extensions.ForceInstalledTime.VerificationStartTo.CopyingStart"; constexpr char kCopyingTimeStats[] = - "Extensions.ForceInstalledTime.CopyingStartTo.UnpackingStart"; + "Extensions.ForceInstalledTime.CopyingStartTo.VerificationStart"; +constexpr char kVerificationTimeStats[] = + "Extensions.ForceInstalledTime.VerificationStartTo.UnpackingStart"; constexpr char kUnpackingTimeStats[] = "Extensions.ForceInstalledTime.UnpackingStartTo.CheckingExpectationsStart"; constexpr char kCheckingExpectationsTimeStats[] = @@ -392,12 +392,12 @@ ReportDownloadingManifestStage(); ReportInstallationStarted(std::nullopt); install_stage_tracker()->ReportCRXInstallationStage( - kExtensionId1, InstallationStage::kVerification); + kExtensionId1, InstallationStage::kCopying); const base::TimeDelta installation_stage_time = base::Milliseconds(200); task_environment_.FastForwardBy(installation_stage_time); install_stage_tracker()->ReportCRXInstallationStage( - kExtensionId1, InstallationStage::kCopying); + kExtensionId1, InstallationStage::kVerification); task_environment_.FastForwardBy(installation_stage_time); install_stage_tracker()->ReportCRXInstallationStage( @@ -422,12 +422,12 @@ // ForceInstalledMetrics shuts down timer because all extension are either // loaded or failed. EXPECT_FALSE(fake_timer_->IsRunning()); - histogram_tester_.ExpectTotalCount(kVerificationTimeStats, 1); - histogram_tester_.ExpectTimeBucketCount(kVerificationTimeStats, - installation_stage_time, 1); histogram_tester_.ExpectTotalCount(kCopyingTimeStats, 1); histogram_tester_.ExpectTimeBucketCount(kCopyingTimeStats, installation_stage_time, 1); + histogram_tester_.ExpectTotalCount(kVerificationTimeStats, 1); + histogram_tester_.ExpectTimeBucketCount(kVerificationTimeStats, + installation_stage_time, 1); histogram_tester_.ExpectTotalCount(kUnpackingTimeStats, 1); histogram_tester_.ExpectTimeBucketCount(kUnpackingTimeStats, installation_stage_time, 1); diff --git a/chrome/browser/extensions/updater/extension_updater_unittest.cc b/chrome/browser/extensions/updater/extension_updater_unittest.cc index 97c8f9f6..6c5ea856 100644 --- a/chrome/browser/extensions/updater/extension_updater_unittest.cc +++ b/chrome/browser/extensions/updater/extension_updater_unittest.cc @@ -1680,12 +1680,14 @@ // Add crx file entry in the cache. base::RunLoop put_extension_run_loop; + base::FilePath cached_crx_path; test_extension_cache.AllowCaching("test_app"); test_extension_cache.PutExtension( kTestExtensionId, "" /* expected hash*/, filename, version, base::BindLambdaForTesting( - [&put_extension_run_loop](const base::FilePath& file_path, - bool file_ownership_passed) { + [&put_extension_run_loop, &cached_crx_path]( + const base::FilePath& file_path, bool file_ownership_passed) { + cached_crx_path = file_path; put_extension_run_loop.Quit(); })); put_extension_run_loop.Run(); @@ -1743,7 +1745,7 @@ LoadErrorReporter::Init(false); updater.SetExtensionCacheForTesting(&test_extension_cache); - CRXFileInfo crx_info(filename, GetTestVerifierFormat()); + CRXFileInfo crx_info(cached_crx_path, GetTestVerifierFormat()); crx_info.extension_id = kTestExtensionId; crx_info.expected_hash = hash; diff --git a/extensions/browser/sandboxed_unpacker_unittest.cc b/extensions/browser/sandboxed_unpacker_unittest.cc index d7d32098..292c06d2 100644 --- a/extensions/browser/sandboxed_unpacker_unittest.cc +++ b/extensions/browser/sandboxed_unpacker_unittest.cc @@ -34,6 +34,7 @@ #include "extensions/browser/extensions_test.h" #include "extensions/browser/install/crx_install_error.h" #include "extensions/browser/install/sandboxed_unpacker_failure_reason.h" +#include "extensions/browser/install_stage.h" #include "extensions/common/constants.h" #include "extensions/common/extension.h" #include "extensions/common/extension_paths.h" @@ -118,6 +119,11 @@ should_compute_hashes_ = should_compute_hashes; } + void set_stage_changed_callback( + base::RepeatingCallback<void(InstallationStage)> callback) { + stage_changed_callback_ = std::move(callback); + } + void SetQuitClosure(base::OnceClosure quit_closure) { quit_closure_ = std::move(quit_closure); } @@ -153,12 +159,19 @@ callback_runner_->PostTask(FROM_HERE, std::move(quit_closure_)); } + void OnStageChanged(InstallationStage stage) override { + if (stage_changed_callback_) { + stage_changed_callback_.Run(stage); + } + } + scoped_refptr<base::SequencedTaskRunner> callback_runner_; std::optional<CrxInstallError> error_; base::OnceClosure quit_closure_; base::FilePath temp_dir_; raw_ptr<bool> deleted_tracker_ = nullptr; bool should_compute_hashes_ = false; + base::RepeatingCallback<void(InstallationStage)> stage_changed_callback_; }; class SandboxedUnpackerTest : public ExtensionsTest { @@ -231,6 +244,19 @@ run_loop.Run(); } + void SetupUnpackerWithPath(const base::FilePath& crx_path) { + extensions::CRXFileInfo crx_info(crx_path, GetTestVerifierFormat()); + + base::RunLoop run_loop; + client_->SetQuitClosure(run_loop.QuitClosure()); + + unpacker_task_runner_->PostTask( + FROM_HERE, base::BindOnce(&SandboxedUnpacker::StartWithCrx, + sandboxed_unpacker_, crx_info)); + // Wait for unpack + run_loop.Run(); + } + void SetupUnpackerWithDirectory(const std::string& crx_name) { base::ScopedTempDir temp_dir; ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); @@ -537,6 +563,46 @@ EXPECT_EQ(CrxInstallErrorType::NONE, GetInstallErrorType()); } +// Signature validation must operate on the same bytes that are unpacked. If +// the source file changes during installation, the unsigned content must not +// be installed. +TEST_F(SandboxedUnpackerTest, SourceCrxReplacedDuringInstall) { + // Stage a writable copy of a valid signed CRX as the install source. + base::ScopedTempDir source_dir; + ASSERT_TRUE(source_dir.CreateUniqueTempDir()); + base::FilePath source_crx = source_dir.GetPath().AppendASCII("ext.crx"); + ASSERT_TRUE(base::CopyFile(GetCrxFullPath("no_l10n.crx"), source_crx)); + + // Prepare a plain ZIP archive (not a signed CRX) to swap in as the source + // file at the point where the source is copied into the working directory. + base::FilePath unsigned_zip = source_dir.GetPath().AppendASCII("unsigned"); + ASSERT_TRUE(zip::Zip(GetCrxFullPath("no_l10n"), unsigned_zip, + /*include_hidden_files=*/true)); + // OnStageChanged(InstallationStage::kCopying) is invoked in StartWithCrx() + // immediately before base::CopyFile() copies the source CRX into Chrome's + // temporary working directory. When the callback fires on the kCopying stage, + // overwrite source_crx with unsigned_zip so base::CopyFile() copies the + // unsigned ZIP into temp_crx_path for signature validation. + client_->set_stage_changed_callback(base::BindRepeating( + [](const base::FilePath& from, const base::FilePath& to, + InstallationStage stage) { + if (stage == InstallationStage::kCopying) { + EXPECT_TRUE(base::CopyFile(from, to)); + } + }, + unsigned_zip, source_crx)); + + SetupUnpackerWithPath(source_crx); + + // The replacement archive has no valid CRX header, so validation must fail. + EXPECT_FALSE(InstallSucceeded()); + ASSERT_EQ(CrxInstallErrorType::SANDBOXED_UNPACKER_FAILURE, + GetInstallErrorType()); + EXPECT_EQ( + static_cast<int>(SandboxedUnpackerFailureReason::CRX_HEADER_INVALID), + GetInstallErrorDetail()); +} + // The following tests simulate the utility services failling. TEST_F(SandboxedUnpackerTest, UnzipperServiceFails) { // We override the Unzipper's launching behavior to drop the interface
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/chrome/browser/extensions/forced_extensions/force_installed_metrics_unittest.cc b/chrome/browser/extensions/forced_extensions/force_installed_metrics_unittest.cc
index d1849b68..b5330b6 100644
--- a/chrome/browser/extensions/forced_extensions/force_installed_metrics_unittest.cc
+++ b/chrome/browser/extensions/forced_extensions/force_installed_metrics_unittest.cc
@@ -150,10 +150,10 @@
constexpr char kCRXDownloadTimeStats[] =
"Extensions.ForceInstalledTime.ManifestDownloadCompleteTo."
"CRXDownloadComplete";
-constexpr char kVerificationTimeStats[] =
- "Extensions.ForceInstalledTime.VerificationStartTo.CopyingStart";
constexpr char kCopyingTimeStats[] =
- "Extensions.ForceInstalledTime.CopyingStartTo.UnpackingStart";
+ "Extensions.ForceInstalledTime.CopyingStartTo.VerificationStart";
+constexpr char kVerificationTimeStats[] =
+ "Extensions.ForceInstalledTime.VerificationStartTo.UnpackingStart";
constexpr char kUnpackingTimeStats[] =
"Extensions.ForceInstalledTime.UnpackingStartTo.CheckingExpectationsStart";
constexpr char kCheckingExpectationsTimeStats[] =
@@ -392,12 +392,12 @@
ReportDownloadingManifestStage();
ReportInstallationStarted(std::nullopt);
install_stage_tracker()->ReportCRXInstallationStage(
- kExtensionId1, InstallationStage::kVerification);
+ kExtensionId1, InstallationStage::kCopying);
const base::TimeDelta installation_stage_time = base::Milliseconds(200);
task_environment_.FastForwardBy(installation_stage_time);
install_stage_tracker()->ReportCRXInstallationStage(
- kExtensionId1, InstallationStage::kCopying);
+ kExtensionId1, InstallationStage::kVerification);
task_environment_.FastForwardBy(installation_stage_time);
install_stage_tracker()->ReportCRXInstallationStage(
@@ -422,12 +422,12 @@
// ForceInstalledMetrics shuts down timer because all extension are either
// loaded or failed.
EXPECT_FALSE(fake_timer_->IsRunning());
- histogram_tester_.ExpectTotalCount(kVerificationTimeStats, 1);
- histogram_tester_.ExpectTimeBucketCount(kVerificationTimeStats,
- installation_stage_time, 1);
histogram_tester_.ExpectTotalCount(kCopyingTimeStats, 1);
histogram_tester_.ExpectTimeBucketCount(kCopyingTimeStats,
installation_stage_time, 1);
+ histogram_tester_.ExpectTotalCount(kVerificationTimeStats, 1);
+ histogram_tester_.ExpectTimeBucketCount(kVerificationTimeStats,
+ installation_stage_time, 1);
histogram_tester_.ExpectTotalCount(kUnpackingTimeStats, 1);
histogram_tester_.ExpectTimeBucketCount(kUnpackingTimeStats,
installation_stage_time, 1);
diff --git a/chrome/browser/extensions/updater/extension_updater_unittest.cc b/chrome/browser/extensions/updater/extension_updater_unittest.cc
index 97c8f9f6..6c5ea856 100644
--- a/chrome/browser/extensions/updater/extension_updater_unittest.cc
+++ b/chrome/browser/extensions/updater/extension_updater_unittest.cc
@@ -1680,12 +1680,14 @@
// Add crx file entry in the cache.
base::RunLoop put_extension_run_loop;
+ base::FilePath cached_crx_path;
test_extension_cache.AllowCaching("test_app");
test_extension_cache.PutExtension(
kTestExtensionId, "" /* expected hash*/, filename, version,
base::BindLambdaForTesting(
- [&put_extension_run_loop](const base::FilePath& file_path,
- bool file_ownership_passed) {
+ [&put_extension_run_loop, &cached_crx_path](
+ const base::FilePath& file_path, bool file_ownership_passed) {
+ cached_crx_path = file_path;
put_extension_run_loop.Quit();
}));
put_extension_run_loop.Run();
@@ -1743,7 +1745,7 @@
LoadErrorReporter::Init(false);
updater.SetExtensionCacheForTesting(&test_extension_cache);
- CRXFileInfo crx_info(filename, GetTestVerifierFormat());
+ CRXFileInfo crx_info(cached_crx_path, GetTestVerifierFormat());
crx_info.extension_id = kTestExtensionId;
crx_info.expected_hash = hash;
diff --git a/extensions/browser/sandboxed_unpacker_unittest.cc b/extensions/browser/sandboxed_unpacker_unittest.cc
index d7d32098..292c06d2 100644
--- a/extensions/browser/sandboxed_unpacker_unittest.cc
+++ b/extensions/browser/sandboxed_unpacker_unittest.cc
@@ -34,6 +34,7 @@
#include "extensions/browser/extensions_test.h"
#include "extensions/browser/install/crx_install_error.h"
#include "extensions/browser/install/sandboxed_unpacker_failure_reason.h"
+#include "extensions/browser/install_stage.h"
#include "extensions/common/constants.h"
#include "extensions/common/extension.h"
#include "extensions/common/extension_paths.h"
@@ -118,6 +119,11 @@
should_compute_hashes_ = should_compute_hashes;
}
+ void set_stage_changed_callback(
+ base::RepeatingCallback<void(InstallationStage)> callback) {
+ stage_changed_callback_ = std::move(callback);
+ }
+
void SetQuitClosure(base::OnceClosure quit_closure) {
quit_closure_ = std::move(quit_closure);
}
@@ -153,12 +159,19 @@
callback_runner_->PostTask(FROM_HERE, std::move(quit_closure_));
}
+ void OnStageChanged(InstallationStage stage) override {
+ if (stage_changed_callback_) {
+ stage_changed_callback_.Run(stage);
+ }
+ }
+
scoped_refptr<base::SequencedTaskRunner> callback_runner_;
std::optional<CrxInstallError> error_;
base::OnceClosure quit_closure_;
base::FilePath temp_dir_;
raw_ptr<bool> deleted_tracker_ = nullptr;
bool should_compute_hashes_ = false;
+ base::RepeatingCallback<void(InstallationStage)> stage_changed_callback_;
};
class SandboxedUnpackerTest : public ExtensionsTest {
@@ -231,6 +244,19 @@
run_loop.Run();
}
+ void SetupUnpackerWithPath(const base::FilePath& crx_path) {
+ extensions::CRXFileInfo crx_info(crx_path, GetTestVerifierFormat());
+
+ base::RunLoop run_loop;
+ client_->SetQuitClosure(run_loop.QuitClosure());
+
+ unpacker_task_runner_->PostTask(
+ FROM_HERE, base::BindOnce(&SandboxedUnpacker::StartWithCrx,
+ sandboxed_unpacker_, crx_info));
+ // Wait for unpack
+ run_loop.Run();
+ }
+
void SetupUnpackerWithDirectory(const std::string& crx_name) {
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
@@ -537,6 +563,46 @@
EXPECT_EQ(CrxInstallErrorType::NONE, GetInstallErrorType());
}
+// Signature validation must operate on the same bytes that are unpacked. If
+// the source file changes during installation, the unsigned content must not
+// be installed.
+TEST_F(SandboxedUnpackerTest, SourceCrxReplacedDuringInstall) {
+ // Stage a writable copy of a valid signed CRX as the install source.
+ base::ScopedTempDir source_dir;
+ ASSERT_TRUE(source_dir.CreateUniqueTempDir());
+ base::FilePath source_crx = source_dir.GetPath().AppendASCII("ext.crx");
+ ASSERT_TRUE(base::CopyFile(GetCrxFullPath("no_l10n.crx"), source_crx));
+
+ // Prepare a plain ZIP archive (not a signed CRX) to swap in as the source
+ // file at the point where the source is copied into the working directory.
+ base::FilePath unsigned_zip = source_dir.GetPath().AppendASCII("unsigned");
+ ASSERT_TRUE(zip::Zip(GetCrxFullPath("no_l10n"), unsigned_zip,
+ /*include_hidden_files=*/true));
+ // OnStageChanged(InstallationStage::kCopying) is invoked in StartWithCrx()
+ // immediately before base::CopyFile() copies the source CRX into Chrome's
+ // temporary working directory. When the callback fires on the kCopying stage,
+ // overwrite source_crx with unsigned_zip so base::CopyFile() copies the
+ // unsigned ZIP into temp_crx_path for signature validation.
+ client_->set_stage_changed_callback(base::BindRepeating(
+ [](const base::FilePath& from, const base::FilePath& to,
+ InstallationStage stage) {
+ if (stage == InstallationStage::kCopying) {
+ EXPECT_TRUE(base::CopyFile(from, to));
+ }
+ },
+ unsigned_zip, source_crx));
+
+ SetupUnpackerWithPath(source_crx);
+
+ // The replacement archive has no valid CRX header, so validation must fail.
+ EXPECT_FALSE(InstallSucceeded());
+ ASSERT_EQ(CrxInstallErrorType::SANDBOXED_UNPACKER_FAILURE,
+ GetInstallErrorType());
+ EXPECT_EQ(
+ static_cast<int>(SandboxedUnpackerFailureReason::CRX_HEADER_INVALID),
+ GetInstallErrorDetail());
+}
+
// The following tests simulate the utility services failling.
TEST_F(SandboxedUnpackerTest, UnzipperServiceFails) {
// We override the Unzipper's launching behavior to drop the interface
Loading diff…
Original Bug Report
reported by [email protected]
TOCTOU in SandboxedUnpacker allows CRX signature bypass via UNC paths
Project Fortify, an experimental security project, has identified the following potential security issue.
Overview: A potential Time-of-Check to Time-of-Use (TOCTOU) vulnerability exists in SandboxedUnpacker::StartWithCrx when installing extensions. An attacker can swap a legitimately signed CRX file with a malicious archive between signature verification and file copying, allowing them to bypass signature checks and spoof the extension’s identity.
Affected files:
extensions/browser/sandboxed_unpacker.ccchrome/browser/extensions/external_registry_loader_win.ccextensions/browser/crx_installer.ccchrome/browser/extensions/external_provider_manager.cc
Estimated timestamp from git blame: 2021-07-22
Summary
A potential Time-of-Check to Time-of-Use (TOCTOU) vulnerability in SandboxedUnpacker::StartWithCrx allows an attacker to bypass CRX signature verification. When Chrome loads an external extension from a network share (e.g., via a Windows registry policy pointing to a UNC path), an attacker who controls the network share can swap the file between the verification step and the copy step. This allows the installation of a malicious extension that inherits the trusted identity of the legitimate extension.
Technical Details
In extensions/browser/sandboxed_unpacker.cc, the StartWithCrx method performs signature validation and file copying in two distinct, sequential operations using the same file path:
// 1. Time of Check: Validates signature and extracts public key.
if (!ValidateSignature(
crx_info.path, expected_hash,
format_verifier_override_.value_or(crx_info.required_format))) {
return;
}
// ...
// 2. Time of Use: Re-opens the file by path to copy it locally.
base::FilePath temp_crx_path =
temp_dir_.GetPath().Append(crx_info.path.BaseName());
if (!base::CopyFile(crx_info.path, temp_crx_path)) {
// ...
}
ValidateSignature calls crx_file::Verify, which opens the file, validates the signature, extracts public_key_ and extension_id_, and then closes the file handle. Immediately after, base::CopyFile re-opens the file by its path.
Because the file handle is dropped between these operations, an attacker can serve a legitimate CRX file during the first read, and a malicious ZIP archive during the second read.
The impact is critical because SandboxedUnpacker::RewriteManifestFile forcibly injects the public_key_ (extracted during the legitimate verification step) into the attacker’s parsed manifest:
final_manifest.Set(manifest_keys::kPublicKey, public_key_);
Since Chrome derives the extension ID from this public key, the malicious extension flawlessly spoof’s the legitimate extension’s ID, passing subsequent CrxInstaller::CheckExpectations checks. Additionally, for off-store external extensions, Chrome’s content verification typically runs in UNSIGNED_HASHES or NONE mode, allowing the malicious extension to run silently without a valid verified_contents.json signature.
Potential Attack Steps
Note: These are potential steps based on code analysis. Our tooling agent does not currently have the ability to run code to provide a working Proof of Concept.
- Configuration: A system administrator configures a Windows machine to install an extension via the registry, pointing to a network share (e.g.,
HKLM\Software\Google\Chrome\Extensions\<id>\path = \\attacker-server\share\ext.crx). - Attacker Setup: The attacker configures their SMB server (
\\attacker-server) to serve a legitimately signed CRX file upon the first file open request, and a malicious ZIP archive upon the second open request. - Time of Check: Chrome attempts to install the extension.
crx_file::Verifyopens the file over SMB. The attacker’s server delivers the legitimate CRX. Chrome extracts the trusted public key and closes the file handle. - Payload Swap: The attacker’s SMB server observes the
Closerequest and immediately swaps the served payload to the malicious ZIP archive. - Time of Use: Chrome calls
base::CopyFile, re-opening the file over SMB. The attacker’s server delivers the malicious ZIP archive, which Chrome copies to its local temporary directory. - Identity Forgery: Chrome unzips the malicious archive, parses its manifest, and injects the trusted public key into it. The malicious extension is installed with the identity and privileges of the legitimate extension.
Suggested Fix
To eliminate the TOCTOU race condition, Chrome should never rely on the same file path for both validation and unpacking without holding an exclusive lock, especially for paths outside of Chrome’s secure directories.
The CRX file should be copied to a secure, locally-owned temporary directory before any signature validation or unpacking takes place.
- Modify
CrxInstallerorSandboxedUnpackerto first executebase::CopyFile(or equivalent) to move the source file into Chrome’stemp_dir_. - Update
ValidateSignatureto operate exclusively on the local, securely-copied temporary file rather than the originalcrx_info.path.
Evaluated with Chrome root at commit: a9cbf6e8b275fe4147435aa905f3b7f5a656f5f0
Results from 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. And please feel free to reach out to me directly if you have concerns or feedback on the project.
References
On This Page