CVE-2026-11269
Overview
Changed Functions
| Function | Change | Notes |
|---|---|---|
IN_PROC_BROWSER_TEST_Fchrome/browser/extensions/extension_disabled_ui_browsertest.cc |
modified | |
ifchrome/browser/extensions/extension_disabled_ui_browsertest.cc |
modified | |
FakeUpdateServicechrome/browser/extensions/extension_service_unittest.cc |
modified |
Files Changed
chrome/browser/extensions/chrome_extensions_browser_client.ccchrome/browser/extensions/extension_disabled_ui_browsertest.ccchrome/browser/extensions/extension_service_unittest.cc
Patch
From e585ba13ce9ed5667530a315850713c1eeb3cd20 Mon Sep 17 00:00:00 2001
From: Joshua Pawlicki <waffles@chromium.org>
Date: Fri, 17 Apr 2026 07:58:49 -0700
Subject: [PATCH] extension updater: Use update_client updater for sync/policy installs
Previously, the first install of an extension (when driven by sync or
policy) would still default to using the old extension update stack.
This isn't necessary; the new stack can be used in this case.
This relands
https://chromium-review.googlesource.com/c/chromium/src/+/7545510
The original CL is patchset 1. Since then,
ExternalProviderImplTest.InAppPayments (which isn't covered by CQ)
is fixed, which was unfortunately a bigger job than I hoped.
Fixed: 482088398, 500551122
Change-Id: Iee3d224770896f5abc288d1facbd492f67ef9554
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7769170
Reviewed-by: Owen Min <zmin@chromium.org>
Auto-Submit: Joshua Pawlicki <waffles@chromium.org>
Commit-Queue: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1616564}
---
diff --git a/chrome/browser/extensions/chrome_extensions_browser_client.cc b/chrome/browser/extensions/chrome_extensions_browser_client.cc
index da1b390..e9122854 100644
--- a/chrome/browser/extensions/chrome_extensions_browser_client.cc
+++ b/chrome/browser/extensions/chrome_extensions_browser_client.cc
@@ -149,9 +149,6 @@
namespace {
-constexpr std::string_view kCrxUrlPath = "/service/update2/crx";
-constexpr std::string_view kJsonUrlPath = "/service/update2/json";
-
// If true, the extensions client will behave as though there is always a
// new chrome update.
bool g_did_chrome_update_for_testing = false;
@@ -657,16 +654,7 @@
scoped_refptr<update_client::Configurator>
ChromeExtensionsBrowserClient::CreateUpdateClientConfigurator(
content::BrowserContext* context) {
- std::optional<GURL> override_url;
- GURL update_url = extension_urls::GetWebstoreUpdateUrl();
- if (update_url != extension_urls::GetDefaultWebstoreUpdateUrl()) {
- if (update_url.GetPath() == kCrxUrlPath) {
- override_url = update_url.GetWithEmptyPath().Resolve(kJsonUrlPath);
- } else {
- override_url = update_url;
- }
- }
- return ChromeUpdateClientConfig::Create(context, override_url);
+ return ChromeUpdateClientConfig::Create(context);
}
std::unique_ptr<ScopedBrowserContextKeepAlive>
diff --git a/chrome/browser/extensions/extension_disabled_ui_browsertest.cc b/chrome/browser/extensions/extension_disabled_ui_browsertest.cc
index aba184e8..f9924d4 100644
--- a/chrome/browser/extensions/extension_disabled_ui_browsertest.cc
+++ b/chrome/browser/extensions/extension_disabled_ui_browsertest.cc
@@ -43,6 +43,7 @@
#include "extensions/browser/extension_system.h"
#include "extensions/browser/test_extension_registry_observer.h"
#include "extensions/common/extension.h"
+#include "extensions/common/verifier_formats.h"
#include "extensions/test/extension_test_message_listener.h"
#include "testing/gmock/include/gmock/gmock.h"
@@ -274,13 +275,76 @@
IN_PROC_BROWSER_TEST_F(ExtensionDisabledGlobalErrorTest, RemoteInstall) {
static const char extension_id[] = "pgdpcfcocojkjfbgpiianjngphoopgmo";
+ auto reset = extensions::DisablePublisherKeyVerificationForTests();
+ content::URLLoaderInterceptor interceptor(base::BindLambdaForTesting(
+ [&](content::URLLoaderInterceptor::RequestParams* params) {
+ std::string path = params->url_request.url.GetPath();
+ if (path == "/autoupdate_nonwebstore/updates.xml") {
+ content::URLLoaderInterceptor::WriteResponse(
+ test_data_dir_.AppendASCII("permissions_increase")
+ .AppendASCII("updates.xml"),
+ params->client.get());
+ return true;
+ } else if (path == "/autoupdate/v2.crx") {
+ content::URLLoaderInterceptor::WriteResponse(path_v2_,
+ params->client.get());
+ return true;
+ }
+ return false;
+ }));
+
+ sync_pb::EntitySpecifics specifics;
+ specifics.mutable_extension()->set_id(extension_id);
+ specifics.mutable_extension()->set_enabled(false);
+ specifics.mutable_extension()->set_remote_install(true);
+ specifics.mutable_extension()->set_disable_reasons(
+ extensions::disable_reason::DISABLE_REMOTE_INSTALL);
+ specifics.mutable_extension()->set_update_url(
+ "http://localhost/autoupdate_nonwebstore/updates.xml");
+ specifics.mutable_extension()->set_version("2");
+ syncer::SyncData sync_data = syncer::SyncData::CreateRemoteData(
+ specifics, syncer::ClientTagHash::FromHashed("unused"));
+
+ ExtensionSyncService* sync_service = ExtensionSyncService::Get(profile());
+ sync_service->MergeDataAndStartSyncing(
+ syncer::EXTENSIONS, syncer::SyncDataList(),
+ std::make_unique<syncer::FakeSyncChangeProcessor>());
+ extensions::TestExtensionRegistryObserver install_observer(
+ extension_registry());
+ sync_service->ProcessSyncChanges(
+ FROM_HERE,
+ syncer::SyncChangeList(
+ 1, syncer::SyncChange(FROM_HERE, syncer::SyncChange::ACTION_ADD,
+ sync_data)));
+
+ install_observer.WaitForExtensionWillBeInstalled();
+ content::RunAllTasksUntilIdle();
+
+ const Extension* extension =
+ extension_registry()->disabled_extensions().GetByID(extension_id);
+ ASSERT_TRUE(extension);
+ EXPECT_EQ("2", extension->VersionString());
+ EXPECT_EQ(1u, extension_registry()->disabled_extensions().size());
+ EXPECT_THAT(ExtensionPrefs::Get(extension_service()->profile())
+ ->GetDisableReasons(extension_id),
+ testing::UnorderedElementsAre(
+ extensions::disable_reason::DISABLE_REMOTE_INSTALL));
+ EXPECT_TRUE(GetExtensionDisabledGlobalError());
+}
+
+// Test that an error appears if an extension gets installed server side.
+IN_PROC_BROWSER_TEST_F(ExtensionDisabledGlobalErrorTest,
+ RemoteInstallFromWebstore) {
+ static const char extension_id[] = "pgdpcfcocojkjfbgpiianjngphoopgmo";
+
+ auto reset = extensions::DisablePublisherKeyVerificationForTests();
content::URLLoaderInterceptor interceptor(base::BindLambdaForTesting(
[&](content::URLLoaderInterceptor::RequestParams* params) {
std::string path = params->url_request.url.GetPath();
if (path == "/autoupdate/updates.xml") {
content::URLLoaderInterceptor::WriteResponse(
test_data_dir_.AppendASCII("permissions_increase")
- .AppendASCII("updates.xml"),
+ .AppendASCII("updates.json"),
params->client.get());
return true;
} else if (path == "/autoupdate/v2.crx") {
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index 73f5bebd..30c3d711 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -8646,6 +8646,31 @@
: public ExtensionServiceTest,
public testing::WithParamInterface<ManifestLocation> {};
+namespace {
+
+class FakeUpdateService : public UpdateService {
+ public:
+ FakeUpdateService() : UpdateService(nullptr, nullptr, base::DoNothing()) {}
+
+ void StartUpdateCheck(const ExtensionUpdateCheckParams& update_params,
+ UpdateFoundCallback update_found_callback,
+ base::OnceClosure callback) override {
+ last_update_params_ = update_params;
+ if (!callback.is_null()) {
+ std::move(callback).Run();
+ }
+ }
+
+ const std::optional<ExtensionUpdateCheckParams>& last_update_params() const {
+ return last_update_params_;
+ }
+
+ private:
+ std::optional<ExtensionUpdateCheckParams> last_update_params_;
+};
+
+} // namespace
+
// Policy-forced extensions should be fetched with FOREGROUND priority,
// otherwise they may be throttled (web store sends “noupdate” response to
// reduce load), which is OK for updates, but not for a new install. This is
@@ -8657,11 +8682,10 @@
params.autoupdate_enabled = true;
InitializeExtensionService(std::move(params));
- ExtensionDownloaderTestHelper helper;
- NullExtensionCache extension_cache;
+ FakeUpdateService fake_update_service;
+ UpdateService::SupplyUpdateServiceForTest(&fake_update_service);
+
ExtensionUpdater* updater = ExtensionUpdater::Get(profile());
- updater->SetExtensionDownloaderForTesting(helper.CreateDownloader());
- updater->SetExtensionCacheForTesting(&extension_cache);
updater->Start();
GURL update_url(extension_urls::kChromeWebstoreUpdateURL);
@@ -8679,18 +8703,21 @@
Regression Test / PoC
diff --git a/chrome/browser/extensions/extension_disabled_ui_browsertest.cc b/chrome/browser/extensions/extension_disabled_ui_browsertest.cc
index aba184e8..f9924d4 100644
--- a/chrome/browser/extensions/extension_disabled_ui_browsertest.cc
+++ b/chrome/browser/extensions/extension_disabled_ui_browsertest.cc
@@ -43,6 +43,7 @@
#include "extensions/browser/extension_system.h"
#include "extensions/browser/test_extension_registry_observer.h"
#include "extensions/common/extension.h"
+#include "extensions/common/verifier_formats.h"
#include "extensions/test/extension_test_message_listener.h"
#include "testing/gmock/include/gmock/gmock.h"
@@ -274,13 +275,76 @@
IN_PROC_BROWSER_TEST_F(ExtensionDisabledGlobalErrorTest, RemoteInstall) {
static const char extension_id[] = "pgdpcfcocojkjfbgpiianjngphoopgmo";
+ auto reset = extensions::DisablePublisherKeyVerificationForTests();
+ content::URLLoaderInterceptor interceptor(base::BindLambdaForTesting(
+ [&](content::URLLoaderInterceptor::RequestParams* params) {
+ std::string path = params->url_request.url.GetPath();
+ if (path == "/autoupdate_nonwebstore/updates.xml") {
+ content::URLLoaderInterceptor::WriteResponse(
+ test_data_dir_.AppendASCII("permissions_increase")
+ .AppendASCII("updates.xml"),
+ params->client.get());
+ return true;
+ } else if (path == "/autoupdate/v2.crx") {
+ content::URLLoaderInterceptor::WriteResponse(path_v2_,
+ params->client.get());
+ return true;
+ }
+ return false;
+ }));
+
+ sync_pb::EntitySpecifics specifics;
+ specifics.mutable_extension()->set_id(extension_id);
+ specifics.mutable_extension()->set_enabled(false);
+ specifics.mutable_extension()->set_remote_install(true);
+ specifics.mutable_extension()->set_disable_reasons(
+ extensions::disable_reason::DISABLE_REMOTE_INSTALL);
+ specifics.mutable_extension()->set_update_url(
+ "http://localhost/autoupdate_nonwebstore/updates.xml");
+ specifics.mutable_extension()->set_version("2");
+ syncer::SyncData sync_data = syncer::SyncData::CreateRemoteData(
+ specifics, syncer::ClientTagHash::FromHashed("unused"));
+
+ ExtensionSyncService* sync_service = ExtensionSyncService::Get(profile());
+ sync_service->MergeDataAndStartSyncing(
+ syncer::EXTENSIONS, syncer::SyncDataList(),
+ std::make_unique<syncer::FakeSyncChangeProcessor>());
+ extensions::TestExtensionRegistryObserver install_observer(
+ extension_registry());
+ sync_service->ProcessSyncChanges(
+ FROM_HERE,
+ syncer::SyncChangeList(
+ 1, syncer::SyncChange(FROM_HERE, syncer::SyncChange::ACTION_ADD,
+ sync_data)));
+
+ install_observer.WaitForExtensionWillBeInstalled();
+ content::RunAllTasksUntilIdle();
+
+ const Extension* extension =
+ extension_registry()->disabled_extensions().GetByID(extension_id);
+ ASSERT_TRUE(extension);
+ EXPECT_EQ("2", extension->VersionString());
+ EXPECT_EQ(1u, extension_registry()->disabled_extensions().size());
+ EXPECT_THAT(ExtensionPrefs::Get(extension_service()->profile())
+ ->GetDisableReasons(extension_id),
+ testing::UnorderedElementsAre(
+ extensions::disable_reason::DISABLE_REMOTE_INSTALL));
+ EXPECT_TRUE(GetExtensionDisabledGlobalError());
+}
+
+// Test that an error appears if an extension gets installed server side.
+IN_PROC_BROWSER_TEST_F(ExtensionDisabledGlobalErrorTest,
+ RemoteInstallFromWebstore) {
+ static const char extension_id[] = "pgdpcfcocojkjfbgpiianjngphoopgmo";
+
+ auto reset = extensions::DisablePublisherKeyVerificationForTests();
content::URLLoaderInterceptor interceptor(base::BindLambdaForTesting(
[&](content::URLLoaderInterceptor::RequestParams* params) {
std::string path = params->url_request.url.GetPath();
if (path == "/autoupdate/updates.xml") {
content::URLLoaderInterceptor::WriteResponse(
test_data_dir_.AppendASCII("permissions_increase")
- .AppendASCII("updates.xml"),
+ .AppendASCII("updates.json"),
params->client.get());
return true;
} else if (path == "/autoupdate/v2.crx") {
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index 73f5bebd..30c3d711 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -8646,6 +8646,31 @@
: public ExtensionServiceTest,
public testing::WithParamInterface<ManifestLocation> {};
+namespace {
+
+class FakeUpdateService : public UpdateService {
+ public:
+ FakeUpdateService() : UpdateService(nullptr, nullptr, base::DoNothing()) {}
+
+ void StartUpdateCheck(const ExtensionUpdateCheckParams& update_params,
+ UpdateFoundCallback update_found_callback,
+ base::OnceClosure callback) override {
+ last_update_params_ = update_params;
+ if (!callback.is_null()) {
+ std::move(callback).Run();
+ }
+ }
+
+ const std::optional<ExtensionUpdateCheckParams>& last_update_params() const {
+ return last_update_params_;
+ }
+
+ private:
+ std::optional<ExtensionUpdateCheckParams> last_update_params_;
+};
+
+} // namespace
+
// Policy-forced extensions should be fetched with FOREGROUND priority,
// otherwise they may be throttled (web store sends “noupdate” response to
// reduce load), which is OK for updates, but not for a new install. This is
@@ -8657,11 +8682,10 @@
params.autoupdate_enabled = true;
InitializeExtensionService(std::move(params));
- ExtensionDownloaderTestHelper helper;
- NullExtensionCache extension_cache;
+ FakeUpdateService fake_update_service;
+ UpdateService::SupplyUpdateServiceForTest(&fake_update_service);
+
ExtensionUpdater* updater = ExtensionUpdater::Get(profile());
- updater->SetExtensionDownloaderForTesting(helper.CreateDownloader());
- updater->SetExtensionCacheForTesting(&extension_cache);
updater->Start();
GURL update_url(extension_urls::kChromeWebstoreUpdateURL);
@@ -8679,18 +8703,21 @@
task_environment()->RunUntilIdle();
- EXPECT_EQ(helper.test_url_loader_factory().NumPending(), 1);
- network::TestURLLoaderFactory::PendingRequest* pending_request =
- helper.test_url_loader_factory().GetPendingRequest(0);
+ ASSERT_TRUE(fake_update_service.last_update_params().has_value());
+ const ExtensionUpdateCheckParams& update_params =
+ *fake_update_service.last_update_params();
+
bool is_high_priority =
GetParam() == ManifestLocation::kExternalPolicyDownload ||
GetParam() == ManifestLocation::kExternalComponent;
- std::string expected_header = is_high_priority ? "fg" : "bg";
- EXPECT_EQ(expected_header, pending_request->request.headers.GetHeader(
- "X-Goog-Update-Interactivity"));
- // Destroy updater's downloader as it uses |helper|.
- updater->SetExtensionDownloaderForTesting(nullptr);
+ ExtensionUpdateCheckParams::UpdateCheckPriority expected_priority =
+ is_high_priority ? ExtensionUpdateCheckParams::FOREGROUND
+ : ExtensionUpdateCheckParams::BACKGROUND;
+
+ EXPECT_EQ(expected_priority, update_params.priority);
+
+ UpdateService::SupplyUpdateServiceForTest(nullptr);
}
INSTANTIATE_TEST_SUITE_P(
diff --git a/chrome/browser/extensions/external_provider_impl_unittest.cc b/chrome/browser/extensions/external_provider_impl_unittest.cc
index dacf301..3067f41 100644
--- a/chrome/browser/extensions/external_provider_impl_unittest.cc
+++ b/chrome/browser/extensions/external_provider_impl_unittest.cc
@@ -35,6 +35,7 @@
#include "chrome/test/base/testing_browser_process.h"
#include "chrome/test/base/testing_profile.h"
#include "content/public/test/test_utils.h"
+#include "crypto/hash.h"
#include "extensions/browser/extension_registrar.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/browser/pref_names.h"
@@ -224,6 +225,12 @@
std::unique_ptr<net::test_server::EmbeddedTestServer> test_server_;
private:
+ base::FilePath GetCrxPath(const std::string& test_path) {
+ base::FilePath test_data_dir;
+ CHECK(base::PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir));
+ return test_data_dir.AppendASCII(test_path);
+ }
+
std::unique_ptr<net::test_server::HttpResponse> HandleRequest(
const net::test_server::HttpRequest& request) {
GURL url = test_server_->GetURL(request.relative_url);
@@ -231,20 +238,34 @@
if (url.GetPath() == test_extension.update_path) {
auto response = std::make_unique<net::test_server::BasicHttpResponse>();
response->set_code(net::HTTP_OK);
- response->set_content(CreateUpdateManifest(
- {UpdateManifestItem(test_extension.app_id)
- .version(test_extension.version)
- .codebase(
- test_server_->GetURL(test_extension.app_path).spec())}));
- response->set_content_type("text/xml");
+ if (url.GetPath() == kInAppPaymentsApp.update_path) {
+ // SetUp() configured kInAppPaymentsApp.update_path as the gallery
+ // URL. A V4 response is needed in this case.
+ std::string contents;
+ base::ReadFileToString(GetCrxPath(test_extension.crx_path),
+ &contents);
+ response->set_content(CreateUpdateManifestV4(
+ {UpdateManifestItem(test_extension.app_id)
+ .version(test_extension.version)
+ .hash_sha256(base::HexEncode(crypto::hash::Sha256(contents)))
+ .size(base::GetFileSize(GetCrxPath(test_extension.crx_path))
+ .value_or(0))
+ .codebase(
+ test_server_->GetURL(test_extension.app_path).spec())}));
+ response->set_content_type("application/json");
+ } else {
+ response->set_content(CreateUpdateManifest(
+ {UpdateManifestItem(test_extension.app_id)
+ .version(test_extension.version)
+ .codebase(
+ test_server_->GetURL(test_extension.app_path).spec())}));
+ response->set_content_type("text/xml");
+ }
return std::move(response);
}
if (url.GetPath() == test_extension.app_path) {
- base::FilePath test_data_dir;
- base::PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir);
std::string contents;
- base::ReadFileToString(
- test_data_dir.AppendASCII(test_extension.crx_path), &contents);
+ base::ReadFileToString(GetCrxPath(test_extension.crx_path), &contents);
auto response = std::make_unique<net::test_server::BasicHttpResponse>();
response->set_code(net::HTTP_OK);
response->set_content(contents);
diff --git a/chrome/browser/extensions/initial_external_extension_loader_browsertest.cc b/chrome/browser/extensions/initial_external_extension_loader_browsertest.cc
index dac19f8..1d81a26 100644
--- a/chrome/browser/extensions/initial_external_extension_loader_browsertest.cc
+++ b/chrome/browser/extensions/initial_external_extension_loader_browsertest.cc
@@ -10,6 +10,7 @@
#include "base/functional/callback.h"
#include "base/run_loop.h"
#include "base/strings/strcat.h"
+#include "base/strings/string_number_conversions.h"
#include "base/test/scoped_feature_list.h"
#include "base/threading/thread_restrictions.h"
#include "base/values.h"
@@ -19,12 +20,14 @@
#include "chrome/browser/extensions/external_provider_impl.h"
#include "chrome/browser/extensions/external_provider_manager.h"
#include "chrome/browser/extensions/external_testing_loader.h"
+#include "chrome/browser/extensions/updater/chrome_update_client_config.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/common/chrome_switches.h"
#include "components/crx_file/id_util.h"
#include "components/prefs/pref_service.h"
#include "content/public/test/browser_test.h"
+#include "crypto/sha2.h"
#include "extensions/browser/extension_prefs.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/browser/pref_names.h"
@@ -32,6 +35,7 @@
#include "extensions/browser/updater/extension_downloader_test_helper.h"
#include "extensions/common/extension_urls.h"
#include "extensions/common/extensions_client.h"
+#include "extensions/common/verifier_formats.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
namespace extensions {
@@ -143,8 +147,17 @@
&manifest_template));
const GURL crx_url = embedded_test_server()->GetURL(
base::StrCat({"/", kAutoupdateDir, "/", crx_name}));
+ const base::FilePath crx =
+ temp_dir.AppendASCII(kAutoupdateDir).AppendASCII(crx_name);
+ std::string contents;
+ ASSERT_TRUE(base::ReadFileToString(crx, &contents));
+ const std::string hash =
+ base::HexEncode(crypto::SHA256HashString(contents));
+ std::optional<int64_t> size = GetFileSize(crx);
+ ASSERT_TRUE(size);
const std::string manifest = base::ReplaceStringPlaceholders(
- manifest_template, {crx_url.spec()}, /*offsets=*/nullptr);
+ manifest_template, {crx_url.spec(), hash, base::NumberToString(*size)},
+ /*offsets=*/nullptr);
ASSERT_TRUE(base::CreateDirectory(temp_dir.AppendASCII(kAutoupdateDir)));
ASSERT_TRUE(base::WriteFile(
@@ -189,7 +202,7 @@
temp_dir_.GetPath(), /*source_dir_name=*/"v2", /*crx_name=*/"v2.crx"));
ASSERT_NO_FATAL_FAILURE(SetUpExtensionUpdateResponse(
temp_dir_.GetPath(), /*crx_name=*/"v2.crx",
- /*manifest_template_name=*/"manifest_v2.xml.template"));
... (truncated)
Original Bug Report
Potential Publisher Proof Bypass for Initial Installs of Webstore Extensions
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 without the security team.
Overview: When extensions are initially installed via Sync or Enterprise Policy, Chrome falls back to using ExtensionDownloader. A logic flaw in how ExtensionDownloader identifies Webstore URLs causes it to skip the requirement for a Google Publisher Proof. An attacker who compromises a developer’s private key and can MITM the CRX CDN can serve malicious updates.
Affected files:
extensions/browser/updater/extension_downloader.ccextensions/common/extension_urls.ccchrome/browser/extensions/updater/extension_updater.cccomponents/crx_file/crx_verifier.cc
Estimated timestamp from git blame: 2019-08-15
Background
Chrome uses publisher proofs as a defense-in-depth mechanism to ensure that extensions originating from the Chrome Web Store (CWS) are legitimately signed by Google. This protects users even if an extension developer’s private signing key is compromised, as an attacker would still need a Google signature to push malicious updates.
The Issue
When an extension is being installed for the first time (e.g., via Chrome Sync, Enterprise Policy force-installs, or ChromeOS Kiosk mode), ExtensionUpdater::CanUseUpdateService returns false because the extension is not yet present on the system. This forces Chrome to use the legacy ExtensionDownloader component instead of the modern UpdateService.
In ExtensionDownloader::NotifyDelegateDownloadFinished (located in extensions/browser/updater/extension_downloader.cc), the code selects the CRX verification strictness level based on whether the download is from the Chrome Web Store. It performs this check by calling extension_urls::IsWebstoreUpdateUrl(fetch_data->url):
const crx_file::VerifierFormat required_format =
extension_urls::IsWebstoreUpdateUrl(fetch_data->url)
? GetWebstoreVerifierFormat(false)
: crx_format_requirement_;
However, fetch_data->url at this point holds the CRX codebase URL (the direct download URL where the actual .crx file is hosted, typically on clients2.googleusercontent.com), which is extracted from the update manifest’s <updatecheck codebase="..."> attribute.
In contrast, extension_urls::IsWebstoreUpdateUrl (in extensions/common/extension_urls.cc) strictly expects the update manifest URL (specifically, https://clients2.google.com/service/update2/crx). Because the CRX CDN host and path do not match the manifest host and path, the check returns false.
Due to this failure, the verification requirement falls back to the default crx_format_requirement_ (which is CRX3 for policy and sync installs). CRX3 requires a valid developer signature but does not require a Google Publisher Proof.
Furthermore, while the genuine CWS update manifest provides an expected package hash, the hash validation in SandboxedUnpacker::StartWithCrx is disabled by default (requiring the --enable-crx-hash-check flag to function), offering no secondary integrity guarantee.
Potential Attack Scenario
Note: These steps describe a potential attack path derived from static analysis; a working proof-of-concept has not been run.
- Setup: An attacker compromises a legitimate extension developer’s private signing key. They also position themselves to MITM the user’s connection to the Chrome extension CDN (
clients2.googleusercontent.com), e.g., via a compromised enterprise TLS proxy or BGP hijacking. - Trigger: The user’s Chrome browser initiates a fresh install of the targeted extension (e.g., logging into a new device with Sync, or receiving a new Enterprise policy).
- Bypass: The browser fetches the genuine update manifest from
clients2.google.com, which points the CRX download toclients2.googleusercontent.com. The attacker intercepts the CRX download and serves a malicious.crxfile signed with the compromised developer key. - Execution: Because
ExtensionDownloaderfails to identifyclients2.googleusercontent.comas a Webstore URL, it skips the publisher proof requirement. The hash check is also bypassed. The malicious extension is silently installed and executed.
Suggested Fix
The check in ExtensionDownloader::NotifyDelegateDownloadFinished should not rely on the potentially dynamic or redirected CRX URL to determine if publisher proof is required. Instead, it should rely on the original manifest URL (as is done during the cache hit path in TryFetchingExtensionsFromCache), or it should explicitly track whether the update check originated from the Webstore via a flag passed down from the manifest fetching stage.
Evaluated with Chrome root at commit: 137d451a126685dd5010e6609db9f6d4a78d8234
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. And please feel free to reach out to me directly if you have concerns or feedback on the project.