From 2b49489ea2cbc9b795dfdc847e9bc1aa9b9815f2 Mon Sep 17 00:00:00 2001
From: Andrea Orru <andreaorru@chromium.org>
Date: Thu, 10 Jul 2025 11:31:11 -0700
Subject: [PATCH] [Extensions] Determine whether to bypass redirect checks per request

This change introduces the BypassRedirectChecksPerRequest feature flag,
which is enabled by default. This flag allows redirect checks to be
bypassed only if the specific request was redirected by the extensions
layer.

Bug: 379337758
Change-Id: I995620efe045ca6eba6a02c3a64e91d18188af6b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6692491
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Andrea Orru <andreaorru@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1485110}
---

diff --git a/chrome/browser/extensions/api/web_request/web_request_apitest.cc b/chrome/browser/extensions/api/web_request/web_request_apitest.cc
index 48495fa..17b7b8fb 100644
--- a/chrome/browser/extensions/api/web_request/web_request_apitest.cc
+++ b/chrome/browser/extensions/api/web_request/web_request_apitest.cc
@@ -31,6 +31,7 @@
 #include "base/test/run_until.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/test/test_future.h"
+#include "base/test/with_feature_override.h"
 #include "base/time/time.h"
 #include "base/time/time_override.h"
 #include "base/values.h"
@@ -7263,6 +7264,69 @@
   EXPECT_TRUE(navigation_observer.last_navigation_succeeded());
 }
 
+class ManifestV3WebRequestApiTestWithBypassRedirectChecksPerRequest
+    : public ManifestV3WebRequestApiTest,
+      public base::test::WithFeatureOverride {
+ public:
+  ManifestV3WebRequestApiTestWithBypassRedirectChecksPerRequest()
+      : WithFeatureOverride(features::kBypassRedirectChecksPerRequest) {}
+};
+
+// Tests service workers can't redirect to unsafe URLs when WebRequest
+// extensions are proxying requests. Regression test for crbug.com/379337758.
+IN_PROC_BROWSER_TEST_P(
+    ManifestV3WebRequestApiTestWithBypassRedirectChecksPerRequest,
+    ServiceWorkerCantRedirectToUnsafeUrls) {
+  ASSERT_TRUE(StartEmbeddedTestServer());
+
+  static constexpr char kManifest[] =
+      R"({
+           "name": "MV3 WebRequest",
+           "version": "0.1",
+           "manifest_version": 3,
+           "permissions": ["webRequest", "webRequestBlocking"],
+           "host_permissions": ["<all_urls>"],
+           "background": {"service_worker": "background.js"}
+         })";
+  // Listen to all requests to ensure they're proxied.
+  static constexpr char kBackgroundJs[] =
+      R"(chrome.webRequest.onBeforeRequest.addListener(
+             (details) => { return {}; },
+             {urls: ['<all_urls>'], types: ['main_frame']},
+             ['blocking']);
+         chrome.test.sendMessage('ready');)";
+
+  TestExtensionDir test_dir;
+  test_dir.WriteManifest(kManifest);
+  test_dir.WriteFile(FILE_PATH_LITERAL("background.js"), kBackgroundJs);
+  const Extension* extension = LoadPolicyExtension(test_dir);
+  ASSERT_TRUE(extension);
+
+  // Setup a service worker that redirects all requests to a "data:" URL.
+  content::WebContents* web_contents = GetActiveWebContents();
+  const GURL sw_url = embedded_test_server()->GetURL(
+      "/service_worker/service_worker_setup_data_redirect.html");
+  EXPECT_TRUE(NavigateToURL(sw_url));
+  EXPECT_EQ("ok", EvalJs(web_contents, "setup();"));
+
+  // Navigate to a URL that the service worker is monitoring.
+  content::TestNavigationObserver nav_observer(web_contents);
+  const GURL url = embedded_test_server()->GetURL("/service_worker/test");
+
+  const bool per_request_bypass = IsParamFeatureEnabled();
+  if (per_request_bypass) {
+    EXPECT_FALSE(NavigateToURL(url));
+    EXPECT_EQ(net::ERR_UNSAFE_REDIRECT, nav_observer.last_net_error_code());
+  } else {
+    EXPECT_TRUE(NavigateToURL(url));
+    EXPECT_EQ(net::OK, nav_observer.last_net_error_code());
+  }
+}
+
+// Toggle `features::BypassRedirectChecksPerRequest`.
+INSTANTIATE_FEATURE_OVERRIDE_TEST_SUITE(
+    ManifestV3WebRequestApiTestWithBypassRedirectChecksPerRequest);
+
 class OnAuthRequiredApiTest : public ExtensionApiTest {
  public:
   static constexpr char kTestDomain[] = "a.test";
diff --git a/chrome/test/data/service_worker/service_worker_data_redirect.js b/chrome/test/data/service_worker/service_worker_data_redirect.js
new file mode 100644
index 0000000..0c0d9df
--- /dev/null
+++ b/chrome/test/data/service_worker/service_worker_data_redirect.js
@@ -0,0 +1,8 @@
+// Copyright 2025 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+self.addEventListener('fetch', event => {
+  event.respondWith(Response.redirect(
+    'data:text/html,<script>console.log("data")</script>'));
+});
diff --git a/chrome/test/data/service_worker/service_worker_setup_data_redirect.html b/chrome/test/data/service_worker/service_worker_setup_data_redirect.html
new file mode 100644
index 0000000..61b93f6
--- /dev/null
+++ b/chrome/test/data/service_worker/service_worker_setup_data_redirect.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>Register a service worker that redirects to data: URLs</title>
+<script>
+async function setup() {
+  let script_file = 'service_worker_data_redirect.js';
+  await navigator.serviceWorker.register(script_file);
+  await navigator.serviceWorker.ready;
+  return 'ok';
+}
+</script>
diff --git a/content/browser/loader/navigation_url_loader_impl.cc b/content/browser/loader/navigation_url_loader_impl.cc
index 6fb346b..af25aee 100644
--- a/content/browser/loader/navigation_url_loader_impl.cc
+++ b/content/browser/loader/navigation_url_loader_impl.cc
@@ -72,6 +72,7 @@
 #include "content/public/browser/url_loader_throttles.h"
 #include "content/public/browser/web_ui_url_loader_factory.h"
 #include "content/public/common/content_client.h"
