From c25b678887fda4fd84d0041857115d6ca6c1c32f Mon Sep 17 00:00:00 2001
From: Ho Cheung <hocheung@chromium.org>
Date: Fri, 28 Aug 2026 10:38:16 -0700
Subject: [PATCH] Check URL chain for SameSite bypass

Require every URL in the request chain to pass one consistent origin
access check before ignoring SameSite restrictions.

And add regression tests.

Fixed: 553155590
Change-Id: I2723ac7ca88de6f199e25a035a610e5154da372a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8300375
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Aaron Teo <hocheung@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1688135}
---

diff --git a/chrome/browser/extensions/extension_cookies_browsertest.cc b/chrome/browser/extensions/extension_cookies_browsertest.cc
index 9c750d8..3707f5b8 100644
--- a/chrome/browser/extensions/extension_cookies_browsertest.cc
+++ b/chrome/browser/extensions/extension_cookies_browsertest.cc
@@ -165,10 +165,11 @@
 // Tests for special handling of SameSite cookies for extensions:
 // A request should be treated as same-site for the purposes of SameSite
 // cookies if either
-//  1) the request initiator is an extension with access to the requested URL,
-//  2) the site_for_cookies is an extension with access to the requested URL,
-//     and the request initiator (if it exists) is same-site to the requested
-//     URL and also the extension has access to it.
+//  1) the request initiator is an extension with access to every URL in the
+//     request's URL chain,
+//  2) the site_for_cookies is an extension with access to the initiator and
+//     every URL in the request's URL chain, and the request initiator is
+//     same-site to every URL in the chain.
 // See url_loader_util::ShouldForceIgnoreSiteForCookies().
 //
 // The test fixture param is whether or not legacy SameSite semantics are
@@ -299,10 +300,12 @@
 }
 
 // Extension initiates a no-cors request to a permitted host that redirects to
-// a disallowed host => SameSite cookies are not sent on the redirected
-// request.
-IN_PROC_BROWSER_TEST_P(ExtensionSameSiteCookiesTest,
-                       ExtensionInitiatedNoCorsRedirectNotPermitted) {
+// a disallowed host => SameSite=Lax/Strict cookies are not sent on the
+// redirected request. A request must not retain the extension's SameSite
+// bypass after crossing an origin access boundary.
+IN_PROC_BROWSER_TEST_P(
+    ExtensionSameSiteCookiesTest,
+    ExtensionInitiatedNoCorsRedirectFromPermittedToNotPermitted) {
   SetCookies(ExtensionCookiesTestHelper::kNotPermittedHost);
   content::RenderFrameHost* frame = NavigateMainFrameToExtensionPage();
   GURL target_url =
@@ -315,6 +318,25 @@
   ExpectNoSameSiteCookies(cookies);
 }
 
+// Extension initiates a no-cors request to a disallowed host that redirects to
+// a permitted host => SameSite=Lax/Strict cookies are not sent on the
+// redirected request. A request must not acquire the extension's SameSite
+// bypass mid-chain.
+IN_PROC_BROWSER_TEST_P(
+    ExtensionSameSiteCookiesTest,
+    ExtensionInitiatedNoCorsRedirectFromNotPermittedToPermitted) {
+  SetCookies(ExtensionCookiesTestHelper::kPermittedHost);
+  content::RenderFrameHost* frame = NavigateMainFrameToExtensionPage();
+  GURL target_url =
+      test_server()->GetURL(ExtensionCookiesTestHelper::kPermittedHost,
+                            ExtensionCookiesTestHelper::kFetchCookiesPath);
+  GURL redirect_url =
+      test_server()->GetURL(ExtensionCookiesTestHelper::kNotPermittedHost,
+                            "/server-redirect-307?" + target_url.spec());
+  std::string cookies = helper().FetchCookiesNoCors(frame, redirect_url);
+  ExpectNoSameSiteCookies(cookies);
+}
+
 // Tests with one frame on an extension page which makes the request.
 
 // Extension is site_for_cookies, initiator and requested URL are permitted,
diff --git a/services/network/network_context_unittest.cc b/services/network/network_context_unittest.cc
index 3912074..8e3522b 100644
--- a/services/network/network_context_unittest.cc
+++ b/services/network/network_context_unittest.cc
@@ -171,6 +171,7 @@
 #include "services/network/net_log_exporter.h"
 #include "services/network/network_qualities_pref_delegate.h"
 #include "services/network/network_service.h"
+#include "services/network/network_service_network_delegate.h"
 #include "services/network/public/cpp/constants.h"
 #include "services/network/public/cpp/features.h"
 #include "services/network/public/cpp/network_service_buildflags.h"
@@ -6518,6 +6519,38 @@
   EXPECT_FALSE(state->IsDohProbeRunning());
 }
 
