Medium chrome Logic Error 📄 Reporter bug report 🔧 Commit mapped

Overview

Medium
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactInappropriate implementation in Extensions
DescriptionInappropriate implementation in Extensions
ComponentExtensions
Bug ClassLogic Error
Tracker497440270
Fix commitc9de88243b26 (chromium/src) +147/-0
CISA KEVNot listed
CreditedGoogle
Disclosed2026-06-02

Changed Functions

FunctionChangeNotes
if
extensions/BUILD.gn
modified
ExtensionDownloaderRedirectTest
extensions/browser/updater/extension_downloader_unittest.cc
modified
TEST_F
extensions/browser/updater/extension_downloader_unittest.cc
modified
if
extensions/browser/updater/extension_downloader_unittest.cc
modified

Files Changed

  • extensions/BUILD.gn
  • extensions/browser/BUILD.gn
  • extensions/browser/updater/extension_downloader.cc
  • extensions/browser/updater/extension_downloader_unittest.cc
From c9de88243b26444e39bb002b5ae8d9c32450c6c5 Mon Sep 17 00:00:00 2001
From: Joshua Pawlicki <waffles@chromium.org>
Date: Wed, 15 Apr 2026 16:20:45 -0700
Subject: [PATCH] extension downloader: Remove authentication header on redirects

The Authorization header is sent to google URLs, but Google may have
open redirects that could cause an attacker in control of the download
URL to capture the header value.

The unit test added in this change is cumbersome, but I thought it
important to actually test that the callback is called and the header
removed, instead of relying on a fake that mimics the callback call.

Fixed: 497440270
Change-Id: I7a1ec48c788c780dcd5e0462dd34ec446da70bb2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7759453
Auto-Submit: Joshua Pawlicki <waffles@chromium.org>
Commit-Queue: Devlin Cronin <rdevlin.cronin@chromium.org>
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1615477}
---

