PatchGap
Chrome · Extensions

CVE-2025-3069

Logic Error in Extensions
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
Tracker40060076
Fix commitce0f402f288d (chromium/src) +260/-17
CISA KEVNot listed
CreditedNDevTK
Disclosed2025-04-01

Changed Functions

FunctionChangeNotes
WebAccessibleResourcesBrowserRedirectTest
chrome/browser/extensions/web_accessible_resources_browsertest.cc
 modified
WebAccessibleResourcesBrowserRedirectTest
chrome/browser/extensions/web_accessible_resources_browsertest.cc
 modified
IN_PROC_BROWSER_TEST_F
chrome/browser/extensions/web_accessible_resources_browsertest.cc
 modified
IN_PROC_BROWSER_TEST_P
chrome/browser/extensions/web_accessible_resources_browsertest.cc
 modified

Files Changed

  • chrome/browser/extensions/web_accessible_resources_browsertest.cc
  • chrome/browser/profiles/profile_keyed_service_browsertest.cc
  • chrome/test/data/extensions/api_test/webrequest/test_redirects/manifest.json
  • extensions/browser/BUILD.gn
  • extensions/browser/api/web_request/extension_web_request_event_router.cc

Patch

view commit
From ce0f402f288d3f0aacfdd843f57d8839613b70db Mon Sep 17 00:00:00 2001
From: Solomon Kinard <solomonkinard@chromium.org>
Date: Wed, 05 Feb 2025 06:54:39 -0800
Subject: [PATCH] Extensions: WAR: Prevent server redirect to non web accessible resources

Doc:
https://docs.google.com/document/d/1ALcxHF2m85pqxEtJVQ_shHqzlIBpr3747ceSDuw7E_w/edit?usp=sharing

Fixed: chromium:40060076
Change-Id: I0440d9556ccc793d1963e434c1bb4bd097745b2e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6160996
Commit-Queue: Solomon Kinard <solomonkinard@chromium.org>
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1416132}
---

diff --git a/chrome/browser/extensions/web_accessible_resources_browsertest.cc b/chrome/browser/extensions/web_accessible_resources_browsertest.cc
index 6d88f39..8e7448a 100644
--- a/chrome/browser/extensions/web_accessible_resources_browsertest.cc
+++ b/chrome/browser/extensions/web_accessible_resources_browsertest.cc
@@ -461,9 +461,18 @@
 // TODO(crbug.com/390687767): Port to desktop Android. Currently the redirect
 // doesn't happen.
 class WebAccessibleResourcesBrowserRedirectTest
-    : public WebAccessibleResourcesBrowserTest {
+    : public WebAccessibleResourcesBrowserTest,
+      public testing::WithParamInterface<bool> {
+ public:
+  WebAccessibleResourcesBrowserRedirectTest() {
+    feature_list_.InitWithFeatureState(
+        extensions_features::kExtensionWARForRedirect, GetParam());
+  }
+
  protected:
-  void TestBrowserRedirect(const char* kManifest, const char* kHistogramName) {
+  void TestBrowserRedirect(const char* kManifest,
+                           const char* kHistogramName,
+                           bool is_war_for_redirect_enabled) {
     // Load extension.
     TestExtensionDir test_dir;
     test_dir.WriteManifest(kManifest);
@@ -496,10 +505,12 @@
 
     // Test cases.
     server_redirect(net::OK, "web_accessible_resource.html", true);
-    server_redirect(net::OK, "resource.html", false);
+    server_redirect(
+        is_war_for_redirect_enabled ? net::ERR_BLOCKED_BY_CLIENT : net::OK,
+        "resource.html", false);
   }
 
-  void TestBrowserRedirectMV2() {
+  void TestBrowserRedirectMV2(bool is_war_for_redirect_enabled) {
     TestBrowserRedirect(
         R"({
           "name": "Test browser redirect",
@@ -507,10 +518,10 @@
           "manifest_version": 2,
           "web_accessible_resources": ["web_accessible_resource.html"]
         })",
-        "Extensions.WAR.XOriginWebAccessible.MV2");
+        "Extensions.WAR.XOriginWebAccessible.MV2", is_war_for_redirect_enabled);
   }
 
-  void TestBrowserRedirectMV3() {
+  void TestBrowserRedirectMV3(bool is_war_for_redirect_enabled) {
     TestBrowserRedirect(
         R"({
           "name": "Redirect Test",
@@ -523,15 +534,24 @@
             }
           ]
         })",
