From 2af692fec79c2041fcd939e82a8718fa00faabe2 Mon Sep 17 00:00:00 2001
From: Jochen Eisinger <[email protected]>
Date: Wed, 24 Jun 2026 06:08:26 -0700
Subject: [PATCH] Fix same-origin bypass in FedCM NavigationInterceptor during redirects.

When a navigation encounters a redirect, NavigationRequest updates its
internal URL to the redirect target before notifying navigation
throttles. As a result, NavigationInterceptor::ProcessRequest (running
during WillRedirectRequest) would validate response headers against the
target URL instead of the redirector URL. This allowed same-origin
bypass checks.

A dangling pointer crash in the new tests was fixed by ensuring
NavigateAndCommit is called before MockFederatedAuthRequest is created,
as NavigateAndCommit destroys/replaces the document associated data
which Request holds a raw_ref to.

TAG=agy

Fixed: 511754400
Change-Id: Ic2488047915a0779e4b8b5961d8ed81aa59b6e63
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7965919
Commit-Queue: Jochen Eisinger <[email protected]>
Reviewed-by: Christian Biesinger <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1651664}
---

diff --git a/content/browser/webid/navigation_interceptor.cc b/content/browser/webid/navigation_interceptor.cc
index 281b332..5458d12 100644
--- a/content/browser/webid/navigation_interceptor.cc
+++ b/content/browser/webid/navigation_interceptor.cc
@@ -75,16 +75,22 @@
 
 NavigationThrottle::ThrottleCheckResult
 NavigationInterceptor::WillRedirectRequest() {
-  return ProcessRequest();
+  // Despite the name of this method, the request was already redirected when
+  // this invoked. This implies that the headers we're about to process came
+  // from the 2nd to last URL on the redirect chain.
+  const std::vector<GURL>& redirect_chain =
+      navigation_handle()->GetRedirectChain();
+  CHECK_GE(redirect_chain.size(), 2u);
+  return ProcessRequest(redirect_chain[redirect_chain.size() - 2]);
 }
 
 NavigationThrottle::ThrottleCheckResult
 NavigationInterceptor::WillProcessResponse() {
-  return ProcessRequest();
+  return ProcessRequest(navigation_handle()->GetURL());
 }
 
-NavigationThrottle::ThrottleCheckResult
-NavigationInterceptor::ProcessRequest() {
+NavigationThrottle::ThrottleCheckResult NavigationInterceptor::ProcessRequest(
+    const GURL& intercepted_url) {
   if (!document_.AsRenderFrameHostIfValid()) {
     // Some other navigation has happened in the meantime.
     return PROCEED;
@@ -165,7 +171,7 @@
       data_decoder::DataDecoder::ParseStructuredHeaderDictionaryIsolated(
           *connection_status_header,
           base::BindOnce(&NavigationInterceptor::OnConnectionStatusHeaderParsed,
-                         weak_ptr_factory_.GetWeakPtr()));
+                         weak_ptr_factory_.GetWeakPtr(), intercepted_url));
     } else {
       return PROCEED;
     }
@@ -173,7 +179,7 @@
     data_decoder::DataDecoder::ParseStructuredHeaderDictionaryIsolated(
         *intercept_header,
         base::BindOnce(&NavigationInterceptor::OnHeaderParsed,
-                       weak_ptr_factory_.GetWeakPtr()));
+                       weak_ptr_factory_.GetWeakPtr(), intercepted_url));
   } else {
     return PROCEED;
   }
