Overview

Low
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactInappropriate implementation in Extensions
DescriptionInappropriate implementation in Extensions
ComponentExtensions
Bug ClassLogic Error
Tracker500551122
Fix commite585ba13ce9e (chromium/src) +567/-176
CISA KEVNot listed
CreditedGoogle
Disclosed2026-06-02

Changed Functions

FunctionChangeNotes
IN_PROC_BROWSER_TEST_F
chrome/browser/extensions/extension_disabled_ui_browsertest.cc
modified
if
chrome/browser/extensions/extension_disabled_ui_browsertest.cc
modified
FakeUpdateService
chrome/browser/extensions/extension_service_unittest.cc
modified

Files Changed

  • chrome/browser/extensions/chrome_extensions_browser_client.cc
  • chrome/browser/extensions/extension_disabled_ui_browsertest.cc
  • chrome/browser/extensions/extension_service_unittest.cc
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 @@
 
Loading diff…

Regression Test / PoC

shipped with the fix
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)
Loading diff…

Original Bug Report

reported by vm...@google.com

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.cc
  • extensions/common/extension_urls.cc
  • chrome/browser/extensions/updater/extension_updater.cc
  • components/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.

  1. 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.
  2. 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).
  3. Bypass: The browser fetches the genuine update manifest from clients2.google.com, which points the CRX download to clients2.googleusercontent.com. The attacker intercepts the CRX download and serves a malicious .crx file signed with the compromised developer key.
  4. Execution: Because ExtensionDownloader fails to identify clients2.googleusercontent.com as 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.

View on issue tracker