-        "Extensions.WAR.XOriginWebAccessible.MV3");
+        "Extensions.WAR.XOriginWebAccessible.MV3", is_war_for_redirect_enabled);
   }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
 };
 
+INSTANTIATE_TEST_SUITE_P(All,
+                         WebAccessibleResourcesBrowserRedirectTest,
+                         testing::Bool());
+
 // Test server redirect to a web accessible or extension resource.
-IN_PROC_BROWSER_TEST_F(WebAccessibleResourcesBrowserRedirectTest, Manifests) {
-  TestBrowserRedirectMV2();
-  TestBrowserRedirectMV3();
+IN_PROC_BROWSER_TEST_P(WebAccessibleResourcesBrowserRedirectTest, Manifests) {
+  bool is_war_for_redirect_enabled = GetParam();
+  TestBrowserRedirectMV2(is_war_for_redirect_enabled);
+  TestBrowserRedirectMV3(is_war_for_redirect_enabled);
 }
+
 #endif  // !BUILDFLAG(IS_ANDROID)
 
 }  // namespace
diff --git a/chrome/browser/profiles/profile_keyed_service_browsertest.cc b/chrome/browser/profiles/profile_keyed_service_browsertest.cc
index 68a1b03..67697d9 100644
--- a/chrome/browser/profiles/profile_keyed_service_browsertest.cc
+++ b/chrome/browser/profiles/profile_keyed_service_browsertest.cc
@@ -376,6 +376,7 @@
     "ExtensionInstallEventRouter",
 #endif  // BUILDFLAG(ENTERPRISE_CONTENT_ANALYSIS)
     "ChromeEnterpriseRealTimeUrlLookupService",
+    "ExtensionNavigationRegistry",
     "ExtensionSystem",
     "ExtensionURLLoaderFactory::BrowserContextShutdownNotifierFactory",
     "FederatedIdentityPermissionContext",
@@ -628,6 +629,7 @@
     "HeavyAdService",
 #if BUILDFLAG(ENABLE_EXTENSIONS)
     "HidConnectionResourceManager",
+    "ExtensionNavigationRegistry",
 #endif
     "HidDeviceManager",
     "HistoryAPI",
diff --git a/chrome/test/data/extensions/api_test/webrequest/test_redirects/manifest.json b/chrome/test/data/extensions/api_test/webrequest/test_redirects/manifest.json
index 1a8c500f..e93bba06 100644
--- a/chrome/test/data/extensions/api_test/webrequest/test_redirects/manifest.json
+++ b/chrome/test/data/extensions/api_test/webrequest/test_redirects/manifest.json
@@ -8,5 +8,6 @@
   "background": {
     "scripts": ["test_redirects.js"],
     "persistent": true
-  }
+  },
+  "web_accessible_resources": ["simpleLoad/a.html"]
 }
diff --git a/extensions/browser/BUILD.gn b/extensions/browser/BUILD.gn
index a70c529..9551a25 100644
--- a/extensions/browser/BUILD.gn
+++ b/extensions/browser/BUILD.gn
@@ -312,6 +312,8 @@
     "extension_icon_manager.h",
     "extension_icon_placeholder.cc",
     "extension_icon_placeholder.h",
+    "extension_navigation_registry.cc",
+    "extension_navigation_registry.h",
     "extension_navigation_throttle.cc",
     "extension_navigation_throttle.h",
     "extension_navigation_ui_data.cc",
diff --git a/extensions/browser/api/web_request/extension_web_request_event_router.cc b/extensions/browser/api/web_request/extension_web_request_event_router.cc
index ed36da5..e98ff00 100644
--- a/extensions/browser/api/web_request/extension_web_request_event_router.cc
+++ b/extensions/browser/api/web_request/extension_web_request_event_router.cc
@@ -40,6 +40,7 @@
 #include "extensions/browser/api/web_request/web_request_time_tracker.h"
 #include "extensions/browser/api_activity_monitor.h"
 #include "extensions/browser/event_router.h"