+TEST_F(NetworkContextTest,
+       ShouldForceIgnoreSiteForCookiesCalledForEveryRedirectHop) {
+  std::vector<std::vector<GURL>> url_chains;
+  std::unique_ptr<NetworkContext> network_context =
+      CreateContextWithParams(CreateNetworkContextParamsForTesting());
+  auto* network_delegate = static_cast<NetworkServiceNetworkDelegate*>(
+      network_context->url_request_context()->network_delegate());
+  network_delegate->SetShouldForceIgnoreSiteForCookiesCallbackForTesting(
+      base::BindLambdaForTesting([&url_chains](const net::URLRequest& request) {
+        url_chains.push_back(request.url_chain());
+      }));
+
+  net::EmbeddedTestServer test_server;
+  test_server.AddDefaultHandlers(base::FilePath());
+  ASSERT_TRUE(test_server.Start());
+  const GURL target_url = test_server.GetURL("/echo");
+  const GURL redirect_url =
+      test_server.GetURL("/server-redirect-307?" + target_url.spec());
+
+  net::TestDelegate delegate;
+  std::unique_ptr<net::URLRequest> request =
+      network_context->url_request_context()->CreateRequest(
+          redirect_url, net::DEFAULT_PRIORITY, &delegate,
+          TRAFFIC_ANNOTATION_FOR_TESTS, net::handles::kInvalidNetworkHandle);
+  request->Start();
+  delegate.RunUntilComplete();
+
+  EXPECT_THAT(delegate.request_status(), net::test::IsOk());
+  EXPECT_THAT(url_chains, Contains(ElementsAre(redirect_url)));
+  EXPECT_THAT(url_chains, Contains(ElementsAre(redirect_url, target_url)));
+}
+
 TEST_F(NetworkContextTest, PrivacyModeDisabledByDefault) {
   const GURL kURL("http://foo.com");
   const GURL kOtherURL("http://other.com");
diff --git a/services/network/network_service_network_delegate.cc b/services/network/network_service_network_delegate.cc
index 1d0afb6..e41cbdd3 100644
--- a/services/network/network_service_network_delegate.cc
+++ b/services/network/network_service_network_delegate.cc
@@ -67,6 +67,13 @@
 
 NetworkServiceNetworkDelegate::~NetworkServiceNetworkDelegate() = default;
 
+void NetworkServiceNetworkDelegate::
+    SetShouldForceIgnoreSiteForCookiesCallbackForTesting(
+        base::RepeatingCallback<void(const net::URLRequest&)> callback) {
+  should_force_ignore_site_for_cookies_callback_for_testing_ =
+      std::move(callback);
+}
+
 void NetworkServiceNetworkDelegate::MaybeTruncateReferrer(
     net::URLRequest* const request,
     const GURL& effective_url) {
@@ -297,8 +304,11 @@
 
 bool NetworkServiceNetworkDelegate::OnShouldForceIgnoreSiteForCookies(
     const net::URLRequest& request) {
+  if (should_force_ignore_site_for_cookies_callback_for_testing_) {
+    should_force_ignore_site_for_cookies_callback_for_testing_.Run(request);
+  }
   return url_loader_util::ShouldForceIgnoreSiteForCookies(
-      request.url(), request.initiator(), request.site_for_cookies(),
+      request.url_chain(), request.initiator(), request.site_for_cookies(),
       network_context_->cors_origin_access_list());
 }
 
diff --git a/services/network/network_service_network_delegate.h b/services/network/network_service_network_delegate.h
index d628564..39d6739 100644
--- a/services/network/network_service_network_delegate.h
+++ b/services/network/network_service_network_delegate.h
@@ -8,6 +8,7 @@
 #include <optional>
 
 #include "base/component_export.h"
+#include "base/functional/callback.h"
 #include "base/memory/raw_ptr.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/remote.h"
@@ -48,6 +49,10 @@
     enable_referrers_ = enable_referrers;
   }
 
+  // Sets a callback invoked by OnShouldForceIgnoreSiteForCookies().
+  void SetShouldForceIgnoreSiteForCookiesCallbackForTesting(
+      base::RepeatingCallback<void(const net::URLRequest&)> callback);
+
  private:
   // net::NetworkDelegateImpl implementation.
   int OnBeforeURLRequest(net::URLRequest* request,
@@ -127,6 +132,9 @@
   mojo::Remote<mojom::ProxyErrorClient> proxy_error_client_;
   raw_ptr<NetworkContext> network_context_;
 
+  base::RepeatingCallback<void(const net::URLRequest&)>
+      should_force_ignore_site_for_cookies_callback_for_testing_;
+
   mutable base::WeakPtrFactory<NetworkServiceNetworkDelegate> weak_ptr_factory_{
       this};
 };
diff --git a/services/network/url_loader_util.cc b/services/network/url_loader_util.cc
index 2a5cbd1..3d87779 100644
--- a/services/network/url_loader_util.cc

diff --git a/chrome/browser/extensions/extension_cookies_browsertest.cc b/chrome/browser/extensions/extension_cookies_browsertest.cc
index 9c750d8..3707f5b8 100644
--- a/chrome/browser/extensions/extension_cookies_browsertest.cc
+++ b/chrome/browser/extensions/extension_cookies_browsertest.cc
@@ -165,10 +165,11 @@
 // Tests for special handling of SameSite cookies for extensions:
 // A request should be treated as same-site for the purposes of SameSite
 // cookies if either
-//  1) the request initiator is an extension with access to the requested URL,
-//  2) the site_for_cookies is an extension with access to the requested URL,
-//     and the request initiator (if it exists) is same-site to the requested
-//     URL and also the extension has access to it.
+//  1) the request initiator is an extension with access to every URL in the
+//     request's URL chain,
+//  2) the site_for_cookies is an extension with access to the initiator and
+//     every URL in the request's URL chain, and the request initiator is
+//     same-site to every URL in the chain.
 // See url_loader_util::ShouldForceIgnoreSiteForCookies().
 //
 // The test fixture param is whether or not legacy SameSite semantics are
@@ -299,10 +300,12 @@
 }
 
 // Extension initiates a no-cors request to a permitted host that redirects to