+#include "content/public/common/content_features.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/referrer.h"
 #include "content/public/common/url_constants.h"
@@ -1263,7 +1264,13 @@
   LogQueueTimeHistogram("Navigation.QueueTime.OnReceiveRedirect",
                         resource_request_->is_outermost_main_frame);
   net::Error error = net::OK;
-  if (!bypass_redirect_checks_ &&
+
+  bool bypass_redirect_checks =
+      base::FeatureList::IsEnabled(features::kBypassRedirectChecksPerRequest)
+          ? head->bypass_redirect_checks
+          : bypass_redirect_checks_;
+
+  if (!bypass_redirect_checks &&
       !IsSafeRedirectTarget(url_, redirect_info.new_url)) {
     error = net::ERR_UNSAFE_REDIRECT;
   } else if (--redirect_limit_ == 0) {
diff --git a/content/public/common/content_features.cc b/content/public/common/content_features.cc
index 60d0d0c4..dd32364b 100644
--- a/content/public/common/content_features.cc
+++ b/content/public/common/content_features.cc
@@ -184,6 +184,12 @@
              "BrokerFileOperationsOnDiskCacheInNetworkService",
              base::FEATURE_DISABLED_BY_DEFAULT);
 
+// Allows the decision to bypass redirect checks to be made based on the
+// specific request.
+BASE_FEATURE(kBypassRedirectChecksPerRequest,
+             "BypassRedirectChecksPerRequest",
+             base::FEATURE_ENABLED_BY_DEFAULT);
+
 // Allows pages with cache-control:no-store to enter the back/forward cache.
 // Feature params can specify whether pages with cache-control:no-store can be
 // restored if cookies change / if HTTPOnly cookies change.
diff --git a/content/public/common/content_features.h b/content/public/common/content_features.h
index f53fa32..fa304b8c 100644
--- a/content/public/common/content_features.h
+++ b/content/public/common/content_features.h
@@ -63,6 +63,7 @@
     kBlockInsecurePrivateNetworkRequestsDeprecationTrial);
 CONTENT_EXPORT BASE_DECLARE_FEATURE(
     kBrokerFileOperationsOnDiskCacheInNetworkService);
+CONTENT_EXPORT BASE_DECLARE_FEATURE(kBypassRedirectChecksPerRequest);
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kCacheControlNoStoreEnterBackForwardCache);
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kCapturedSurfaceControlKillswitch);
 CONTENT_EXPORT BASE_DECLARE_FEATURE(
diff --git a/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc b/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc
index e9a9f23..0e15df2 100644
--- a/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc
+++ b/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc
@@ -452,7 +452,19 @@
                           TRACE_ID_LOCAL(request_id_)),
       TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT);
 
-  if (redirect_url_ != redirect_info.new_url &&
+  // An extension can intercept the headers of a response and issue a redirect
+  // to a different URL. In that case `redirect_url_` was set by the proxying
+  // extension and passed to the Network Service to synthesize a redirect.

diff --git a/chrome/browser/extensions/api/web_request/web_request_apitest.cc b/chrome/browser/extensions/api/web_request/web_request_apitest.cc
index 48495fa..17b7b8fb 100644
--- a/chrome/browser/extensions/api/web_request/web_request_apitest.cc
+++ b/chrome/browser/extensions/api/web_request/web_request_apitest.cc
@@ -31,6 +31,7 @@
 #include "base/test/run_until.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/test/test_future.h"
+#include "base/test/with_feature_override.h"
 #include "base/time/time.h"
 #include "base/time/time_override.h"
 #include "base/values.h"
@@ -7263,6 +7264,69 @@
   EXPECT_TRUE(navigation_observer.last_navigation_succeeded());
 }
 