+#include "extensions/browser/extension_navigation_registry.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extensions_browser_client.h"
 #include "extensions/browser/process_map.h"
@@ -408,6 +409,23 @@
   return dynamic_url.value_or(redirect_url);
 }
 
+// Write that extension caused redirect OnBeforeRequest, ExecuteDeltas.
+void RecordThatNavigationWasInitiatedByExtension(
+    const WebRequestInfo* request,
+    content::BrowserContext* browser_context,
+    GURL* new_url) {
+  GURL new_location = new_url ? *new_url : GURL();
+
+  // Now that the event type has been signaled, record that webRequest has
+  // intercepted this redirect.
+  if (request->navigation_id.has_value()) {
+    // Store the target url.
+    // TODO(crbug.com/40060076): Record the extension id that caused the action.
+    ExtensionNavigationRegistry::Get(browser_context)
+        ->RecordExtensionRedirect(request->navigation_id.value(), new_location);
+  }
+}
+
 using CallbacksForPageLoad = std::list<base::OnceClosure>;
 
 // TODO(crbug.com/40264286): We need to investigate why this is a global
@@ -1006,7 +1024,8 @@
                     browser_context, action.extension_id, request->url,
                     action.redirect_url.value());
           }
-
+          RecordThatNavigationWasInitiatedByExtension(request, browser_context,
+                                                      new_url);
           return net::OK;
         case DNRRequestAction::Type::MODIFY_HEADERS:
           // Unlike other actions, allow web request extensions to intercept
@@ -2462,6 +2481,9 @@
     OnDNRActionMatched(browser_context, *request, *action);
   }
 
+  RecordThatNavigationWasInitiatedByExtension(request, browser_context,
+                                              blocked_request.new_url);
+
   const bool redirected =
       blocked_request.new_url && !blocked_request.new_url->is_empty();
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/chrome/browser/extensions/web_accessible_resources_browsertest.cc b/chrome/browser/extensions/web_accessible_resources_browsertest.cc
index 6d88f39..8e7448a 100644
--- a/chrome/browser/extensions/web_accessible_resources_browsertest.cc
+++ b/chrome/browser/extensions/web_accessible_resources_browsertest.cc
@@ -461,9 +461,18 @@
 // TODO(crbug.com/390687767): Port to desktop Android. Currently the redirect
 // doesn't happen.
 class WebAccessibleResourcesBrowserRedirectTest
-    : public WebAccessibleResourcesBrowserTest {
+    : public WebAccessibleResourcesBrowserTest,
+      public testing::WithParamInterface<bool> {
+ public:
+  WebAccessibleResourcesBrowserRedirectTest() {
+    feature_list_.InitWithFeatureState(
+        extensions_features::kExtensionWARForRedirect, GetParam());
+  }
+
  protected:
-  void TestBrowserRedirect(const char* kManifest, const char* kHistogramName) {
+  void TestBrowserRedirect(const char* kManifest,
+                           const char* kHistogramName,
+                           bool is_war_for_redirect_enabled) {
     // Load extension.
     TestExtensionDir test_dir;
     test_dir.WriteManifest(kManifest);
@@ -496,10 +505,12 @@
 
     // Test cases.
     server_redirect(net::OK, "web_accessible_resource.html", true);
-    server_redirect(net::OK, "resource.html", false);
+    server_redirect(
+        is_war_for_redirect_enabled ? net::ERR_BLOCKED_BY_CLIENT : net::OK,
+        "resource.html", false);
   }
 
-  void TestBrowserRedirectMV2() {
+  void TestBrowserRedirectMV2(bool is_war_for_redirect_enabled) {
     TestBrowserRedirect(
         R"({
           "name": "Test browser redirect",
@@ -507,10 +518,10 @@
           "manifest_version": 2,
           "web_accessible_resources": ["web_accessible_resource.html"]
         })",
-        "Extensions.WAR.XOriginWebAccessible.MV2");
+        "Extensions.WAR.XOriginWebAccessible.MV2", is_war_for_redirect_enabled);
   }
 
-  void TestBrowserRedirectMV3() {
+  void TestBrowserRedirectMV3(bool is_war_for_redirect_enabled) {
     TestBrowserRedirect(
         R"({
           "name": "Redirect Test",
@@ -523,15 +534,24 @@
             }
           ]
         })",
