Chrome · Navigation › Navigation
CVE-2026-87541
Info Disclosure in Navigation (Navigation)
Overview
Medium
Severity
6.5
CVSS
No
Exploited ITW
Fixed
Fix Status
Background
- `SanitizeRedirectsForCommit`
- Browser-side helper in
NavigationRequestthat reduces intermediate redirect URLs to their origin before the commit IPC is sent to the renderer, to avoid leaking sensitive redirect data into a process that should not see it. - `redirect_params`
- The vector inside
CommitNavigationParamsPtrholding oneNavigationRedirectParamsPtrper redirect hop; each entry carries aredirect_info.new_urland aresponse_head(including theLocationheader). - `ComputeErrorPageProcess()` / `ErrorPageProcess::kCurrentProcess`
- Logic that decides which process a failed navigation’s error page commits in;
kCurrentProcessmeans the error page commits in the initiator’s (current) process rather than a dedicated one. - `common_params_->url`
- The final URL the navigation claims to be committing, carried in
CommonNavigationParamsPtr; historically preserved in full by the sanitizer on the assumption the receiving renderer is the one committing that URL.
Root Cause Analysis
Before the patch, SanitizeRedirectsForCommit sanitized every redirect hop except the last redirect_params entry and common_params_->url, because “we need to commit the real URL as part of the navigation” — an assumption that the renderer receiving these params is the one that will host that final URL. That assumption is broken in the error-page path: when ComputeErrorPageProcess() returns ErrorPageProcess::kCurrentProcess (a renderer-initiated navigation blocked after a redirect, with subframe error isolation off), the error page commits in the initiator’s process, which may be cross-origin to the post-redirect final URL.
As a result, the full final URL (path and query) of the cross-origin redirect target was passed to — and committed in — a process that had no right to see it, leaking sensitive data.
The fix adds a should_sanitize_final_url_for_error_page condition (feature enabled, error page in kCurrentProcess, non-empty redirect_params, and common_params->url cross-origin to GetRenderFrameHost()->GetLastCommittedOrigin()); when true it reduces common_params->url to its origin, includes the previously-exempt last redirect entry in the origin-reduction span, and evaluates Location-header sanitization against the receiving process’s origin instead of the destination URL’s origin. The change purely tightens sanitization for this specific committing-process mismatch.
Key insight
The single mistake was treating “the final committed URL” as always safe to hand to the receiving renderer, ignoring that an error page can commit in a cross-origin initiator process; the fix stops exempting the final URL from origin-reduction precisely when the committing process’s origin differs from the URL’s origin.
Attack Path
- Set up a cross-origin subframe navigation An attacker page embeds (or scripts) a renderer-initiated subframe navigation whose start URL is cross-origin to the parent/initiator, so the initiator’s process is not where the destination URL would normally commit.
- Redirect to a sensitive cross-origin target
The navigation is server-redirected to a cross-origin final URL carrying sensitive path/query data, populating a last
redirect_paramsentry andcommon_params->urlwith the full URL. - Trigger a post-redirect block into the current process
The redirected navigation is blocked (e.g.
net::ERR_BLOCKED_BY_CLIENT, as exercised by the extension and subresource-filter regression tests) soComputeErrorPageProcess()returnsErrorPageProcess::kCurrentProcessand the error page commits in the initiator’s process. - Read the leaked full URL
Because the pre-patch sanitizer left
common_params_->urland the last redirect entry (and the cross-originLocationheader) intact, the initiator process receives and commits the full post-redirect URL, observable via the committed frame URL / navigation URL (inferred: the concrete read primitive available to the attacker page is the standard committed-URL surface, which the patch itself only demonstrates through the regression tests’GetLastCommittedURL()assertions).
Impact Assessment
An attacker-controlled initiator can observe the full URL (path and query) of a cross-origin redirect target that a subframe navigation was redirected to before being blocked, disclosing information the initiator’s origin should not have — an information leak in the browser-driven navigation commit path, occurring in the initiator’s renderer process. Preconditions are a renderer-initiated subframe navigation, a redirect to a cross-origin URL, a post-redirect block that routes the error page to
kCurrentProcess, and subframe error-page isolation being off. The vendor advisory rates this medium severity and does not report in-the-wild exploitation; NVD scores it CVSS 6.5. Any escalation beyond URL disclosure is not supported by the diff (inferred: none).Changed Functions
| Function | Change | Notes |
|---|---|---|
ifcontent/browser/renderer_host/navigation_request.cc |
modified | |
forcontent/browser/renderer_host/navigation_request.cc |
modified |
Files Changed
content/common/features.hchrome/browser/subresource_filter/subresource_filter_browsertest.cccontent/browser/renderer_host/navigation_request.cccontent/browser/renderer_host/navigation_request.hchrome/browser/extensions/process_manager_browsertest.ccchrome/browser/subresource_filter/subresource_filter_fenced_frame_browsertest.cccontent/common/features.cccontent/browser/renderer_host/navigation_request_unittest.cc
Audit Directions
- Committing-process vs. URL-origin mismatchAudit other commit paths that assume the receiving renderer is the one that will host the destination URL (error pages,
about:blank/inherited origins, CSP-sandbox opaque origins) and verify sensitive URL data is not preserved into a cross-origin process. - Exempted "final" values in sanitizersWherever sanitization logic deliberately skips the last/“real” entry (as the original
redirect_params_span.first(size - 1)did), re-check that the exemption holds for every process the params can be delivered to, not just the common success case. - Redirect metadata leaksReview other fields carried through redirects (e.g.
response_headheaders beyondLocation,redirects, referrer/origin data) for full-URL or cross-origin data that reaches error-page or blocked-navigation commits in the initiator’s process.
Patch
From c600e0a60ac08d7485c6f4c0100d104ac3503d80 Mon Sep 17 00:00:00 2001
From: Camillia Smith Barnes <cammie@chromium.org>
Date: Mon, 10 Aug 2026 18:06:00 -0700
Subject: [PATCH] Navigation: Sanitize error page final URL committed in current process
SanitizeRedirectsForCommit() preserves the last redirect_infos entry and
common_params->url on the assumption that the renderer receiving them is
the one committing that URL. That does not hold for CommitErrorPage()
when ComputeErrorPageProcess() returns kCurrentProcess
(renderer-initiated navigation blocked after a redirect, subframe error
isolation off): the error page commits in the initiator's process, which
may be cross-origin to the post-redirect URL.
When committing an error page in kCurrentProcess after a redirect, also
reduce common_params_->url and the last redirect_infos entry to their
origin, behind the new kSanitizeFailedSubframeNavigationUrls feature.
Notes on Potential Regressions: Because this changes the URL that claims
to be committing, it introduces a few minor behavioral
changes/regressions:
* Error Page Strings: Some strings shown in error pages that usually
depend on the full URL will now display only the origin instead of the
full path.
* "Reload Frame" Context Menu: Subframe reloads via the "Reload Frame"
context menu option might break or stop working (e.g., if a developer
modifies a CSP that caused the block and attempts to reload just the
frame).
* Embedded Reload Buttons: Reload buttons embedded directly in error
pages could break, though these are generally hidden in subframes.
These minor regressions are considered an acceptable trade-off for the
security benefits of preventing redirect URL leaks. This change acts as
a bandaid and the regressions will be resolved when Subframe Error Page
Isolation (crbug.com/40134629) ships. The
`kSanitizeFailedSubframeNavigationUrls` feature flag acts as a kill
switch in the event of major breakage in the wild.
Bug: 517156678
Change-Id: I11f4869923b517e899fc56d2b6d4ba420ef28c53
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8178743
Reviewed-by: Charlie Reis <creis@chromium.org>
Commit-Queue: Cammie Smith Barnes <cammie@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Reviewed-by: Kelvin Jiang <kelvinjiang@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1676871}
---
diff --git a/content/browser/renderer_host/navigation_request.cc b/content/browser/renderer_host/navigation_request.cc
index f55259a4..4c1b80e 100644
--- a/content/browser/renderer_host/navigation_request.cc
+++ b/content/browser/renderer_host/navigation_request.cc
@@ -6818,7 +6818,7 @@
// 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_);
+ SanitizeRedirectsForCommit(common_params_, commit_params_);
GetRenderFrameHost()->FailedNavigation(
this, *common_params_, *commit_params_, has_stale_copy_in_cache_,
@@ -7277,7 +7277,7 @@
// 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);
+ SanitizeRedirectsForCommit(common_params, commit_params);
GetRenderFrameHost()->CommitNavigation(
this, std::move(common_params), std::move(commit_params),
@@ -8424,10 +8424,26 @@
}
void NavigationRequest::SanitizeRedirectsForCommit(
+ blink::mojom::CommonNavigationParamsPtr& common_params,
blink::mojom::CommitNavigationParamsPtr& commit_params) {
if (!base::FeatureList::IsEnabled(kSanitizeRedirectUrlsDuringNavigation)) {
return;
}
+
+ // TODO(crbug.com/40134629): Remove the sanitization once Subframe Error
+ // Pages are isolated.
+ const bool should_sanitize_final_url_for_error_page =
+ base::FeatureList::IsEnabled(
+ features::kSanitizeFailedSubframeNavigationUrls) &&
+ ComputeErrorPageProcess() == ErrorPageProcess::kCurrentProcess &&
+ !commit_params->redirect_params.empty() &&
+ !url::Origin::Create(common_params->url)
+ .IsSameOriginWith(GetRenderFrameHost()->GetLastCommittedOrigin());
+
+ if (should_sanitize_final_url_for_error_page) {
+ common_params->url = common_params->url.DeprecatedGetOriginAsURL();
+ }
+
// 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
@@ -8438,36 +8454,51 @@
// In the redirect_params vector, the last entry contains 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.
+ // we need to commit the real URL as part of the navigation. Make an exception
+ // if the error page commits in the initiator's process and the navigation was
+ // redirected before failing, in which case that final URL may be cross-origin
+ // to the receiving process and must be reduced to origin as well.
if (!commit_params->redirect_params.empty()) {
- auto redirect_params_span = base::span(commit_params->redirect_params);
- for (blink::mojom::NavigationRedirectParamsPtr& redirect :
- redirect_params_span.first(redirect_params_span.size() - 1)) {
- redirect->redirect_info.new_url =
- redirect->redirect_info.new_url.DeprecatedGetOriginAsURL();
+ base::span<blink::mojom::NavigationRedirectParamsPtr> redirect_params_span(
+ commit_params->redirect_params);
+ if (!should_sanitize_final_url_for_error_page) {
+ redirect_params_span =
+ redirect_params_span.first(redirect_params_span.size() - 1);
+ }
+ for (auto& redirect_param : redirect_params_span) {
+ redirect_param->redirect_info.new_url =
+ redirect_param->redirect_info.new_url.DeprecatedGetOriginAsURL();
}
}
if (base::FeatureList::IsEnabled(
features::kSanitizeLocationHeadersDuringNavigation)) {
- url::Origin final_origin = url::Origin::Create(common_params_->url);
+ // The expected origin of the process that will host the committed document.
+ // We use this to determine if a redirect is cross-origin to the committing
+ // process. For successful navigations, we assume the committing process
+ // will match the origin of the destination URL. For error pages that are
+ // allowed to commit in the current process, we use the current origin of
+ // the RenderFrameHost.
+ const url::Origin expected_commit_process_origin =
+ should_sanitize_final_url_for_error_page
+ ? GetRenderFrameHost()->GetLastCommittedOrigin()
+ : url::Origin::Create(common_params->url);
// Sanitize the "Location" headers for redirects that are cross-origin to
- // the final committed URL.
+ // the final committed URL (or receiving process for error pages).
// TODO(crbug.com/495463654): Consider if we need to handle cases that
// inherit an origin (e.g. about:blank), or if we cross a CSP sandbox
// boundary where the origin becomes unique/opaque.
- for (size_t i = 0; i < commit_params->redirect_params.size(); ++i) {
- auto& response_head = commit_params->redirect_params[i]->response_head;
+ for (auto& redirect_param : commit_params->redirect_params) {
+ auto& response_head = redirect_param->response_head;
if (!response_head || !response_head->headers ||
!response_head->headers->HasHeader("Location")) {
continue;
}
- const GURL& target_url =
- commit_params->redirect_params[i]->redirect_info.new_url;
+ const GURL& target_url = redirect_param->redirect_info.new_url;
const url::Origin target_origin = url::Origin::Create(target_url);
- if (!target_origin.IsSameOriginWith(final_origin)) {
+ if (!target_origin.IsSameOriginWith(expected_commit_process_origin)) {
response_head->headers->SetHeader("Location",
target_origin.GetURL().spec());
}
diff --git a/content/browser/renderer_host/navigation_request.h b/content/browser/renderer_host/navigation_request.h
index de88c2f..3130273 100644
--- a/content/browser/renderer_host/navigation_request.h
+++ b/content/browser/renderer_host/navigation_request.h
@@ -2258,7 +2258,12 @@
// 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.
+ // `common_params` is needed for determining if it's an error page that
+ // requires sanitizing the final URL.
+ // TODO(crbug.com/40134629): Remove `common_params` once Subframe Error
+ // Pages are isolated.
void SanitizeRedirectsForCommit(
+ blink::mojom::CommonNavigationParamsPtr& common_params,
blink::mojom::CommitNavigationParamsPtr& commit_params);
// The disconnect handler for the NavigationClient Mojo interface; used as a
diff --git a/content/common/features.cc b/content/common/features.cc
index 12dfbf3..5ab6aa0 100644
--- a/content/common/features.cc
+++ b/content/common/features.cc
@@ -709,6 +709,12 @@
BASE_FEATURE(kSanitizeLocationHeadersDuringNavigation,
base::FEATURE_ENABLED_BY_DEFAULT);
+// Controls whether the final URL of a failed subframe navigation is sanitized
+// when committing an error page in the initiator's process.
+// See https://crbug.com/517156678.
+BASE_FEATURE(kSanitizeFailedSubframeNavigationUrls,
+ base::FEATURE_ENABLED_BY_DEFAULT);
+
// Controls whether the `original_url` contains the full URL or just the
// sanitized origin when sent to the renderer on commit.
// See https://crbug.com/495463654.
diff --git a/content/common/features.h b/content/common/features.h
index f8cc42b..30dd52b8 100644
--- a/content/common/features.h
+++ b/content/common/features.h
@@ -225,6 +225,7 @@
CONTENT_EXPORT BASE_DECLARE_FEATURE(kScrollAfterOSKViewportShrinkFix);
#endif
CONTENT_EXPORT BASE_DECLARE_FEATURE(kSanitizeLocationHeadersDuringNavigation);
+CONTENT_EXPORT BASE_DECLARE_FEATURE(kSanitizeFailedSubframeNavigationUrls);
CONTENT_EXPORT BASE_DECLARE_FEATURE(kSanitizeOriginalUrlDuringNavigation);
CONTENT_EXPORT BASE_DECLARE_FEATURE(kSendBeaconThrowForBlobWithNonSimpleType);
CONTENT_EXPORT BASE_DECLARE_FEATURE(
diff --git a/chrome/browser/extensions/process_manager_browsertest.cc b/chrome/browser/extensions/process_manager_browsertest.cc
index 985e218b..6733a6a 100644
--- a/chrome/browser/extensions/process_manager_browsertest.cc
+++ b/chrome/browser/extensions/process_manager_browsertest.cc
@@ -1673,9 +1673,12 @@
<< "The initial navigation should be allowed, but not the server "
"redirect to extension2's manifest";
EXPECT_EQ(net::ERR_BLOCKED_BY_CLIENT, nav_observer.last_net_error_code());
- EXPECT_EQ(extension2_manifest, nav_observer.last_navigation_url());
- EXPECT_EQ(extension2_manifest,
- ChildFrameAt(main_frame, 1)->GetLastCommittedURL());
+ // We expect the URL to be sanitized, per https://crbug.com/517156678.
+ // TODO(crbug.com/40134629): Remove the sanitization once Subframe Error
+ // Page Isolation ships.
+ GURL expected_url = extension2_manifest.DeprecatedGetOriginAsURL();
+ EXPECT_EQ(expected_url, nav_observer.last_navigation_url());
+ EXPECT_EQ(expected_url, ChildFrameAt(main_frame, 1)->GetLastCommittedURL());
EXPECT_EQ(1u, pm->GetAllFrames().size());
EXPECT_EQ(1u, pm->GetRenderFrameHostsForExtension(extension1->id()).size());
EXPECT_EQ(0u, pm->GetRenderFrameHostsForExtension(extension2->id()).size());
diff --git a/chrome/browser/subresource_filter/subresource_filter_browsertest.cc b/chrome/browser/subresource_filter/subresource_filter_browsertest.cc
index 15c1c9a..403cc30 100644
--- a/chrome/browser/subresource_filter/subresource_filter_browsertest.cc
+++ b/chrome/browser/subresource_filter/subresource_filter_browsertest.cc
@@ -383,7 +383,11 @@
content::RenderFrameHost* frame = FindFrameByName(kSubframeNames[0]);
ASSERT_TRUE(frame);
- EXPECT_EQ(disallowed_subdocument_url, frame->GetLastCommittedURL());
+ // We expect the URL to be sanitized, per https://crbug.com/517156678.
+ // TODO(crbug.com/40134629): Remove the sanitization once Subframe Error Page
+ // Isolation ships.
+ EXPECT_EQ(disallowed_subdocument_url.DeprecatedGetOriginAsURL(),
+ frame->GetLastCommittedURL());
ExpectFramesIncludedInLayout(kSubframeNames, kExpectOnlySecondSubframe);
}
diff --git a/chrome/browser/subresource_filter/subresource_filter_fenced_frame_browsertest.cc b/chrome/browser/subresource_filter/subresource_filter_fenced_frame_browsertest.cc
index 36d4826..ffc1ccec 100644
--- a/chrome/browser/subresource_filter/subresource_filter_fenced_frame_browsertest.cc
+++ b/chrome/browser/subresource_filter/subresource_filter_fenced_frame_browsertest.cc
@@ -273,7 +273,11 @@
EXPECT_FALSE(WasParsedScriptElementLoaded(subframe));
EXPECT_TRUE(subframe->IsErrorDocument());
- EXPECT_EQ(kUrlWithIncludedScript, subframe->GetLastCommittedURL());
+ // We expect the URL to be sanitized, per https://crbug.com/517156678.
+ // TODO(crbug.com/40134629): Remove the sanitization once Subframe Error Page
+ // Isolation ships.
+ GURL expected_url = url::Origin::Create(kUrlWithIncludedScript).GetURL();
+ EXPECT_EQ(expected_url, subframe->GetLastCommittedURL());
}
} // namespace subresource_filter
diff --git a/content/browser/renderer_host/navigation_request_unittest.cc b/content/browser/renderer_host/navigation_request_unittest.cc
index 013e09d..0b532ad 100644
--- a/content/browser/renderer_host/navigation_request_unittest.cc
+++ b/content/browser/renderer_host/navigation_request_unittest.cc
@@ -811,8 +811,9 @@
NavigationRequest* request =
NavigationRequest::From(navigation->GetNavigationHandle());
+ auto common_params = request->common_params().Clone();
auto commit_params = request->commit_params().Clone();
- request->SanitizeRedirectsForCommit(commit_params);
+ request->SanitizeRedirectsForCommit(common_params, commit_params);
// redirect_params contains entries for B, C, and D, but not the starting URL.
// Ensure that the full URL for D is preserved.
@@ -877,9 +878,10 @@
NavigationRequest* request =
NavigationRequest::From(navigation->GetNavigationHandle());
+ auto common_params = request->common_params().Clone();
auto commit_params = request->commit_params().Clone();
- request->SanitizeRedirectsForCommit(commit_params);
+ request->SanitizeRedirectsForCommit(common_params, commit_params);
EXPECT_EQ(4u, commit_params->redirect_params.size());
@@ -955,9 +957,10 @@
NavigationRequest* request =
NavigationRequest::From(navigation->GetNavigationHandle());
+ auto common_params = request->common_params().Clone();
auto commit_params = request->commit_params().Clone();
- request->SanitizeRedirectsForCommit(commit_params);
+ request->SanitizeRedirectsForCommit(common_params, commit_params);
EXPECT_EQ(3u, commit_params->redirect_params.size());
@@ -1014,9 +1017,10 @@
NavigationRequest* request =
NavigationRequest::From(navigation->GetNavigationHandle());
+ auto common_params = request->common_params().Clone();
auto commit_params = request->commit_params().Clone();
- request->SanitizeRedirectsForCommit(commit_params);
+ request->SanitizeRedirectsForCommit(common_params, commit_params);
EXPECT_EQ(2u, commit_params->redirect_params.size());
@@ -1081,6 +1085,251 @@
EXPECT_EQ(start_url, request->original_url());
}
+// Test that when a redirected subframe navigation is blocked and the resulting
+// error page commits in the initiator's process, the final URL is reduced to
+// its origin in the parameters sent to the renderer. See crbug.com/517156678.
+TEST_F(NavigationRequestTest,
+ SanitizeRedirectsForCommitErrorPageInCurrentProcess) {
+ base::test::ScopedFeatureList feature_list;
+ feature_list.InitWithFeatures(
+ /*enabled_features=*/{features::kSanitizeFailedSubframeNavigationUrls,
+ features::kSanitizeLocationHeadersDuringNavigation},
+ /*disabled_features=*/{});
+
+ // Commit an initial page so the subframe has a parent document.
+ NavigationSimulator::NavigateAndCommitFromDocument(GURL("https://a.com/"),
+ main_test_rfh());
+ auto* child_frame = static_cast<TestRenderFrameHost*>(
+ content::RenderFrameHostTester::For(main_rfh())->AppendChild("child"));
+
+ const GURL start_url("https://b.com/start?param=1");
+ const GURL final_url("https://c.com/path?param=2");
+ std::unique_ptr<NavigationSimulator> navigation =
+ NavigationSimulator::CreateRendererInitiated(start_url, child_frame);
+ navigation->Start();
+
+ auto headers =
+ base::MakeRefCounted<net::HttpResponseHeaders>("HTTP/1.1 302 Found");
+ headers->SetHeader("Location", "https://c.com/path?param=2");
+ navigation->SetRedirectHeaders(headers);
+
+ navigation->Redirect(final_url);
+ navigation->Fail(net::ERR_BLOCKED_BY_CLIENT);
+
+ NavigationRequest* request =
+ NavigationRequest::From(navigation->GetNavigationHandle());
+ ASSERT_EQ(NavigationRequest::ErrorPageProcess::kCurrentProcess,
+ request->ComputeErrorPageProcess());
+
+ // The error page commits in the initiator's process, so the final URL (which
+ // is the post-redirect target) should be reduced to its origin in both the
+ // common and commit params.
+ EXPECT_EQ(GURL("https://c.com/"), request->common_params().url);
+ ASSERT_EQ(1u, request->commit_params().redirect_params.size());
+ EXPECT_EQ(GURL("https://c.com/"),
+ request->commit_params().redirect_params[0]->redirect_info.new_url);
+ ASSERT_EQ(1u, request->commit_params().redirects.size());
+ EXPECT_EQ(GURL("https://b.com/"), request->commit_params().redirects[0]);
+
+ if (base::FeatureList::IsEnabled(
+ features::kSanitizeLocationHeadersDuringNavigation)) {
+ size_t iter = 0;
+ std::optional<std::string_view> location =
+ request->commit_params()
+ .redirect_params[0]
+ ->response_head->headers->EnumerateHeader(&iter, "Location");
+ ASSERT_TRUE(location.has_value());
+ EXPECT_EQ("https://c.com/", location.value());
+ }
+}
+
+TEST_F(
+ NavigationRequestTest,
+ SanitizeRedirectsForCommitErrorPageInCurrentProcess_FinalURLFeatureDisabled) {
+ base::test::ScopedFeatureList feature_list;
+ feature_list.InitWithFeatures(
+ /*enabled_features=*/{features::kSanitizeLocationHeadersDuringNavigation},
+ /*disabled_features=*/{features::kSanitizeFailedSubframeNavigationUrls});
+
+ // Commit an initial page so the subframe has a parent document.
+ NavigationSimulator::NavigateAndCommitFromDocument(GURL("https://a.com/"),
+ main_test_rfh());
+ auto* child_frame = static_cast<TestRenderFrameHost*>(
+ content::RenderFrameHostTester::For(main_rfh())->AppendChild("child"));
+
+ const GURL start_url("https://b.com/start?param=1");
+ const GURL final_url("https://c.com/path?param=2");
+ std::unique_ptr<NavigationSimulator> navigation =
+ NavigationSimulator::CreateRendererInitiated(start_url, child_frame);
+ navigation->Start();
+
+ auto headers =
+ base::MakeRefCounted<net::HttpResponseHeaders>("HTTP/1.1 302 Found");
+ headers->SetHeader("Location", "https://c.com/path?param=2");
+ navigation->SetRedirectHeaders(headers);
+
+ navigation->Redirect(final_url);
... (file diff truncated)
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/chrome/browser/extensions/process_manager_browsertest.cc b/chrome/browser/extensions/process_manager_browsertest.cc
index 985e218b..6733a6a 100644
--- a/chrome/browser/extensions/process_manager_browsertest.cc
+++ b/chrome/browser/extensions/process_manager_browsertest.cc
@@ -1673,9 +1673,12 @@
<< "The initial navigation should be allowed, but not the server "
"redirect to extension2's manifest";
EXPECT_EQ(net::ERR_BLOCKED_BY_CLIENT, nav_observer.last_net_error_code());
- EXPECT_EQ(extension2_manifest, nav_observer.last_navigation_url());
- EXPECT_EQ(extension2_manifest,
- ChildFrameAt(main_frame, 1)->GetLastCommittedURL());
+ // We expect the URL to be sanitized, per https://crbug.com/517156678.
+ // TODO(crbug.com/40134629): Remove the sanitization once Subframe Error
+ // Page Isolation ships.
+ GURL expected_url = extension2_manifest.DeprecatedGetOriginAsURL();
+ EXPECT_EQ(expected_url, nav_observer.last_navigation_url());
+ EXPECT_EQ(expected_url, ChildFrameAt(main_frame, 1)->GetLastCommittedURL());
EXPECT_EQ(1u, pm->GetAllFrames().size());
EXPECT_EQ(1u, pm->GetRenderFrameHostsForExtension(extension1->id()).size());
EXPECT_EQ(0u, pm->GetRenderFrameHostsForExtension(extension2->id()).size());
diff --git a/chrome/browser/subresource_filter/subresource_filter_browsertest.cc b/chrome/browser/subresource_filter/subresource_filter_browsertest.cc
index 15c1c9a..403cc30 100644
--- a/chrome/browser/subresource_filter/subresource_filter_browsertest.cc
+++ b/chrome/browser/subresource_filter/subresource_filter_browsertest.cc
@@ -383,7 +383,11 @@
content::RenderFrameHost* frame = FindFrameByName(kSubframeNames[0]);
ASSERT_TRUE(frame);
- EXPECT_EQ(disallowed_subdocument_url, frame->GetLastCommittedURL());
+ // We expect the URL to be sanitized, per https://crbug.com/517156678.
+ // TODO(crbug.com/40134629): Remove the sanitization once Subframe Error Page
+ // Isolation ships.
+ EXPECT_EQ(disallowed_subdocument_url.DeprecatedGetOriginAsURL(),
+ frame->GetLastCommittedURL());
ExpectFramesIncludedInLayout(kSubframeNames, kExpectOnlySecondSubframe);
}
diff --git a/chrome/browser/subresource_filter/subresource_filter_fenced_frame_browsertest.cc b/chrome/browser/subresource_filter/subresource_filter_fenced_frame_browsertest.cc
index 36d4826..ffc1ccec 100644
--- a/chrome/browser/subresource_filter/subresource_filter_fenced_frame_browsertest.cc
+++ b/chrome/browser/subresource_filter/subresource_filter_fenced_frame_browsertest.cc
@@ -273,7 +273,11 @@
EXPECT_FALSE(WasParsedScriptElementLoaded(subframe));
EXPECT_TRUE(subframe->IsErrorDocument());
- EXPECT_EQ(kUrlWithIncludedScript, subframe->GetLastCommittedURL());
+ // We expect the URL to be sanitized, per https://crbug.com/517156678.
+ // TODO(crbug.com/40134629): Remove the sanitization once Subframe Error Page
+ // Isolation ships.
+ GURL expected_url = url::Origin::Create(kUrlWithIncludedScript).GetURL();
+ EXPECT_EQ(expected_url, subframe->GetLastCommittedURL());
}
} // namespace subresource_filter
diff --git a/content/browser/renderer_host/navigation_request_unittest.cc b/content/browser/renderer_host/navigation_request_unittest.cc
index 013e09d..0b532ad 100644
--- a/content/browser/renderer_host/navigation_request_unittest.cc
+++ b/content/browser/renderer_host/navigation_request_unittest.cc
@@ -811,8 +811,9 @@
NavigationRequest* request =
NavigationRequest::From(navigation->GetNavigationHandle());
+ auto common_params = request->common_params().Clone();
auto commit_params = request->commit_params().Clone();
- request->SanitizeRedirectsForCommit(commit_params);
+ request->SanitizeRedirectsForCommit(common_params, commit_params);
// redirect_params contains entries for B, C, and D, but not the starting URL.
// Ensure that the full URL for D is preserved.
@@ -877,9 +878,10 @@
NavigationRequest* request =
NavigationRequest::From(navigation->GetNavigationHandle());
+ auto common_params = request->common_params().Clone();
auto commit_params = request->commit_params().Clone();
- request->SanitizeRedirectsForCommit(commit_params);
+ request->SanitizeRedirectsForCommit(common_params, commit_params);
EXPECT_EQ(4u, commit_params->redirect_params.size());
@@ -955,9 +957,10 @@
NavigationRequest* request =
NavigationRequest::From(navigation->GetNavigationHandle());
+ auto common_params = request->common_params().Clone();
auto commit_params = request->commit_params().Clone();
- request->SanitizeRedirectsForCommit(commit_params);
+ request->SanitizeRedirectsForCommit(common_params, commit_params);
EXPECT_EQ(3u, commit_params->redirect_params.size());
@@ -1014,9 +1017,10 @@
NavigationRequest* request =
NavigationRequest::From(navigation->GetNavigationHandle());
+ auto common_params = request->common_params().Clone();
auto commit_params = request->commit_params().Clone();
- request->SanitizeRedirectsForCommit(commit_params);
+ request->SanitizeRedirectsForCommit(common_params, commit_params);
EXPECT_EQ(2u, commit_params->redirect_params.size());
@@ -1081,6 +1085,251 @@
EXPECT_EQ(start_url, request->original_url());
}
+// Test that when a redirected subframe navigation is blocked and the resulting
+// error page commits in the initiator's process, the final URL is reduced to
+// its origin in the parameters sent to the renderer. See crbug.com/517156678.
+TEST_F(NavigationRequestTest,
+ SanitizeRedirectsForCommitErrorPageInCurrentProcess) {
+ base::test::ScopedFeatureList feature_list;
+ feature_list.InitWithFeatures(
+ /*enabled_features=*/{features::kSanitizeFailedSubframeNavigationUrls,
+ features::kSanitizeLocationHeadersDuringNavigation},
+ /*disabled_features=*/{});
+
+ // Commit an initial page so the subframe has a parent document.
+ NavigationSimulator::NavigateAndCommitFromDocument(GURL("https://a.com/"),
+ main_test_rfh());
+ auto* child_frame = static_cast<TestRenderFrameHost*>(
+ content::RenderFrameHostTester::For(main_rfh())->AppendChild("child"));
+
+ const GURL start_url("https://b.com/start?param=1");
+ const GURL final_url("https://c.com/path?param=2");
+ std::unique_ptr<NavigationSimulator> navigation =
+ NavigationSimulator::CreateRendererInitiated(start_url, child_frame);
+ navigation->Start();
+
+ auto headers =
+ base::MakeRefCounted<net::HttpResponseHeaders>("HTTP/1.1 302 Found");
+ headers->SetHeader("Location", "https://c.com/path?param=2");
+ navigation->SetRedirectHeaders(headers);
+
+ navigation->Redirect(final_url);
+ navigation->Fail(net::ERR_BLOCKED_BY_CLIENT);
+
+ NavigationRequest* request =
+ NavigationRequest::From(navigation->GetNavigationHandle());
+ ASSERT_EQ(NavigationRequest::ErrorPageProcess::kCurrentProcess,
+ request->ComputeErrorPageProcess());
+
+ // The error page commits in the initiator's process, so the final URL (which
+ // is the post-redirect target) should be reduced to its origin in both the
+ // common and commit params.
+ EXPECT_EQ(GURL("https://c.com/"), request->common_params().url);
+ ASSERT_EQ(1u, request->commit_params().redirect_params.size());
+ EXPECT_EQ(GURL("https://c.com/"),
+ request->commit_params().redirect_params[0]->redirect_info.new_url);
+ ASSERT_EQ(1u, request->commit_params().redirects.size());
+ EXPECT_EQ(GURL("https://b.com/"), request->commit_params().redirects[0]);
+
+ if (base::FeatureList::IsEnabled(
+ features::kSanitizeLocationHeadersDuringNavigation)) {
+ size_t iter = 0;
+ std::optional<std::string_view> location =
+ request->commit_params()
+ .redirect_params[0]
+ ->response_head->headers->EnumerateHeader(&iter, "Location");
+ ASSERT_TRUE(location.has_value());
+ EXPECT_EQ("https://c.com/", location.value());
+ }
+}
+
+TEST_F(
+ NavigationRequestTest,
+ SanitizeRedirectsForCommitErrorPageInCurrentProcess_FinalURLFeatureDisabled) {
+ base::test::ScopedFeatureList feature_list;
+ feature_list.InitWithFeatures(
+ /*enabled_features=*/{features::kSanitizeLocationHeadersDuringNavigation},
+ /*disabled_features=*/{features::kSanitizeFailedSubframeNavigationUrls});
+
+ // Commit an initial page so the subframe has a parent document.
+ NavigationSimulator::NavigateAndCommitFromDocument(GURL("https://a.com/"),
+ main_test_rfh());
+ auto* child_frame = static_cast<TestRenderFrameHost*>(
+ content::RenderFrameHostTester::For(main_rfh())->AppendChild("child"));
+
+ const GURL start_url("https://b.com/start?param=1");
+ const GURL final_url("https://c.com/path?param=2");
+ std::unique_ptr<NavigationSimulator> navigation =
+ NavigationSimulator::CreateRendererInitiated(start_url, child_frame);
+ navigation->Start();
+
+ auto headers =
+ base::MakeRefCounted<net::HttpResponseHeaders>("HTTP/1.1 302 Found");
+ headers->SetHeader("Location", "https://c.com/path?param=2");
+ navigation->SetRedirectHeaders(headers);
+
+ navigation->Redirect(final_url);
+ navigation->Fail(net::ERR_BLOCKED_BY_CLIENT);
+
+ NavigationRequest* request =
+ NavigationRequest::From(navigation->GetNavigationHandle());
+ ASSERT_EQ(NavigationRequest::ErrorPageProcess::kCurrentProcess,
+ request->ComputeErrorPageProcess());
+
+ // The feature is disabled, so the final URL should NOT be reduced to its
+ // origin.
+ EXPECT_EQ(final_url, request->common_params().url);
+ ASSERT_EQ(1u, request->commit_params().redirect_params.size());
+ EXPECT_EQ(final_url,
+ request->commit_params().redirect_params[0]->redirect_info.new_url);
+ ASSERT_EQ(1u, request->commit_params().redirects.size());
+ EXPECT_EQ(GURL("https://b.com/"), request->commit_params().redirects[0]);
+
+ // Even if kSanitizeLocationHeadersDuringNavigation is enabled, it should not
+ // sanitize the Location header because sanitize_final_url is false (due to
+ // the disabled feature flag), which makes it use the final URL's origin
+ // (c.com) as target_commit_origin, which is same-origin with the redirect
+ // target (c.com).
+ if (base::FeatureList::IsEnabled(
+ features::kSanitizeLocationHeadersDuringNavigation)) {
+ size_t iter = 0;
+ std::optional<std::string_view> location =
+ request->commit_params()
+ .redirect_params[0]
+ ->response_head->headers->EnumerateHeader(&iter, "Location");
+ ASSERT_TRUE(location.has_value());
+ EXPECT_EQ("https://c.com/path?param=2", location.value());
+ }
+}
+
+TEST_F(NavigationRequestTest,
+ DontSanitizeRedirectsForCommitErrorPageInCurrentProcessSameOrigin) {
+ base::test::ScopedFeatureList feature_list;
+ feature_list.InitWithFeatures(
+ /*enabled_features=*/{features::kSanitizeFailedSubframeNavigationUrls,
+ features::kSanitizeLocationHeadersDuringNavigation},
+ /*disabled_features=*/{});
+
+ // Commit an initial page so the subframe has a parent document.
+ NavigationSimulator::NavigateAndCommitFromDocument(GURL("https://c.com/"),
+ main_test_rfh());
+ auto* child_frame = static_cast<TestRenderFrameHost*>(
+ content::RenderFrameHostTester::For(main_rfh())->AppendChild("child"));
+
+ const GURL start_url("https://b.com/start?param=1");
+ const GURL final_url("https://c.com/path?param=2");
+ std::unique_ptr<NavigationSimulator> navigation =
+ NavigationSimulator::CreateRendererInitiated(start_url, child_frame);
+ navigation->Start();
+
+ auto headers =
+ base::MakeRefCounted<net::HttpResponseHeaders>("HTTP/1.1 302 Found");
+ headers->SetHeader("Location", "https://c.com/path?param=2");
+ navigation->SetRedirectHeaders(headers);
+
+ navigation->Redirect(final_url);
+ navigation->Fail(net::ERR_BLOCKED_BY_CLIENT);
+
+ NavigationRequest* request =
+ NavigationRequest::From(navigation->GetNavigationHandle());
+ ASSERT_EQ(NavigationRequest::ErrorPageProcess::kCurrentProcess,
+ request->ComputeErrorPageProcess());
+
+ // The final URL is same-origin with the receiving process (c.com), so it
+ // should NOT be reduced to its origin.
+ EXPECT_EQ(final_url, request->common_params().url);
+ ASSERT_EQ(1u, request->commit_params().redirect_params.size());
+ EXPECT_EQ(final_url,
+ request->commit_params().redirect_params[0]->redirect_info.new_url);
+ ASSERT_EQ(1u, request->commit_params().redirects.size());
+ EXPECT_EQ(GURL("https://b.com/"), request->commit_params().redirects[0]);
+
+ if (base::FeatureList::IsEnabled(
+ features::kSanitizeLocationHeadersDuringNavigation)) {
+ size_t iter = 0;
+ std::optional<std::string_view> location =
+ request->commit_params()
+ .redirect_params[0]
+ ->response_head->headers->EnumerateHeader(&iter, "Location");
+ ASSERT_TRUE(location.has_value());
+ EXPECT_EQ("https://c.com/path?param=2", location.value());
+ }
+}
+
+// Test that when a subframe navigation with multiple redirects (same-origin to
+// each other, but cross-origin to the main page) is blocked and commits an
+// error page in the initiator's process, all redirect URLs are reduced to
+// origin.
+TEST_F(
+ NavigationRequestTest,
+ SanitizeRedirectsForCommitErrorPageInCurrentProcessMultipleRedirectsSameOriginWithEachOther) {
+ base::test::ScopedFeatureList feature_list;
+ feature_list.InitWithFeatures(
+ /*enabled_features=*/{features::kSanitizeFailedSubframeNavigationUrls,
+ features::kSanitizeLocationHeadersDuringNavigation},
+ /*disabled_features=*/{});
+
+ // Commit an initial page so the subframe has a parent document (origin A).
+ NavigationSimulator::NavigateAndCommitFromDocument(GURL("https://a.com/"),
+ main_test_rfh());
+ auto* child_frame = static_cast<TestRenderFrameHost*>(
+ content::RenderFrameHostTester::For(main_rfh())->AppendChild("child"));
+
+ const GURL start_url("https://b.com/start?param=1");
+ const GURL url_2("https://b.com/path1?param=2");
... (truncated)
Loading diff…
Original Bug Report
reported by vm...@google.com
Leak of full cross-origin redirect URL during blocked subframe navigation
Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.
Overview: When a renderer-initiated subframe navigation is blocked (e.g., by CSP), an error page commits in the initiator’s process because subframe error page isolation is disabled. Before committing, the browser does not sanitize the final redirect URL in the commit parameters. This can potentially allow a compromised initiator renderer to access the full path and query parameters of cross-origin redirect targets.
Affected files:
content/browser/renderer_host/navigation_request.cc
Estimated timestamp from git blame: 2025-06-02
Summary
A potential cross-origin information disclosure vulnerability exists in Google Chrome where the full URL (including path and query parameters) of a cross-origin redirect target can be leaked to an untrusted initiator renderer process during a failed subframe navigation.
Root Cause Analysis
To prevent leaking sensitive data across process boundaries, NavigationRequest::SanitizeRedirectsForCommit() is called before committing navigation parameters. This function is designed to sanitize redirect URLs by stripping their path and query components. However, it deliberately skips sanitizing the last entry of the redirect_infos vector (and never touches common_params->url) under the assumption that the final destination URL will be committed inside the destination’s own isolated process:
// content/browser/renderer_host/navigation_request.cc
for (GURL& redirect : commit_params->redirects) {
redirect = redirect.DeprecatedGetOriginAsURL();
}
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();
}
}
While this assumption is correct for successful navigations, it is incorrect when the navigation fails and commits an error page in the initiator’s process (i.e., when ComputeErrorPageProcess() returns ErrorPageProcess::kCurrentProcess).
For a renderer-initiated subframe navigation blocked by a request-blocking error (such as ERR_BLOCKED_BY_CSP), and when subframe error page isolation is disabled (which is the default on Chrome), ComputeErrorPageProcess() decides to reuse the iframe’s pre-navigation process to commit the error page. Consequently, both common_params->url and commit_params->redirect_infos.back().new_url are sent completely unsanitized to the initiator’s process via the CommitFailedNavigation IPC.
Potential Attack Scenario
(Note: The following steps are potential/suggested based on static analysis of the source code, as our tooling does not currently have the capability to run code or construct a live proof-of-concept.)
- A compromised renderer under the control of an attacker (e.g.,
attacker.com) embeds an iframe pointing tohttps://victim.com/sensitive-actionand sets aContent-Security-Policy: frame-src https://victim.comthat permits loading the initial victim URL. - The request to
victim.comredirects tohttps://cdn.example/file?token=SECRET(a cross-origin destination containing sensitive query parameters, such as access tokens, OAuth codes, or pre-signed URLs). - The browser receives the redirect, updates
common_params->urland the last entry ofredirect_infos, and then evaluates the parent’s CSP. Because the redirect target iscdn.example(which is not allowed byframe-src), the navigation fails withERR_BLOCKED_BY_CSP. - Since subframe error page isolation is disabled,
ComputeErrorPageProcessselectskCurrentProcess(the attacker’s process) to render the error page. SanitizeRedirectsForCommit()skips sanitizing the final redirect URL. The browser then transmits the full, unsanitized URL to the attacker’s renderer via theCommitFailedNavigationMojo IPC.- The compromised renderer intercepts the Mojo IPC payload and extracts the sensitive token, bypassing cross-origin boundary protections.
Suggested Fix
When committing a failed navigation inside the initiator’s process (ErrorPageProcess::kCurrentProcess), we should sanitize all the entries in commit_params->redirect_infos (including the last one) and sanitize common_params->url down to their origins to prevent sensitive cross-origin data from being disclosed to the initiator.
In NavigationRequest::SanitizeRedirectsForCommit:
bool is_failed_in_current_process =
net_error_ != net::OK &&
ComputeErrorPageProcess() == ErrorPageProcess::kCurrentProcess;
if (!commit_params->redirect_infos.empty()) {
auto redirect_infos_span = base::span(commit_params->redirect_infos);
size_t sanitize_count = is_failed_in_current_process
? redirect_infos_span.size()
: redirect_infos_span.size() - 1;
for (net::RedirectInfo& redirect :
redirect_infos_span.first(sanitize_count)) {
redirect.new_url = redirect.new_url.DeprecatedGetOriginAsURL();
}
}
Evaluated with Chrome root at commit: b1520ef4a76878853a31f0943b565e42060edec8
Results so far have been promising, but there can be wrong deductions. Feel free to adjust as follows:
- If you are familiar with the severity guidelines, you may adjust the severity.
- If this is a false positive, and there’s no work to be done, please close as WAI.
- If there is work to do here but not a vulnerability, please change the issue type to Task/Bug/FR.
Data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.
References
On This Page