From 446871399858d161c079f14e8f6440a080889d36 Mon Sep 17 00:00:00 2001
From: Mike West <[email protected]>
Date: Mon, 09 Mar 2026 02:12:25 -0700
Subject: [PATCH] Sanitize redirect chain when committing error pages.

When committing successful navigations, we execute
`SanitizeRedirectsForCommit(commit_params)` in order to reduce the
amount of cross-site information which might flow into a renderer. We
should do the same for error pages.

This CL copies the sanitization logic from
`NavigationRequest::CommitNavigation()` into
`NavigationRequest::CommitErrorPage()`, and adds a unittest based on
`NavigationRequestTest.SanitizeRedirectsForCommit`.

Bug: 490139441
Change-Id: I61ffc6550848a97dc036383ea9ddfce738472bff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7642049
Reviewed-by: Nasko Oskov <[email protected]>
Commit-Queue: Mike West <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1596201}
---

diff --git a/content/browser/renderer_host/navigation_request.cc b/content/browser/renderer_host/navigation_request.cc
index 7ed0a1b..26d4c43 100644
--- a/content/browser/renderer_host/navigation_request.cc
+++ b/content/browser/renderer_host/navigation_request.cc
@@ -6510,6 +6510,13 @@
   // Use a separate cache shard, and no cookies, for error pages.
   isolation_info_for_subresources_ =
       net::IsolationInfo::CreateTransient(/*nonce=*/std::nullopt);
+
+  // 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()->FailedNavigation(
       this, *common_params_, *commit_params_, has_stale_copy_in_cache_,
       net_error_, extended_error_code_, error_page_content, *document_token_);
diff --git a/content/browser/renderer_host/navigation_request_unittest.cc b/content/browser/renderer_host/navigation_request_unittest.cc
index 0c4f49a..30df8db02 100644
--- a/content/browser/renderer_host/navigation_request_unittest.cc
+++ b/content/browser/renderer_host/navigation_request_unittest.cc
@@ -876,6 +876,39 @@
   EXPECT_EQ(GURL("https://c.com"), commit_params->redirects[2]);
 }
 
+// Test to ensure that SanitizeRedirectsForCommit is called when a navigation
+// fails and commits an error page.
+TEST_F(NavigationRequestTest, SanitizeRedirectsForCommitErrorPage) {
+  const GURL start_url("https://a.com?param=1");
+  const GURL url_2("https://b.com?param=2#foo");
+  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(final_url);
+  navigation->Fail(net::ERR_CONNECTION_RESET);
+
+  NavigationRequest* request =
+      NavigationRequest::From(navigation->GetNavigationHandle());
+
+  // We expect the redirects in the NavigationRequest's commit_params_ to be
+  // sanitized.
+  const auto& commit_params = request->commit_params();
+
+  // redirects contains entries for A and B.
+  EXPECT_EQ(2u, commit_params.redirects.size());
+  EXPECT_EQ(GURL("https://a.com"), commit_params.redirects[0]);
+  EXPECT_EQ(GURL("https://b.com"), commit_params.redirects[1]);
+
+  // redirect_infos contains entries for B and D.
+  // The last entry (D) should NOT be sanitized.
+  EXPECT_EQ(2u, commit_params.redirect_infos.size());
+  EXPECT_EQ(GURL("https://b.com"), commit_params.redirect_infos[0].new_url);
+  EXPECT_EQ(final_url, commit_params.redirect_infos[1].new_url);
+}
+
 TEST_F(NavigationRequestTest, AbortsDeletedNavigationInProgress) {
   const GURL kUrl1 = GURL("http://a.com");
   std::unique_ptr<NavigationSimulator> navigation =

diff --git a/content/browser/renderer_host/navigation_request_unittest.cc b/content/browser/renderer_host/navigation_request_unittest.cc
index 0c4f49a..30df8db02 100644
--- a/content/browser/renderer_host/navigation_request_unittest.cc
+++ b/content/browser/renderer_host/navigation_request_unittest.cc
@@ -876,6 +876,39 @@
   EXPECT_EQ(GURL("https://c.com"), commit_params->redirects[2]);
 }
 
+// Test to ensure that SanitizeRedirectsForCommit is called when a navigation
+// fails and commits an error page.
+TEST_F(NavigationRequestTest, SanitizeRedirectsForCommitErrorPage) {
+  const GURL start_url("https://a.com?param=1");
+  const GURL url_2("https://b.com?param=2#foo");
+  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(final_url);
+  navigation->Fail(net::ERR_CONNECTION_RESET);
+
+  NavigationRequest* request =
+      NavigationRequest::From(navigation->GetNavigationHandle());
+
+  // We expect the redirects in the NavigationRequest's commit_params_ to be
+  // sanitized.
+  const auto& commit_params = request->commit_params();
+
+  // redirects contains entries for A and B.
+  EXPECT_EQ(2u, commit_params.redirects.size());
+  EXPECT_EQ(GURL("https://a.com"), commit_params.redirects[0]);
+  EXPECT_EQ(GURL("https://b.com"), commit_params.redirects[1]);
+
+  // redirect_infos contains entries for B and D.
+  // The last entry (D) should NOT be sanitized.
+  EXPECT_EQ(2u, commit_params.redirect_infos.size());
+  EXPECT_EQ(GURL("https://b.com"), commit_params.redirect_infos[0].new_url);
+  EXPECT_EQ(final_url, commit_params.redirect_infos[1].new_url);
+}
+
 TEST_F(NavigationRequestTest, AbortsDeletedNavigationInProgress) {
   const GURL kUrl1 = GURL("http://a.com");
   std::unique_ptr<NavigationSimulator> navigation =