diff --git a/extensions/BUILD.gn b/extensions/BUILD.gn
index e5a5193..95d18bf 100644
--- a/extensions/BUILD.gn
+++ b/extensions/BUILD.gn
@@ -314,6 +314,7 @@
 
   if (is_android) {
     deps += [
+      "//components/signin/public/android:signin_java_test_support",
       "//content/public/android:content_full_java",
       "//gin:v8_snapshot_assets",
       "//ui/android:ui_full_java",
diff --git a/extensions/browser/BUILD.gn b/extensions/browser/BUILD.gn
index 3b12f70..d827576 100644
--- a/extensions/browser/BUILD.gn
+++ b/extensions/browser/BUILD.gn
@@ -1140,6 +1140,7 @@
     "//components/safe_browsing/core/common/proto:crx_info_proto_gen",
     "//components/services/unzip:in_process",
     "//components/services/unzip/content",
+    "//components/signin/public/identity_manager:test_support",
     "//components/sync_preferences:test_support",
     "//components/update_client",
     "//components/url_matcher",
diff --git a/extensions/browser/updater/extension_downloader.cc b/extensions/browser/updater/extension_downloader.cc
index 0e52cbe..46befdd 100644
--- a/extensions/browser/updater/extension_downloader.cc
+++ b/extensions/browser/updater/extension_downloader.cc
@@ -1257,6 +1257,17 @@
       GetURLLoaderFactoryToUse(extension_loader_resource_request_->url);
   extension_loader_ = network::SimpleURLLoader::Create(
       std::move(extension_loader_resource_request_), traffic_annotation);
+
+  // Remove Authorization headers on redirect to avoid open redirect attacks.
+  extension_loader_->SetOnRedirectCallback(
+      base::BindRepeating([](const GURL& url, const net::RedirectInfo& redirect,
+                             const network::mojom::URLResponseHead& head,
+                             std::vector<std::string>* to_be_removed_headers) {
+        CHECK(to_be_removed_headers);
+        to_be_removed_headers->emplace_back(
+            net::HttpRequestHeaders::kAuthorization);
+      }));
+
   // Retry up to 3 times.
   extension_loader_->SetRetryOptions(
       3, network::SimpleURLLoader::RetryMode::RETRY_ON_NETWORK_CHANGE);
diff --git a/extensions/browser/updater/extension_downloader_unittest.cc b/extensions/browser/updater/extension_downloader_unittest.cc
index b8a7957b..d904226 100644
--- a/extensions/browser/updater/extension_downloader_unittest.cc
+++ b/extensions/browser/updater/extension_downloader_unittest.cc
@@ -4,9 +4,15 @@
 
 #include "extensions/browser/updater/extension_downloader.h"
 
+#include "base/command_line.h"
 #include "base/functional/callback_helpers.h"
+#include "base/run_loop.h"
 #include "base/task/sequenced_task_runner.h"
 #include "base/test/bind.h"
+#include "components/signin/public/identity_manager/identity_test_environment.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/storage_partition.h"
+#include "content/public/common/content_switches.h"
 #include "content/public/test/browser_task_environment.h"
 #include "content/public/test/test_utils.h"
 #include "extensions/browser/extensions_test.h"
@@ -15,6 +21,14 @@
 #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 "net/dns/mock_host_resolver.h"
+#include "net/http/http_request_headers.h"
+#include "net/test/cert_test_util.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "net/test/embedded_test_server/http_request.h"
+#include "net/test/embedded_test_server/http_response.h"
+#include "services/data_decoder/public/cpp/test_support/in_process_data_decoder.h"
 #include "services/network/test/test_utils.h"
 
 using testing::_;
@@ -798,4 +812,124 @@
   testing::Mock::VerifyAndClearExpectations(&mock_cache);
 }
 
+class ExtensionDownloaderRedirectTest : public ExtensionsTest {
+ public:
+  ExtensionDownloaderRedirectTest()
+      : ExtensionsTest(content::BrowserTaskEnvironment::IO_MAINLOOP) {}
+};
+
+TEST_F(ExtensionDownloaderRedirectTest, AuthorizationHeaderRemovedOnRedirect) {
+  // Use HTTPS server because Authorization header is only added for secure
+  // URLs.
+  net::EmbeddedTestServer test_server(net::EmbeddedTestServer::TYPE_HTTPS);
+  test_server.SetCertHostnames(
+      {"google.com", "other.com", "localhost", "127.0.0.1"});
+
+  int crx_request_count = 0;
+  bool redirect_had_auth = false;
+  bool final_had_auth = false;
+
+  test_server.RegisterRequestHandler(base::BindLambdaForTesting(
+      [&](const net::test_server::HttpRequest& request)
+          -> std::unique_ptr<net::test_server::HttpResponse> {
+        GURL url = request.GetURL();
+        if (url.path() == "/manifest") {
+          auto response =
+              std::make_unique<net::test_server::BasicHttpResponse>();
+          response->set_code(net::HTTP_OK);
+          // Point to google.com so that ExtensionDownloader is willing to use
+          // OAuth2 tokens.
+          response->set_content(CreateUpdateManifest(
+              {UpdateManifestItem(kTestExtensionId)
+                   .version("1.1")
+                   .codebase(
+                       test_server.GetURL("google.com", "/crx").spec())}));
+          response->set_content_type("text/xml");
+          return response;
+        } else if (url.path() == "/crx") {
+          crx_request_count++;
+          if (crx_request_count == 1) {
+            // First request fails with 401 to trigger OAuth2 retry.
+            auto response =
+                std::make_unique<net::test_server::BasicHttpResponse>();
+            response->set_code(net::HTTP_UNAUTHORIZED);
+            return response;
+          }
+          // Second request (retry) should have the Authorization header.
+          redirect_had_auth =
+              request.headers.contains(net::HttpRequestHeaders::kAuthorization);
+          auto response =
+              std::make_unique<net::test_server::BasicHttpResponse>();
+          response->set_code(net::HTTP_FOUND);
+          response->AddCustomHeader(
+              "Location", test_server.GetURL("other.com", "/final").spec());
+          return response;
+        } else if (url.path() == "/final") {
+          // Third request (after redirect) should NOT have the Authorization
+          // header.
+          final_had_auth =
+              request.headers.contains(net::HttpRequestHeaders::kAuthorization);
+          auto response =
+              std::make_unique<net::test_server::BasicHttpResponse>();
+          response->set_code(net::HTTP_OK);
+          response->set_content("crx_content");
+          return response;
+        }
+        return nullptr;
+      }));
+
+  ASSERT_TRUE(test_server.Start());
+
+  net::ScopedTestRoot scoped_test_root(test_server.GetRoot());
+
+  data_decoder::test::InProcessDataDecoder in_process_data_decoder;
+
+  auto host_resolver_proc =
+      base::MakeRefCounted<net::RuleBasedHostResolverProc>(nullptr);
+  host_resolver_proc->AddRule("*", "127.0.0.1");
+  net::ScopedDefaultHostResolverProc scoped_host_resolver_proc(
+      host_resolver_proc.get());
+
+  signin::IdentityTestEnvironment identity_test_env;
+  identity_test_env.MakePrimaryAccountAvailable("test@example.com",
+                                                signin::ConsentLevel::kSignin);
+  identity_test_env.SetAutomaticIssueOfAccessTokens(true);
+
+  MockExtensionDownloaderDelegate delegate;
+  ExtensionDownloader downloader(&delegate,
+                                 browser_context()
+                                     ->GetDefaultStoragePartition()
+                                     ->GetURLLoaderFactoryForBrowserProcess(),
+                                 GetTestVerifierFormat());
+  downloader.SetIdentityManager(identity_test_env.identity_manager());
+  downloader.SetBackoffPolicy(kZeroBackoffPolicy);
+
+  base::RunLoop run_loop;
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/extensions/browser/updater/extension_downloader_unittest.cc b/extensions/browser/updater/extension_downloader_unittest.cc
index b8a7957b..d904226 100644
--- a/extensions/browser/updater/extension_downloader_unittest.cc
+++ b/extensions/browser/updater/extension_downloader_unittest.cc
@@ -4,9 +4,15 @@
 
 #include "extensions/browser/updater/extension_downloader.h"
 
+#include "base/command_line.h"
 #include "base/functional/callback_helpers.h"
+#include "base/run_loop.h"
 #include "base/task/sequenced_task_runner.h"
 #include "base/test/bind.h"
+#include "components/signin/public/identity_manager/identity_test_environment.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/storage_partition.h"
+#include "content/public/common/content_switches.h"
 #include "content/public/test/browser_task_environment.h"
 #include "content/public/test/test_utils.h"
 #include "extensions/browser/extensions_test.h"
@@ -15,6 +21,14 @@
 #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 "net/dns/mock_host_resolver.h"
+#include "net/http/http_request_headers.h"
+#include "net/test/cert_test_util.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "net/test/embedded_test_server/http_request.h"
+#include "net/test/embedded_test_server/http_response.h"
+#include "services/data_decoder/public/cpp/test_support/in_process_data_decoder.h"
 #include "services/network/test/test_utils.h"
 
 using testing::_;
@@ -798,4 +812,124 @@
   testing::Mock::VerifyAndClearExpectations(&mock_cache);
 }
 
+class ExtensionDownloaderRedirectTest : public ExtensionsTest {
+ public:
+  ExtensionDownloaderRedirectTest()
+      : ExtensionsTest(content::BrowserTaskEnvironment::IO_MAINLOOP) {}
+};
+
+TEST_F(ExtensionDownloaderRedirectTest, AuthorizationHeaderRemovedOnRedirect) {
+  // Use HTTPS server because Authorization header is only added for secure
+  // URLs.
+  net::EmbeddedTestServer test_server(net::EmbeddedTestServer::TYPE_HTTPS);
+  test_server.SetCertHostnames(
+      {"google.com", "other.com", "localhost", "127.0.0.1"});
+
+  int crx_request_count = 0;
+  bool redirect_had_auth = false;
+  bool final_had_auth = false;
+
+  test_server.RegisterRequestHandler(base::BindLambdaForTesting(
+      [&](const net::test_server::HttpRequest& request)
+          -> std::unique_ptr<net::test_server::HttpResponse> {
+        GURL url = request.GetURL();
+        if (url.path() == "/manifest") {
+          auto response =
+              std::make_unique<net::test_server::BasicHttpResponse>();
+          response->set_code(net::HTTP_OK);
+          // Point to google.com so that ExtensionDownloader is willing to use
+          // OAuth2 tokens.
+          response->set_content(CreateUpdateManifest(
+              {UpdateManifestItem(kTestExtensionId)
+                   .version("1.1")
+                   .codebase(
+                       test_server.GetURL("google.com", "/crx").spec())}));
+          response->set_content_type("text/xml");
+          return response;
+        } else if (url.path() == "/crx") {
+          crx_request_count++;
+          if (crx_request_count == 1) {
+            // First request fails with 401 to trigger OAuth2 retry.
+            auto response =
+                std::make_unique<net::test_server::BasicHttpResponse>();
+            response->set_code(net::HTTP_UNAUTHORIZED);
+            return response;
+          }
+          // Second request (retry) should have the Authorization header.
+          redirect_had_auth =
+              request.headers.contains(net::HttpRequestHeaders::kAuthorization);
+          auto response =
+              std::make_unique<net::test_server::BasicHttpResponse>();
+          response->set_code(net::HTTP_FOUND);
+          response->AddCustomHeader(
+              "Location", test_server.GetURL("other.com", "/final").spec());
+          return response;
+        } else if (url.path() == "/final") {
+          // Third request (after redirect) should NOT have the Authorization
+          // header.
+          final_had_auth =
+              request.headers.contains(net::HttpRequestHeaders::kAuthorization);
+          auto response =
+              std::make_unique<net::test_server::BasicHttpResponse>();
+          response->set_code(net::HTTP_OK);
+          response->set_content("crx_content");
+          return response;
+        }
+        return nullptr;
+      }));
+
+  ASSERT_TRUE(test_server.Start());
+
+  net::ScopedTestRoot scoped_test_root(test_server.GetRoot());
+
+  data_decoder::test::InProcessDataDecoder in_process_data_decoder;
+
+  auto host_resolver_proc =
+      base::MakeRefCounted<net::RuleBasedHostResolverProc>(nullptr);
+  host_resolver_proc->AddRule("*", "127.0.0.1");
+  net::ScopedDefaultHostResolverProc scoped_host_resolver_proc(
+      host_resolver_proc.get());
+
+  signin::IdentityTestEnvironment identity_test_env;
+  identity_test_env.MakePrimaryAccountAvailable("test@example.com",
+                                                signin::ConsentLevel::kSignin);
+  identity_test_env.SetAutomaticIssueOfAccessTokens(true);
+
+  MockExtensionDownloaderDelegate delegate;
+  ExtensionDownloader downloader(&delegate,
+                                 browser_context()
+                                     ->GetDefaultStoragePartition()
+                                     ->GetURLLoaderFactoryForBrowserProcess(),
+                                 GetTestVerifierFormat());
+  downloader.SetIdentityManager(identity_test_env.identity_manager());
+  downloader.SetBackoffPolicy(kZeroBackoffPolicy);
+
+  base::RunLoop run_loop;
+
+  ExtensionDownloaderTask task(kTestExtensionId,
+                               test_server.GetURL("/manifest"),
+                               mojom::ManifestLocation::kInternal, false, 0,
+                               DownloadFetchPriority::kBackground);
+  downloader.AddPendingExtension(std::move(task));
+
+  EXPECT_CALL(delegate, IsExtensionPending(kTestExtensionId))
+      .WillRepeatedly(testing::Return(true));
+  EXPECT_CALL(delegate,
+              OnExtensionDownloadFinished_(testing::_, testing::_, testing::_,
+                                           testing::_, testing::_, testing::_))
+      .WillOnce(testing::InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
+  EXPECT_CALL(delegate,
+              OnExtensionDownloadFailed(testing::_, testing::_, testing::_,
+                                        testing::_, testing::_))
+      .WillRepeatedly(
+          testing::InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
+
+  downloader.StartAllPending(nullptr);
+  run_loop.Run();
+
+  EXPECT_EQ(crx_request_count, 2);
+  EXPECT_TRUE(redirect_had_auth);
+  EXPECT_FALSE(final_had_auth);
+}
+
 }  // namespace extensions
Loading diff…

Original Bug Report

reported by vm...@google.com

Potential OAuth2 token leak in ExtensionDownloader via open redirect

Project Fortify, an experimental security project, has identified the following potential security issue.

Overview: The ExtensionDownloader uses the attacker-controlled codebase URL from an update manifest to determine if OAuth2 tokens should be attached. By pointing this URL to a Google open redirect, an attacker can trigger credential escalation, and the resulting Authorization header is leaked to the attacker’s domain across the redirect.

Affected files:

  • extensions/browser/updater/extension_downloader.cc
  • extensions/browser/updater/safe_manifest_parser.cc

Estimated timestamp from git blame: 2021-08-23

Summary

A logic error in ExtensionDownloader allows an attacker-controlled update manifest to trigger credential escalation. The downloader decides whether to attach OAuth2 tokens based on whether the download URL (crx_url) belongs to a Google domain. Since the crx_url is provided in the codebase attribute of the update XML, an attacker can specify a *.google.com open redirect URL to bypass the intended trust boundary. Furthermore, because the downloader uses network::SimpleURLLoader without configuring it to strip sensitive headers on cross-origin redirects, the OAuth2 token is leaked to the attacker’s final destination.

Technical Details

In extensions/browser/updater/extension_downloader.cc, the logic for escalating credentials is gated by a check on the request URL:

// extensions/browser/updater/extension_downloader.cc:1425
if (fetch->url.DomainIs(kGoogleDotCom) && identity_manager_) {
  fetch->credentials = ExtensionFetch::CREDENTIALS_OAUTH2_TOKEN;
}

This fetch->url is initialized from the codebase attribute of the update manifest parsed in safe_manifest_parser.cc.

If the request fails with a 401/403, IterateFetchCredentialsAfterFailure checks the original requested URL, not the final URL. If the original URL is a Google domain, it escalates to CREDENTIALS_OAUTH2_TOKEN and retries.

During the retry, CreateExtensionLoader fetches an OAuth2 token (scoped to chromewebstore.readonly) and adds it to the request:

// extensions/browser/updater/extension_downloader.cc:1204
extension_loader_resource_request_->headers.SetHeader(
    net::HttpRequestHeaders::kAuthorization,
    base::StringPrintf("Bearer %s", access_token_.c_str()));

Crucially, ExtensionDownloader does not set an OnRedirectCallback on its SimpleURLLoader to strip the Authorization header on cross-origin redirects. The network service will transparently follow the redirect and send the sensitive header to the attacker’s domain.

Potential Attack Steps

Note: These steps are suggested based on code analysis; our tooling has not executed them to confirm exploitation.

  1. A user installs an extension (e.g., via enterprise policy or developer mode) with an update_url pointing to an attacker-controlled server.
  2. The attacker’s server responds to the update check with a manifest where the codebase attribute is an open redirect on a Google domain (e.g., https://google.com/url?q=https://attacker.com/crx).
  3. The ExtensionDownloader attempts to fetch the CRX. The Google endpoint redirects to the attacker’s server, which returns an HTTP 401 Unauthorized.
  4. IterateFetchCredentialsAfterFailure sees the 401 error and checks the original URL (google.com). It escalates the request to CREDENTIALS_OAUTH2_TOKEN.
  5. The downloader fetches an OAuth2 token and retries the request to the Google open redirect, this time with an Authorization: Bearer <token> header.
  6. The Google endpoint again redirects to the attacker’s server. Because SimpleURLLoader is not configured to remove the Authorization header, the token is sent to the attacker, leaking the user’s credentials.

Suggested Fix

  1. ExtensionDownloader should use SimpleURLLoader::SetOnRedirectCallback to explicitly remove the Authorization header (and any cookie-related headers) if the redirect destination is no longer a trusted Google domain.
  2. IterateFetchCredentialsAfterFailure should ideally inspect the final URL that served the 401/403, rather than the initial fetch->url, before deciding to attach credentials.

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.

View on issue tracker
Links in the report