Chrome · Navigation and Loading › Internals>Sandbox>SiteIsolation
CVE-2025-13992
Info Disclosure in Navigation and Loading (Internals>Sandbox>SiteIsolation)
Overview
Medium
Severity
4.7
CVSS
No
Exploited ITW
Fixed
Fix Status
Background
- `CommitNavigationParams`
- the
blink::mojom::CommitNavigationParamsPtrbundle of navigation state (includingredirectsandredirect_infos) that the browser process sends to the renderer to commit a navigation. - `redirects` / `redirect_infos`
- parallel vectors describing a redirect chain:
redirectsholds theGURLs traversed, whileredirect_infosholdsnet::RedirectInfoentries whose last element is the actual URL being committed. - `DeprecatedGetOriginAsURL`
- a
GURLmethod that reduces a URL to just its origin (scheme, host, port), discarding path, query, and fragment. - `kSanitizeRedirectUrlsDuringNavigation`
- the feature flag gating whether redirect URLs are stripped to their origins before being sent cross-process.
Root Cause Analysis
Before the fix, the sanitization loops in the commit path iterated by value (for (auto redirect : commit_params->redirect_infos) and for (auto redirect : commit_params->redirects)), so each assignment of DeprecatedGetOriginAsURL() mutated a throwaway local copy that was discarded at the end of the loop body. As the commit message states, the original CL was effectively a no-op: the underlying commit_params vectors were never modified, so the full redirect URLs — including paths and query parameters that can carry sensitive data — were still shipped to the renderer.
The fix extracts the logic into NavigationRequest::SanitizeRedirectsForCommit, which iterates redirects by reference (for (GURL& redirect : ...)) so the sanitization actually persists. It additionally corrects a scope error: for redirect_infos it sanitizes only redirect_infos_span.first(size() - 1), deliberately preserving the last entry because that is the real URL that must be committed.
The result is that intermediate redirect URLs are genuinely reduced to origins before crossing the process boundary, while the committed URL is left intact.
Key insight
The original “fix” iterated the redirect vectors by value, so origin-stripping was applied to discarded copies and never took effect; the patch iterates by reference (and correctly excludes the final commit URL) so sanitization actually mutates the parameters sent to the renderer.
Attack Path
- Set up a redirecting endpoint An attacker arranges a navigation that passes through one or more redirect hops whose URLs embed sensitive path/query data (e.g., tokens or identifiers).
- Trigger a cross-site commit The navigation ultimately commits in a renderer process that is cross-site to the redirect URLs, so those URLs would ordinarily be withheld from that process.
- Ineffective sanitization
Because the pre-patch loops operated on by-value copies,
commit_params->redirectsandcommit_params->redirect_infosretained their full URLs despite the feature being enabled. - Leak to the renderer
The unsanitized
CommitNavigationParamsare sent to the renderer viaCommitNavigation, exposing the full intermediate redirect URLs to a process that should only see their origins, enabling side-channel information disclosure. Any further use of that leaked data by malicious renderer code is(inferred).
Impact Assessment
An attacker-controlled or compromised renderer receives full intermediate redirect URLs (path and query included) that should have been reduced to bare origins, yielding a side-channel information leak in the Navigation and Loading component. The precondition is a redirecting navigation that commits into a cross-site renderer while
kSanitizeRedirectUrlsDuringNavigation is enabled but ineffective. The advisory classifies this as side-channel information disclosure; no exploited-in-the-wild status is stated, and any escalation beyond disclosure of the URL data is (inferred).Changed Functions
| Function | Change | Notes |
|---|---|---|
forcontent/browser/renderer_host/navigation_request.cc |
modified | |
TEST_Fcontent/browser/renderer_host/navigation_request_unittest.cc |
modified | |
CSPEmbeddedEnforcementUnitTestcontent/browser/renderer_host/navigation_request_unittest.cc |
modified |
Files Changed
content/browser/renderer_host/navigation_request.cccontent/browser/renderer_host/navigation_request.hcontent/browser/renderer_host/navigation_request_unittest.cc
Audit Directions
- By-value mutation loopsSearch for range-based
forloops that assign into the loop variable declaredauto x(notauto& x) expecting to modify container elements; such loops silently no-op and are especially dangerous when they perform security sanitization. - Boundary elements in sanitize-all loopsReview sanitization passes over vectors where one element (first or last) has different semantics, ensuring the intended element is preserved rather than corrupted or leaked.
- Cross-process parameter strippingAudit all data placed into
CommitNavigationParamsand similar cross-process IPC structs to confirm sensitive URL/path/query fields are actually reduced before crossing a process boundary, and that feature-flagged sanitization is verified by tests to take effect.
Patch
From 32f95890ffdc45c3f129caa32bd992d366a001b8 Mon Sep 17 00:00:00 2001 From: Nasko Oskov <[email protected]> Date: Mon, 02 Jun 2025 13:45:08 -0700 Subject: [PATCH] Fix redirect info sanitization to avoid a copy and actually sanitize. The initial fix to sanitize URLs had a flaw where it made a local copy for each iteration of the for loop, then operated on the copy and it was discarded when moving to the next item. Effectively, the original CL was a noop. This change updates the code to actually sanitize properly and also account for the fact that the last element in the redirect_infos vector is the actual URL to commit, which should not be sanitized. Bug: 40095391 Change-Id: I6c94bc9d794c98b3ef66a99231b763c40b16f412 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6512516 Commit-Queue: Nasko Oskov <[email protected]> Reviewed-by: Charlie Reis <[email protected]> Cr-Commit-Position: refs/heads/main@{#1468327} --- diff --git a/content/browser/renderer_host/navigation_request.cc b/content/browser/renderer_host/navigation_request.cc index a0cf9e8..ac5a84d 100644 --- a/content/browser/renderer_host/navigation_request.cc +++ b/content/browser/renderer_host/navigation_request.cc @@ -6718,18 +6718,11 @@ // consistently upheld condition. DUMP_WILL_BE_CHECK(commit_params->redirect_response.size() == commit_params->redirect_infos.size()); - if (base::FeatureList::IsEnabled(kSanitizeRedirectUrlsDuringNavigation)) { - // Before sending the commit parameters to the renderer process, sanitize - // the redirect URLs to avoid leaking pontentially sensitive data into - // processes which are cross-site. There is no dependency on the - // cross-site-ness, therefore just sanitize unilaterally. - for (auto redirect : commit_params->redirect_infos) { - redirect.new_url = redirect.new_url.DeprecatedGetOriginAsURL(); - } - for (auto redirect : commit_params->redirects) { - redirect = redirect.DeprecatedGetOriginAsURL(); - } - } + // Before sending the commit parameters to the renderer process, sanitize + // the redirect URLs to avoid leaking potentially sensitive data into + // processes which are cross-site. There is no dependency on the + // cross-site-ness, therefore just sanitize unilaterally. + SanitizeRedirectsForCommit(commit_params); GetRenderFrameHost()->CommitNavigation( this, std::move(common_params), std::move(commit_params), @@ -7515,6 +7508,31 @@ navigation_controller.GetEntryCount(); } +void NavigationRequest::SanitizeRedirectsForCommit( + blink::mojom::CommitNavigationParamsPtr& commit_params) { + if (!base::FeatureList::IsEnabled(kSanitizeRedirectUrlsDuringNavigation)) { + return; + } + // It is safe to convert GURL to an Origin and back in the code below because + // we only want to discard the rest of the URL (e.g., path and params). The + // actual underlying Origin is not needed, which could be inherited or opaque + // in sandbox cases. + for (GURL& redirect : commit_params->redirects) { + redirect = redirect.DeprecatedGetOriginAsURL(); + } + + // In the redirect_infos vector, the last entry is the URL we are going to + // commit after following all redirects. We should not be sanitizing it, as + // we need to commit the real URL as part of the navigation. + if (!commit_params->redirect_infos.empty()) { + auto redirect_infos_span = base::span(commit_params->redirect_infos); + for (net::RedirectInfo& redirect : + redirect_infos_span.first(redirect_infos_span.size() - 1)) { + redirect.new_url = redirect.new_url.DeprecatedGetOriginAsURL(); + } + } +} + void NavigationRequest::RendererRequestedNavigationCancellationForTesting() { OnNavigationClientDisconnected(0, ""); } diff --git a/content/browser/renderer_host/navigation_request.h b/content/browser/renderer_host/navigation_request.h index 3e84acf..e73e2a591 100644 --- a/content/browser/renderer_host/navigation_request.h +++ b/content/browser/renderer_host/navigation_request.h @@ -1722,6 +1722,7 @@ private: friend class NavigationRequestTest; + FRIEND_TEST_ALL_PREFIXES(NavigationRequestTest, SanitizeRedirectsForCommit); struct ConsoleMessage { blink::mojom::ConsoleMessageLevel level; @@ -2006,6 +2007,11 @@ // renderer process. void UpdateHistoryParamsInCommitNavigationParams(); + // Helper method to sanitize URLs for redirects before the commit IPC is sent + // to the renderer process. Must be called right before sending the IPC. + void SanitizeRedirectsForCommit( + blink::mojom::CommitNavigationParamsPtr& commit_params); + // The disconnect handler for the NavigationClient Mojo interface; used as a // signal to potentially cancel navigations, e.g. when the renderer replaces // an existing NavigationClient connection with a new one or when the renderer diff --git a/content/browser/renderer_host/navigation_request_unittest.cc b/content/browser/renderer_host/navigation_request_unittest.cc index b41ae0a..f78672b 100644 --- a/content/browser/renderer_host/navigation_request_unittest.cc +++ b/content/browser/renderer_host/navigation_request_unittest.cc @@ -35,6 +35,7 @@ #include "testing/gmock/include/gmock/gmock.h" #include "third_party/blink/public/common/features.h" #include "third_party/blink/public/common/navigation/navigation_params.h" +#include "third_party/blink/public/common/navigation/navigation_params_mojom_traits.h" #include "third_party/blink/public/common/origin_trials/scoped_test_origin_trial_policy.h" #include "third_party/blink/public/common/runtime_feature_state/runtime_feature_state_context.h" #include "third_party/blink/public/mojom/fetch/fetch_api_request.mojom.h" @@ -927,6 +928,40 @@ navigation->Commit(); } +// Test to ensure that the SanitizeRedirectsForCommit method correctly removes +// the query parameters parts of the URL that can contain sensitive information. +TEST_F(NavigationRequestTest, SanitizeRedirectsForCommit) { + const GURL start_url("https://a.com?param=1"); + const GURL url_2("https://b.com?param=2#foo"); + const GURL url_3("https://c.com?param=3"); + const GURL final_url("https://d.com?param=4"); + std::unique_ptr<NavigationSimulator> navigation = + NavigationSimulator::CreateRendererInitiated(start_url, main_test_rfh()); + navigation->Start(); + navigation->Redirect(url_2); + navigation->Redirect(url_3); + navigation->Redirect(final_url); + + NavigationRequest* request = + NavigationRequest::From(navigation->GetNavigationHandle()); + auto commit_params = request->commit_params().Clone(); + request->SanitizeRedirectsForCommit(commit_params); + + // redirect_infos contains entries for B, C, and D, but not the starting URL. + // Ensure that the full URL for D is preserved. + EXPECT_EQ(3, commit_params->redirect_infos.size()); + EXPECT_EQ(GURL("https://b.com"), commit_params->redirect_infos[0].new_url); + EXPECT_EQ(GURL("https://c.com"), commit_params->redirect_infos[1].new_url); + EXPECT_EQ(final_url, commit_params->redirect_infos[2].new_url); + + // In contrast, redirects contains A, B, and C (i.e., the starting URL but not + // the final URL). + EXPECT_EQ(3, commit_params->redirects.size()); + EXPECT_EQ(GURL("https://a.com"), commit_params->redirects[0]); + EXPECT_EQ(GURL("https://b.com"), commit_params->redirects[1]); + EXPECT_EQ(GURL("https://c.com"), commit_params->redirects[2]); +} + // Test that the required CSP of every frame is computed/inherited correctly and // that the Sec-Required-CSP header is set. class CSPEmbeddedEnforcementUnitTest : public NavigationRequestTest {
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/content/browser/renderer_host/navigation_request_unittest.cc b/content/browser/renderer_host/navigation_request_unittest.cc
index b41ae0a..f78672b 100644
--- a/content/browser/renderer_host/navigation_request_unittest.cc
+++ b/content/browser/renderer_host/navigation_request_unittest.cc
@@ -35,6 +35,7 @@
#include "testing/gmock/include/gmock/gmock.h"
#include "third_party/blink/public/common/features.h"
#include "third_party/blink/public/common/navigation/navigation_params.h"
+#include "third_party/blink/public/common/navigation/navigation_params_mojom_traits.h"
#include "third_party/blink/public/common/origin_trials/scoped_test_origin_trial_policy.h"
#include "third_party/blink/public/common/runtime_feature_state/runtime_feature_state_context.h"
#include "third_party/blink/public/mojom/fetch/fetch_api_request.mojom.h"
@@ -927,6 +928,40 @@
navigation->Commit();
}
+// Test to ensure that the SanitizeRedirectsForCommit method correctly removes
+// the query parameters parts of the URL that can contain sensitive information.
+TEST_F(NavigationRequestTest, SanitizeRedirectsForCommit) {
+ const GURL start_url("https://a.com?param=1");
+ const GURL url_2("https://b.com?param=2#foo");
+ const GURL url_3("https://c.com?param=3");
+ const GURL final_url("https://d.com?param=4");
+ std::unique_ptr<NavigationSimulator> navigation =
+ NavigationSimulator::CreateRendererInitiated(start_url, main_test_rfh());
+ navigation->Start();
+ navigation->Redirect(url_2);
+ navigation->Redirect(url_3);
+ navigation->Redirect(final_url);
+
+ NavigationRequest* request =
+ NavigationRequest::From(navigation->GetNavigationHandle());
+ auto commit_params = request->commit_params().Clone();
+ request->SanitizeRedirectsForCommit(commit_params);
+
+ // redirect_infos contains entries for B, C, and D, but not the starting URL.
+ // Ensure that the full URL for D is preserved.
+ EXPECT_EQ(3, commit_params->redirect_infos.size());
+ EXPECT_EQ(GURL("https://b.com"), commit_params->redirect_infos[0].new_url);
+ EXPECT_EQ(GURL("https://c.com"), commit_params->redirect_infos[1].new_url);
+ EXPECT_EQ(final_url, commit_params->redirect_infos[2].new_url);
+
+ // In contrast, redirects contains A, B, and C (i.e., the starting URL but not
+ // the final URL).
+ EXPECT_EQ(3, commit_params->redirects.size());
+ EXPECT_EQ(GURL("https://a.com"), commit_params->redirects[0]);
+ EXPECT_EQ(GURL("https://b.com"), commit_params->redirects[1]);
+ EXPECT_EQ(GURL("https://c.com"), commit_params->redirects[2]);
+}
+
// Test that the required CSP of every frame is computed/inherited correctly and
// that the Sec-Required-CSP header is set.
class CSPEmbeddedEnforcementUnitTest : public NavigationRequestTest {
Loading diff…
Original Bug Report
reported by [email protected]
Security: site isolation bypass: Cross-origin URL disclosure via OnReceiveRedirect
When a redirect happens when loading resources, the following function gets called in the renderer process via a mojo callback: https://cs.chromium.org/chromium/src/content/common/throttling_url_loader.cc?l=547&rcl=148c4ab91ab47341c2532adc3dbce87ad65e5550 so the renderer process gets the full redirect URL as well as response headers (though apparently not all of them, e.g. I don’t see ‘Set-Cookie’ header)
Leaking a redirect URL is potentially interesting for attackers, e.g.
- IIRC Oauth2 is going to set authorization code via a redirect
- Some google properties (unrelated to Oauth2) also set secrets via redirect URLs, for example https://accounts.google.com/ServiceLogin?service=mail&passive=true&rm=false&continue=https://mail.google.com/mail/&ss=1&scc=1<mpl=default<mplcache=2&emr=1&osid=1. IIRC this is used to set osid cookie on mail.google.com
To reproduce
-
set a breakpoint or modify ResourceDispatcher::OnReceivedRedirect to print out received redirect url / headers
-
load <img src=“https://mail.google.com/mail/&ss=1&scc=1<mpl=default<mplcache=2&emr=1&osid=1. IIRC this is used to set osid cookie on mail.google.com”> while logged into gmail
- https://accounts.google.com/ServiceLogin?service=mail&passive=true&rm=false&continue=https://mail.google.com/mail/&ss=1&scc=1<mpl=default<mplcache=2&emr=1&osid=1
- https://cs.chromium.org/chromium/src/content/common/throttling_url_loader.cc?l=547&rcl=148c4ab91ab47341c2532adc3dbce87ad65e5550
- https://mail.google.com/mail/&ss=1&scc=1<mpl=default<mplcache=2&emr=1&osid=1
References
On This Page