-// a disallowed host => SameSite cookies are not sent on the redirected
-// request.
-IN_PROC_BROWSER_TEST_P(ExtensionSameSiteCookiesTest,
-                       ExtensionInitiatedNoCorsRedirectNotPermitted) {
+// a disallowed host => SameSite=Lax/Strict cookies are not sent on the
+// redirected request. A request must not retain the extension's SameSite
+// bypass after crossing an origin access boundary.
+IN_PROC_BROWSER_TEST_P(
+    ExtensionSameSiteCookiesTest,
+    ExtensionInitiatedNoCorsRedirectFromPermittedToNotPermitted) {
   SetCookies(ExtensionCookiesTestHelper::kNotPermittedHost);
   content::RenderFrameHost* frame = NavigateMainFrameToExtensionPage();
   GURL target_url =
@@ -315,6 +318,25 @@
   ExpectNoSameSiteCookies(cookies);
 }
 
+// Extension initiates a no-cors request to a disallowed host that redirects to
+// a permitted host => SameSite=Lax/Strict cookies are not sent on the
+// redirected request. A request must not acquire the extension's SameSite
+// bypass mid-chain.
+IN_PROC_BROWSER_TEST_P(
+    ExtensionSameSiteCookiesTest,
+    ExtensionInitiatedNoCorsRedirectFromNotPermittedToPermitted) {
+  SetCookies(ExtensionCookiesTestHelper::kPermittedHost);
+  content::RenderFrameHost* frame = NavigateMainFrameToExtensionPage();
+  GURL target_url =
+      test_server()->GetURL(ExtensionCookiesTestHelper::kPermittedHost,
+                            ExtensionCookiesTestHelper::kFetchCookiesPath);
+  GURL redirect_url =
+      test_server()->GetURL(ExtensionCookiesTestHelper::kNotPermittedHost,
+                            "/server-redirect-307?" + target_url.spec());
+  std::string cookies = helper().FetchCookiesNoCors(frame, redirect_url);
+  ExpectNoSameSiteCookies(cookies);
+}
+
 // Tests with one frame on an extension page which makes the request.
 
 // Extension is site_for_cookies, initiator and requested URL are permitted,
diff --git a/services/network/network_context_unittest.cc b/services/network/network_context_unittest.cc
index 3912074..8e3522b 100644
--- a/services/network/network_context_unittest.cc
+++ b/services/network/network_context_unittest.cc
@@ -171,6 +171,7 @@
 #include "services/network/net_log_exporter.h"
 #include "services/network/network_qualities_pref_delegate.h"
 #include "services/network/network_service.h"
+#include "services/network/network_service_network_delegate.h"
 #include "services/network/public/cpp/constants.h"
 #include "services/network/public/cpp/features.h"
 #include "services/network/public/cpp/network_service_buildflags.h"
@@ -6518,6 +6519,38 @@
   EXPECT_FALSE(state->IsDohProbeRunning());
 }
 