@@ -187,6 +193,7 @@
 }
 
 void NavigationInterceptor::OnConnectionStatusHeaderParsed(
+    const GURL& intercepted_url,
     base::expected<net::structured_headers::Dictionary, std::string> result) {
   content::RenderFrameHost* rfh = document_.AsRenderFrameHostIfValid();
   if (!rfh) {
@@ -224,9 +231,8 @@
     }
 
     // The server can send this header without embedder login request.
-    if (net::SchemefulSite::IsSameSite(
-            embedder_login_request->idp_origin(),
-            url::Origin::Create(navigation_handle()->GetURL()))) {
+    if (net::SchemefulSite::IsSameSite(embedder_login_request->idp_origin(),
+                                       url::Origin::Create(intercepted_url))) {
       if (account_id == embedder_login_request->account_id()) {
         embedder_login_request->OnFederatedResultReceived(
             FederatedLoginResult::kSuccess);
@@ -242,6 +248,7 @@
 }
 
 void NavigationInterceptor::OnHeaderParsed(
+    const GURL& intercepted_url,
     base::expected<net::structured_headers::Dictionary, std::string> result) {
   content::RenderFrameHost* rfh = document_.AsRenderFrameHostIfValid();
   if (!rfh) {
@@ -260,8 +267,7 @@
   }
 
   RequestBuilder request_builder;
-  auto idp_get_params_vector =
-      request_builder.Build(navigation_handle()->GetURL(), *result);
+  auto idp_get_params_vector = request_builder.Build(intercepted_url, *result);
 
   if (!idp_get_params_vector) {
     // The header was available, parsed, but contained an invalid set of
@@ -274,7 +280,7 @@
   request_factory_.Run(rfh)->RequestToken(
       std::move(*idp_get_params_vector),
       password_manager::CredentialMediationRequirement::kOptional,
-      navigation_handle(),
+      navigation_handle(), intercepted_url,
       base::BindOnce(&NavigationInterceptor::OnTokenResponse,
                      weak_ptr_factory_.GetWeakPtr()));
 }
diff --git a/content/browser/webid/navigation_interceptor.h b/content/browser/webid/navigation_interceptor.h
index ae024a5e..34cd03e3 100644
--- a/content/browser/webid/navigation_interceptor.h
+++ b/content/browser/webid/navigation_interceptor.h
@@ -66,11 +66,13 @@
   static void MaybeCreateAndAdd(NavigationThrottleRegistry& registry);
 
  private:
-  ThrottleCheckResult ProcessRequest();
+  ThrottleCheckResult ProcessRequest(const GURL& intercepted_url);
 
   void OnHeaderParsed(
+      const GURL& intercepted_url,
       base::expected<net::structured_headers::Dictionary, std::string> result);
   void OnConnectionStatusHeaderParsed(
+      const GURL& intercepted_url,
       base::expected<net::structured_headers::Dictionary, std::string> result);
   void OnTokenResponse(
       blink::mojom::RequestTokenStatus status,
diff --git a/content/browser/webid/navigation_interceptor_unittest.cc b/content/browser/webid/navigation_interceptor_unittest.cc
index 838d463..44fdedf1 100644
--- a/content/browser/webid/navigation_interceptor_unittest.cc
+++ b/content/browser/webid/navigation_interceptor_unittest.cc
@@ -71,6 +71,7 @@
            idp_get_params,
        password_manager::CredentialMediationRequirement mediation_requirement,
        NavigationHandle* navigation_handle,
+       const GURL& intercepted_url,
        RequestTokenCallback callback),
       (override));
   MOCK_METHOD(void, CancelTokenRequest, (), (override));
@@ -269,11 +270,10 @@
           }));
 
   base::RunLoop run_loop;
-  EXPECT_CALL(*federated_auth_request.get(), RequestToken)
-      .WillOnce([&](auto, auto, auto, auto) {
-        // When RequestToken is finally called, quit the RunLoop.
-        run_loop.Quit();
-      });
+  EXPECT_CALL(*federated_auth_request.get(), RequestToken).WillOnce([&]() {
+    // When RequestToken is finally called, quit the RunLoop.
+    run_loop.Quit();
+  });
 
   interceptor.WillStartRequest();
   auto result = interceptor.WillProcessResponse();
@@ -323,11 +323,10 @@
           }));
 
   base::RunLoop run_loop;
-  EXPECT_CALL(*federated_auth_request.get(), RequestToken)
-      .WillOnce([&](auto, auto, auto, auto) {
-        // When RequestToken is finally called, quit the RunLoop.
-        run_loop.Quit();
-      });
+  EXPECT_CALL(*federated_auth_request.get(), RequestToken).WillOnce([&]() {
+    // When RequestToken is finally called, quit the RunLoop.
+    run_loop.Quit();
+  });
 
   interceptor.WillStartRequest();
   auto result = interceptor.WillProcessResponse();
@@ -381,11 +380,10 @@
           }));
 
   base::RunLoop run_loop;
-  EXPECT_CALL(*federated_auth_request.get(), RequestToken)
-      .WillOnce([&](auto, auto, auto, auto) {
-        // When RequestToken is finally called, quit the RunLoop.
-        run_loop.Quit();
-      });
+  EXPECT_CALL(*federated_auth_request.get(), RequestToken).WillOnce([&]() {
+    // When RequestToken is finally called, quit the RunLoop.
+    run_loop.Quit();
+  });
 

diff --git a/content/browser/webid/navigation_interceptor_unittest.cc b/content/browser/webid/navigation_interceptor_unittest.cc
index 838d463..44fdedf1 100644
--- a/content/browser/webid/navigation_interceptor_unittest.cc
+++ b/content/browser/webid/navigation_interceptor_unittest.cc
@@ -71,6 +71,7 @@
            idp_get_params,
        password_manager::CredentialMediationRequirement mediation_requirement,
        NavigationHandle* navigation_handle,