-        "Extensions.WAR.XOriginWebAccessible.MV3");
+        "Extensions.WAR.XOriginWebAccessible.MV3", is_war_for_redirect_enabled);
   }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
 };
 
+INSTANTIATE_TEST_SUITE_P(All,
+                         WebAccessibleResourcesBrowserRedirectTest,
+                         testing::Bool());
+
 // Test server redirect to a web accessible or extension resource.
-IN_PROC_BROWSER_TEST_F(WebAccessibleResourcesBrowserRedirectTest, Manifests) {
-  TestBrowserRedirectMV2();
-  TestBrowserRedirectMV3();
+IN_PROC_BROWSER_TEST_P(WebAccessibleResourcesBrowserRedirectTest, Manifests) {
+  bool is_war_for_redirect_enabled = GetParam();
+  TestBrowserRedirectMV2(is_war_for_redirect_enabled);
+  TestBrowserRedirectMV3(is_war_for_redirect_enabled);
 }
+
 #endif  // !BUILDFLAG(IS_ANDROID)
 
 }  // namespace
diff --git a/chrome/browser/profiles/profile_keyed_service_browsertest.cc b/chrome/browser/profiles/profile_keyed_service_browsertest.cc
index 68a1b03..67697d9 100644
--- a/chrome/browser/profiles/profile_keyed_service_browsertest.cc
+++ b/chrome/browser/profiles/profile_keyed_service_browsertest.cc
@@ -376,6 +376,7 @@
     "ExtensionInstallEventRouter",
 #endif  // BUILDFLAG(ENTERPRISE_CONTENT_ANALYSIS)
     "ChromeEnterpriseRealTimeUrlLookupService",
+    "ExtensionNavigationRegistry",
     "ExtensionSystem",
     "ExtensionURLLoaderFactory::BrowserContextShutdownNotifierFactory",
     "FederatedIdentityPermissionContext",
@@ -628,6 +629,7 @@
     "HeavyAdService",
 #if BUILDFLAG(ENABLE_EXTENSIONS)
     "HidConnectionResourceManager",
+    "ExtensionNavigationRegistry",
 #endif
     "HidDeviceManager",
     "HistoryAPI",
diff --git a/chrome/test/data/extensions/api_test/webrequest/test_redirects/manifest.json b/chrome/test/data/extensions/api_test/webrequest/test_redirects/manifest.json
index 1a8c500f..e93bba06 100644
--- a/chrome/test/data/extensions/api_test/webrequest/test_redirects/manifest.json
+++ b/chrome/test/data/extensions/api_test/webrequest/test_redirects/manifest.json
@@ -8,5 +8,6 @@
   "background": {
     "scripts": ["test_redirects.js"],
     "persistent": true
-  }
+  },
+  "web_accessible_resources": ["simpleLoad/a.html"]
 }
Loading diff…

Original Bug Report

reported by nd...@protonmail.com

Prevent server redirect to non web accessible resource

Steps to reproduce the problem:
chrome.window.create({url: ‘chrome-extension://nkoccljplnhpfnfiajclkommnmllphnl/html/crosh.html?command=vmshell&args[]=’})

Problem Description:
The URL parameters args[] and command on chrome-extension://nkoccljplnhpfnfiajclkommnmllphnl/html/crosh.html
Are used in chrome.terminalPrivate.openTerminalProcess or chrome.terminalPrivate.openVmshellProcess

https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/extensions/api/terminal/terminal_private_api.cc

This allows an extension to trigger crosh and run vmshell with arguments.
Interestingly if you type a url in the omnibox a https:// resource is allowed to redirect to crosh. its treated as sec-fetch-site none not sure if its meant to be cross-origin.

Unrelated but I noticed ChromeOS leaks if files and folders exist to guest users file:///etc/passwd and file:///etc/ says ERROR_ACCESS_DENIED while file:///etc/foo says ERROR_FILE_NOT_FOUND

I understand if this is a non-issue.

Additional Comments:

**Chrome version: ** 103.0.0.0 **Channel: ** Not sure

OS: Windows

View on issue tracker
Links in the report
On This Page