+TEST_F(NetworkContextTest,
+       ShouldForceIgnoreSiteForCookiesCalledForEveryRedirectHop) {
+  std::vector<std::vector<GURL>> url_chains;
+  std::unique_ptr<NetworkContext> network_context =
+      CreateContextWithParams(CreateNetworkContextParamsForTesting());
+  auto* network_delegate = static_cast<NetworkServiceNetworkDelegate*>(
+      network_context->url_request_context()->network_delegate());
+  network_delegate->SetShouldForceIgnoreSiteForCookiesCallbackForTesting(
+      base::BindLambdaForTesting([&url_chains](const net::URLRequest& request) {
+        url_chains.push_back(request.url_chain());
+      }));
+
+  net::EmbeddedTestServer test_server;
+  test_server.AddDefaultHandlers(base::FilePath());
+  ASSERT_TRUE(test_server.Start());
+  const GURL target_url = test_server.GetURL("/echo");
+  const GURL redirect_url =
+      test_server.GetURL("/server-redirect-307?" + target_url.spec());
+
+  net::TestDelegate delegate;
+  std::unique_ptr<net::URLRequest> request =
+      network_context->url_request_context()->CreateRequest(
+          redirect_url, net::DEFAULT_PRIORITY, &delegate,
+          TRAFFIC_ANNOTATION_FOR_TESTS, net::handles::kInvalidNetworkHandle);
+  request->Start();
+  delegate.RunUntilComplete();
+
+  EXPECT_THAT(delegate.request_status(), net::test::IsOk());
+  EXPECT_THAT(url_chains, Contains(ElementsAre(redirect_url)));
+  EXPECT_THAT(url_chains, Contains(ElementsAre(redirect_url, target_url)));
+}
+
 TEST_F(NetworkContextTest, PrivacyModeDisabledByDefault) {
   const GURL kURL("http://foo.com");
   const GURL kOtherURL("http://other.com");
diff --git a/services/network/url_loader_util_unittest.cc b/services/network/url_loader_util_unittest.cc
index 75a644fa..fc295667 100644
--- a/services/network/url_loader_util_unittest.cc
+++ b/services/network/url_loader_util_unittest.cc
@@ -192,9 +192,97 @@
   // initiator and target, and they are same-site, so the delegate path
   // should return true.
   EXPECT_TRUE(ShouldForceIgnoreSiteForCookies(
-      url_request->url(), url_request->initiator(),
+      url_request->url_chain(), url_request->initiator(),
       url_request->site_for_cookies(), origin_access_list));
 }
 