+       const GURL& intercepted_url,
        RequestTokenCallback callback),
       (override));
   MOCK_METHOD(void, CancelTokenRequest, (), (override));
@@ -269,11 +270,10 @@
           }));
 
   base::RunLoop run_loop;
-  EXPECT_CALL(*federated_auth_request.get(), RequestToken)
-      .WillOnce([&](auto, auto, auto, auto) {
-        // When RequestToken is finally called, quit the RunLoop.
-        run_loop.Quit();
-      });
+  EXPECT_CALL(*federated_auth_request.get(), RequestToken).WillOnce([&]() {
+    // When RequestToken is finally called, quit the RunLoop.
+    run_loop.Quit();
+  });
 
   interceptor.WillStartRequest();
   auto result = interceptor.WillProcessResponse();
@@ -323,11 +323,10 @@
           }));
 
   base::RunLoop run_loop;
-  EXPECT_CALL(*federated_auth_request.get(), RequestToken)
-      .WillOnce([&](auto, auto, auto, auto) {
-        // When RequestToken is finally called, quit the RunLoop.
-        run_loop.Quit();
-      });
+  EXPECT_CALL(*federated_auth_request.get(), RequestToken).WillOnce([&]() {
+    // When RequestToken is finally called, quit the RunLoop.
+    run_loop.Quit();
+  });
 
   interceptor.WillStartRequest();
   auto result = interceptor.WillProcessResponse();
@@ -381,11 +380,10 @@
           }));
 
   base::RunLoop run_loop;
-  EXPECT_CALL(*federated_auth_request.get(), RequestToken)
-      .WillOnce([&](auto, auto, auto, auto) {
-        // When RequestToken is finally called, quit the RunLoop.
-        run_loop.Quit();
-      });
+  EXPECT_CALL(*federated_auth_request.get(), RequestToken).WillOnce([&]() {
+    // When RequestToken is finally called, quit the RunLoop.
+    run_loop.Quit();
+  });
 
   interceptor.WillStartRequest();
   auto result = interceptor.WillProcessResponse();
@@ -517,7 +515,7 @@
           }));
 
   EXPECT_CALL(*federated_auth_request.get(), RequestToken)
