diff --git a/extensions/browser/updater/extension_downloader_unittest.cc b/extensions/browser/updater/extension_downloader_unittest.cc index b8a7957bc4c0d..e3e235ddda80a 100644 --- a/extensions/browser/updater/extension_downloader_unittest.cc +++ b/extensions/browser/updater/extension_downloader_unittest.cc @@ -15,6 +15,7 @@ #include "extensions/browser/updater/extension_downloader_types.h" #include "extensions/common/extension.h" #include "extensions/common/extension_urls.h" +#include "extensions/common/verifier_formats.h" #include "services/network/test/test_utils.h" using testing::_; @@ -798,4 +799,154 @@ TEST_F(ExtensionDownloaderTest, TestMultipleCacheAccess) { testing::Mock::VerifyAndClearExpectations(&mock_cache); } +// ----------------------------------------------------------------------------- +// POC: Publisher-proof verification bypass due to codebase-URL vs +// manifest-URL confusion in NotifyDelegateDownloadFinished(). +// +// A pending (not-yet-installed) extension whose update manifest comes from the +// real Chrome Web Store update endpoint should always be verified with +// CRX3_WITH_PUBLISHER_PROOF. The real CWS update manifest serves blobs from a +// separate CDN host (clients2.googleusercontent.com), so the *codebase* URL +// never matches the *update-manifest* URL. extension_downloader.cc:1140 checks +// IsWebstoreUpdateUrl() against the codebase URL instead of the manifest URL, +// causing the publisher-proof requirement to be silently dropped. +// +// This test reproduces the production flow exactly (manifest URL = +// clients2.google.com/service/update2/crx, codebase = +// clients2.googleusercontent.com/...) and asserts the *correct* behaviour. The +// test FAILS on current code, demonstrating the bug. +TEST_F(ExtensionDownloaderTest, POC_WebstorePublisherProofBypass) { + ExtensionDownloaderTestHelper helper; + + // The real Chrome Web Store update-manifest URL. + const GURL kCwsUpdateUrl = extension_urls::GetWebstoreUpdateUrl(); + ASSERT_TRUE(extension_urls::IsWebstoreUpdateUrl(kCwsUpdateUrl)); + + // The real CDN host that CWS update manifests point at via . Crucially, this is a *different* host+path from the + // update-manifest URL, so IsWebstoreUpdateUrl() on it returns false. + const char kCwsCodebaseUrl[] = + "https://clients2.googleusercontent.com/crx/blobs/" + "abcd/abcdefghijklmnopabcdefghijklmnop_1.crx"; + ASSERT_FALSE(extension_urls::IsWebstoreUpdateUrl(GURL(kCwsCodebaseUrl))); + + // Pending policy force-install: not yet in the registry, so production + // routes it to ExtensionDownloader::AddPendingExtension (see + // extension_updater.cc CanUseUpdateService -> false when + // GetInstalledExtension()==nullptr). + ExtensionDownloaderTask task = + CreateDownloaderTask(kTestExtensionId, kCwsUpdateUrl); + task.install_location = mojom::ManifestLocation::kExternalPolicyDownload; + + // Serve the update manifest (from clients2.google.com) and the CRX blob + // (from clients2.googleusercontent.com). The blob body is irrelevant for + // this test; we capture the verifier format chosen *before* unpacking. + helper.test_url_loader_factory().SetInterceptor( + base::BindLambdaForTesting([&](const network::ResourceRequest& request) { + if (request.url.host() == kCwsUpdateUrl.host()) { + helper.test_url_loader_factory().AddResponse( + request.url.spec(), + CreateUpdateManifest({std::make_tuple(kTestExtensionId, "1.0", + kCwsCodebaseUrl)}), + net::HTTP_OK); + } else { + helper.test_url_loader_factory().AddResponse(request.url.spec(), + "crx-bytes", + net::HTTP_OK); + } + })); + + MockExtensionDownloaderDelegate& delegate = helper.delegate(); + EXPECT_CALL(delegate, IsExtensionPending(kTestExtensionId)) + .WillRepeatedly(Return(true)); + + std::optional selected_format; + EXPECT_CALL(delegate, OnExtensionDownloadFinished_(_, _, _, _, _, _)) + .WillOnce( + [&](const CRXFileInfo& file, bool, const GURL&, + const ExtensionDownloaderDelegate::PingResult&, + const std::set&, + ExtensionDownloaderDelegate::InstallCallback&) { + selected_format = file.required_format; + }); + + helper.downloader().AddPendingExtension(std::move(task)); + helper.downloader().StartAllPending(nullptr); + content::RunAllTasksUntilIdle(); + + ASSERT_TRUE(selected_format.has_value()) + << "OnExtensionDownloadFinished was never called"; + + // CORRECT behaviour: a CRX whose update manifest came from the Chrome Web + // Store must be verified with publisher proof. + // + // ACTUAL behaviour on vulnerable code: VerifierFormat::CRX3 (publisher proof + // *not* required). This assertion FAILS, demonstrating the bypass. + EXPECT_EQ(*selected_format, GetWebstoreVerifierFormat(false)) + << "BUG: ExtensionDownloader selected VerifierFormat " + << static_cast(*selected_format) + << " (CRX3, no publisher proof) for a CRX whose update " + "manifest came from the Chrome Web Store. An attacker holding only " + "the developer key can sign a malicious CRX that passes " + "verification."; +} + +// Companion test proving the check at extension_downloader.cc:1140 is keyed on +// the *codebase* URL, not the manifest URL: if the manifest happens to point +// the codebase at a URL with the same host+path as the CWS update endpoint +// (which never happens in practice), publisher proof IS required. Together +// with the test above, this isolates the URL confusion as the root cause. +TEST_F(ExtensionDownloaderTest, + POC_WebstorePublisherProofBypass_ControlCodebaseMatchesUpdateUrl) { + ExtensionDownloaderTestHelper helper; + + const GURL kCwsUpdateUrl = extension_urls::GetWebstoreUpdateUrl(); + // Codebase intentionally set to the same host+path as the update URL (only + // the query differs). IsWebstoreUpdateUrl() ignores query, so this matches. + const std::string kCodebase = kCwsUpdateUrl.spec() + "?crxblob=1"; + ASSERT_TRUE(extension_urls::IsWebstoreUpdateUrl(GURL(kCodebase))); + + ExtensionDownloaderTask task = + CreateDownloaderTask(kTestExtensionId, kCwsUpdateUrl); + task.install_location = mojom::ManifestLocation::kExternalPolicyDownload; + + helper.test_url_loader_factory().SetInterceptor( + base::BindLambdaForTesting([&](const network::ResourceRequest& request) { + if (request.url.query() == "crxblob=1") { + helper.test_url_loader_factory().AddResponse(request.url.spec(), + "crx-bytes", + net::HTTP_OK); + } else { + helper.test_url_loader_factory().AddResponse( + request.url.spec(), + CreateUpdateManifest( + {std::make_tuple(kTestExtensionId, "1.0", kCodebase)}), + net::HTTP_OK); + } + })); + + MockExtensionDownloaderDelegate& delegate = helper.delegate(); + EXPECT_CALL(delegate, IsExtensionPending(kTestExtensionId)) + .WillRepeatedly(Return(true)); + + std::optional selected_format; + EXPECT_CALL(delegate, OnExtensionDownloadFinished_(_, _, _, _, _, _)) + .WillOnce( + [&](const CRXFileInfo& file, bool, const GURL&, + const ExtensionDownloaderDelegate::PingResult&, + const std::set&, + ExtensionDownloaderDelegate::InstallCallback&) { + selected_format = file.required_format; + }); + + helper.downloader().AddPendingExtension(std::move(task)); + helper.downloader().StartAllPending(nullptr); + content::RunAllTasksUntilIdle(); + + ASSERT_TRUE(selected_format.has_value()); + // When the codebase URL *does* match, publisher proof IS required - proving + // line 1140 keys on the codebase URL. + EXPECT_EQ(*selected_format, GetWebstoreVerifierFormat(false)); +} + } // namespace extensions