+TEST_F(ConfigureUrlRequestTest,
+       ForceIgnoreSiteForCookiesRequiresAccessToEntireUrlChain) {
+  url::ScopedSchemeRegistryForTests scoped_registry;
+  url::AddStandardScheme("chrome-extension", url::SCHEME_WITH_HOST);
+
+  const url::Origin extension_origin =
+      url::Origin::Create(GURL("chrome-extension://abc123"));
+  const GURL allowed_url("https://allowed.example/resource");
+  const GURL another_allowed_url("https://allowed.example/redirected");
+  const GURL disallowed_url("https://disallowed.example/redirect");
+
+  cors::OriginAccessList origin_access_list;
+  origin_access_list.AddAllowListEntryForOrigin(
+      extension_origin, "https", "allowed.example",
+      /*port=*/0, mojom::CorsDomainMatchMode::kDisallowSubdomains,
+      mojom::CorsPortMatchMode::kAllowAnyPort,
+      mojom::CorsOriginAccessMatchPriority::kDefaultPriority);
+
+  const net::SiteForCookies extension_site_for_cookies =
+      net::SiteForCookies::FromOrigin(extension_origin);
+  EXPECT_TRUE(ShouldForceIgnoreSiteForCookies({allowed_url}, extension_origin,
+                                              extension_site_for_cookies,
+                                              origin_access_list));
+  EXPECT_TRUE(ShouldForceIgnoreSiteForCookies(
+      {allowed_url, another_allowed_url}, extension_origin,
+      extension_site_for_cookies, origin_access_list));
+
+  // A redirect chain must not acquire the SameSite bypass when it enters the
+  // extension's allowed origins, or retain it after leaving them.
+  EXPECT_FALSE(ShouldForceIgnoreSiteForCookies(
+      {disallowed_url, allowed_url}, extension_origin,
+      extension_site_for_cookies, origin_access_list));
+  EXPECT_FALSE(ShouldForceIgnoreSiteForCookies(
+      {allowed_url, disallowed_url}, extension_origin,
+      extension_site_for_cookies, origin_access_list));
+  EXPECT_FALSE(ShouldForceIgnoreSiteForCookies(
+      {allowed_url, disallowed_url, another_allowed_url}, extension_origin,
+      extension_site_for_cookies, origin_access_list));
+  EXPECT_FALSE(ShouldForceIgnoreSiteForCookies(
+      {}, extension_origin, extension_site_for_cookies, origin_access_list));
+}
+
+TEST_F(ConfigureUrlRequestTest,
+       ForceIgnoreSiteForCookiesDoesNotCombineAuthorizationPaths) {
+  url::ScopedSchemeRegistryForTests scoped_registry;
+  url::AddStandardScheme("chrome-extension", url::SCHEME_WITH_HOST);
+
+  const url::Origin extension_origin =
+      url::Origin::Create(GURL("chrome-extension://abc123"));
+  const url::Origin web_origin =
+      url::Origin::Create(GURL("https://initiator.example"));
+  const GURL web_origin_allowed_url(
+      "https://web-origin-allowed.example/resource");
+  const GURL extension_allowed_url("https://sub.initiator.example/resource");
+  const GURL another_extension_allowed_url(
+      "https://other.initiator.example/redirected");
+
+  cors::OriginAccessList origin_access_list;
+  origin_access_list.AddAllowListEntryForOrigin(
+      web_origin, "https", "web-origin-allowed.example",
+      /*port=*/0, mojom::CorsDomainMatchMode::kDisallowSubdomains,
+      mojom::CorsPortMatchMode::kAllowAnyPort,
+      mojom::CorsOriginAccessMatchPriority::kDefaultPriority);
+  origin_access_list.AddAllowListEntryForOrigin(
+      extension_origin, "https", "initiator.example",
+      /*port=*/0, mojom::CorsDomainMatchMode::kAllowSubdomains,
+      mojom::CorsPortMatchMode::kAllowAnyPort,
+      mojom::CorsOriginAccessMatchPriority::kDefaultPriority);
+
+  const net::SiteForCookies extension_site_for_cookies =
+      net::SiteForCookies::FromOrigin(extension_origin);
+  EXPECT_TRUE(ShouldForceIgnoreSiteForCookies(
+      {web_origin_allowed_url}, web_origin, extension_site_for_cookies,
+      origin_access_list));
+  EXPECT_TRUE(ShouldForceIgnoreSiteForCookies(
+      {extension_allowed_url}, web_origin, extension_site_for_cookies,
+      origin_access_list));
+  EXPECT_TRUE(ShouldForceIgnoreSiteForCookies(
+      {extension_allowed_url, another_extension_allowed_url}, web_origin,
+      extension_site_for_cookies, origin_access_list));
+
+  // Each URL is allowed by one of the two authorization paths, but neither
+  // path covers the whole chain.
+  EXPECT_FALSE(ShouldForceIgnoreSiteForCookies(
+      {web_origin_allowed_url, extension_allowed_url}, web_origin,
+      extension_site_for_cookies, origin_access_list));
+}
+
 }  // namespace
 }  // namespace network::url_loader_util