+class ManifestV3WebRequestApiTestWithBypassRedirectChecksPerRequest
+    : public ManifestV3WebRequestApiTest,
+      public base::test::WithFeatureOverride {
+ public:
+  ManifestV3WebRequestApiTestWithBypassRedirectChecksPerRequest()
+      : WithFeatureOverride(features::kBypassRedirectChecksPerRequest) {}
+};
+
+// Tests service workers can't redirect to unsafe URLs when WebRequest
+// extensions are proxying requests. Regression test for crbug.com/379337758.
+IN_PROC_BROWSER_TEST_P(
+    ManifestV3WebRequestApiTestWithBypassRedirectChecksPerRequest,
+    ServiceWorkerCantRedirectToUnsafeUrls) {
+  ASSERT_TRUE(StartEmbeddedTestServer());
+
+  static constexpr char kManifest[] =
+      R"({
+           "name": "MV3 WebRequest",
+           "version": "0.1",
+           "manifest_version": 3,
+           "permissions": ["webRequest", "webRequestBlocking"],
+           "host_permissions": ["<all_urls>"],
+           "background": {"service_worker": "background.js"}
+         })";
+  // Listen to all requests to ensure they're proxied.
+  static constexpr char kBackgroundJs[] =
+      R"(chrome.webRequest.onBeforeRequest.addListener(
+             (details) => { return {}; },
+             {urls: ['<all_urls>'], types: ['main_frame']},
+             ['blocking']);
+         chrome.test.sendMessage('ready');)";
+
+  TestExtensionDir test_dir;
+  test_dir.WriteManifest(kManifest);
+  test_dir.WriteFile(FILE_PATH_LITERAL("background.js"), kBackgroundJs);
+  const Extension* extension = LoadPolicyExtension(test_dir);
+  ASSERT_TRUE(extension);
+
+  // Setup a service worker that redirects all requests to a "data:" URL.
+  content::WebContents* web_contents = GetActiveWebContents();
+  const GURL sw_url = embedded_test_server()->GetURL(
+      "/service_worker/service_worker_setup_data_redirect.html");
+  EXPECT_TRUE(NavigateToURL(sw_url));
+  EXPECT_EQ("ok", EvalJs(web_contents, "setup();"));
+
+  // Navigate to a URL that the service worker is monitoring.
+  content::TestNavigationObserver nav_observer(web_contents);
+  const GURL url = embedded_test_server()->GetURL("/service_worker/test");
+
+  const bool per_request_bypass = IsParamFeatureEnabled();
+  if (per_request_bypass) {
+    EXPECT_FALSE(NavigateToURL(url));
+    EXPECT_EQ(net::ERR_UNSAFE_REDIRECT, nav_observer.last_net_error_code());
+  } else {
+    EXPECT_TRUE(NavigateToURL(url));
+    EXPECT_EQ(net::OK, nav_observer.last_net_error_code());
+  }
+}
+
+// Toggle `features::BypassRedirectChecksPerRequest`.
+INSTANTIATE_FEATURE_OVERRIDE_TEST_SUITE(
+    ManifestV3WebRequestApiTestWithBypassRedirectChecksPerRequest);
+
 class OnAuthRequiredApiTest : public ExtensionApiTest {
  public:
   static constexpr char kTestDomain[] = "a.test";
diff --git a/chrome/test/data/service_worker/service_worker_data_redirect.js b/chrome/test/data/service_worker/service_worker_data_redirect.js
new file mode 100644
index 0000000..0c0d9df
--- /dev/null
+++ b/chrome/test/data/service_worker/service_worker_data_redirect.js
@@ -0,0 +1,8 @@
+// Copyright 2025 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+self.addEventListener('fetch', event => {
+  event.respondWith(Response.redirect(
+    'data:text/html,<script>console.log("data")</script>'));
+});
diff --git a/chrome/test/data/service_worker/service_worker_setup_data_redirect.html b/chrome/test/data/service_worker/service_worker_setup_data_redirect.html
new file mode 100644
index 0000000..61b93f6
--- /dev/null
+++ b/chrome/test/data/service_worker/service_worker_setup_data_redirect.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>Register a service worker that redirects to data: URLs</title>
+<script>
+async function setup() {
+  let script_file = 'service_worker_data_redirect.js';
+  await navigator.serviceWorker.register(script_file);
+  await navigator.serviceWorker.ready;
+  return 'ok';
+}
+</script>