-      .WillOnce(WithArgs<3>(
+      .WillOnce(WithArgs<4>(
           [](blink::mojom::FederatedAuthRequest::RequestTokenCallback
                  callback) {
             std::move(callback).Run(
@@ -1066,6 +1064,152 @@
   EXPECT_TRUE(was_resumed);
 }
 
+TEST_F(NavigationInterceptorTest,
+       WillRedirectRequestWithValidSameOriginInterception) {
+  // Uses an in-process data decoder service for testing.
+  data_decoder::test::InProcessDataDecoder in_process_data_decoder;
+
+  NavigateAndCommit(GURL("https://rp.example/"));
+
+  std::unique_ptr<MockFederatedAuthRequest> federated_auth_request =
+      std::make_unique<MockFederatedAuthRequest>(
+          web_contents()->GetPrimaryMainFrame());
+  InterceptorMockNavigationHandle mock_navigation_handle(web_contents());
+  EXPECT_CALL(mock_navigation_handle, GetPreviousRenderFrameHostId)
+      .WillRepeatedly(
+          Return(web_contents()->GetPrimaryMainFrame()->GetGlobalId()));
+  mock_navigation_handle.set_render_frame_host(
+      web_contents()->GetPrimaryMainFrame());
+  mock_navigation_handle.set_is_in_primary_main_frame(true);
+
+  // Simulate a redirect: idp.example/redirect -> idp.example/login.
+  // Note that NavigationHandle::GetURL() returns the post-redirect URL
+  // when WillRedirectRequest is called.
+  mock_navigation_handle.set_url(GURL("https://idp.example/login"));
+  mock_navigation_handle.set_is_same_document(false);
+  mock_navigation_handle.set_redirect_chain(
+      {GURL("https://idp.example/redirect"),
+       GURL("https://idp.example/login")});
+
+  auto headers = base::MakeRefCounted<net::HttpResponseHeaders>("");
+  headers->AddHeader("FedCM-Intercept-Navigation",
+                     net::structured_headers::SerializeDictionary(
+                         webid::EncodeParams({
+                             {"config_url", "https://idp.example/fedcm.json"},
+                             {"client_id", "1234"},
+                         }))
+                         .value());
+  mock_navigation_handle.set_response_headers(headers);
+
+  content::MockNavigationThrottleRegistry registry(&mock_navigation_handle);
+
+  webid::NavigationInterceptor interceptor(
+      registry,
+      base::BindLambdaForTesting(
+          [&federated_auth_request](RenderFrameHost* rfh) -> Request* {
+            return federated_auth_request.get();
+          }));
+
+  base::RunLoop run_loop;
+  bool request_token_called = false;
+  EXPECT_CALL(*federated_auth_request.get(),
+              RequestToken(_, _, _, GURL("https://idp.example/redirect"), _))
+      .WillOnce([&]() {
+        request_token_called = true;
+        run_loop.Quit();
+      });
+
+  bool was_cancelled = false;
+  interceptor.set_cancel_deferred_navigation_callback_for_testing(
+      base::BindLambdaForTesting(
+          [&](NavigationThrottle::ThrottleCheckResult result) {
+            was_cancelled = true;
+            run_loop.Quit();
+          }));
+
+  interceptor.WillStartRequest();
+  auto result = interceptor.WillRedirectRequest();
+  EXPECT_EQ(result, content::NavigationThrottle::DEFER);
+
+  run_loop.Run();
+
+  EXPECT_FALSE(was_cancelled);
+  EXPECT_TRUE(request_token_called);
+}
+
+TEST_F(NavigationInterceptorTest,
+       WillRedirectRequestWithCrossOriginBypassAttempt) {
+  // Uses an in-process data decoder service for testing.
+  data_decoder::test::InProcessDataDecoder in_process_data_decoder;
+
+  NavigateAndCommit(GURL("https://rp.example/"));
+
+  std::unique_ptr<MockFederatedAuthRequest> federated_auth_request =
+      std::make_unique<MockFederatedAuthRequest>(
+          web_contents()->GetPrimaryMainFrame());
+  InterceptorMockNavigationHandle mock_navigation_handle(web_contents());
+  EXPECT_CALL(mock_navigation_handle, GetPreviousRenderFrameHostId)
+      .WillRepeatedly(
+          Return(web_contents()->GetPrimaryMainFrame()->GetGlobalId()));
+  mock_navigation_handle.set_render_frame_host(
+      web_contents()->GetPrimaryMainFrame());
+  mock_navigation_handle.set_is_in_primary_main_frame(true);
+
+  // Simulate a redirect: attacker.example -> victim.example.
+  // Note that NavigationHandle::GetURL() returns the post-redirect URL
+  // when WillRedirectRequest is called.
+  mock_navigation_handle.set_url(GURL("https://victim.example/"));
+  mock_navigation_handle.set_is_same_document(false);
+  mock_navigation_handle.set_redirect_chain(
+      {GURL("https://attacker.example/redirect"),
+       GURL("https://victim.example/")});
+
+  auto headers = base::MakeRefCounted<net::HttpResponseHeaders>("");
+  headers->AddHeader(
+      "FedCM-Intercept-Navigation",
+      net::structured_headers::SerializeDictionary(
+          webid::EncodeParams({
+              {"config_url", "https://victim.example/fedcm.json"},
+              {"client_id", "1234"},
+          }))
+          .value());
+  mock_navigation_handle.set_response_headers(headers);
+
+  content::MockNavigationThrottleRegistry registry(&mock_navigation_handle);
+
+  webid::NavigationInterceptor interceptor(
+      registry,
+      base::BindLambdaForTesting(
+          [&federated_auth_request](RenderFrameHost* rfh) -> Request* {
+            return federated_auth_request.get();
+          }));
+
+  base::RunLoop run_loop;
+  bool request_token_called = false;
+  EXPECT_CALL(*federated_auth_request.get(), RequestToken)
+      .WillRepeatedly([&]() {
+        request_token_called = true;
+        run_loop.Quit();
+      });
+
+  bool was_cancelled = false;
+  interceptor.set_cancel_deferred_navigation_callback_for_testing(
+      base::BindLambdaForTesting(
+          [&](NavigationThrottle::ThrottleCheckResult result) {
+            was_cancelled = true;
+            run_loop.Quit();
+          }));
+
+  interceptor.WillStartRequest();
+  auto result = interceptor.WillRedirectRequest();
+  EXPECT_EQ(result, content::NavigationThrottle::DEFER);
+
+  run_loop.Run();
+
+  EXPECT_TRUE(was_cancelled);
+  EXPECT_FALSE(request_token_called);
+}
+
 class EmbedderLoginNavigationInterceptorTest
     : public RenderViewHostTestHarness {
  public: