From 6c42a0732b89de096f492051e8aa56935f8c40a3 Mon Sep 17 00:00:00 2001
From: Liam Brady <lbrady@google.com>
Date: Tue, 04 Aug 2026 15:20:56 -0700
Subject: [PATCH] Validate client_side_redirect_url origin in BeginNavigation.

A compromised renderer can supply an arbitrary cross-origin URL as
`client_side_redirect_url` in BeginNavigationParams. The browser
previously only ran FilterURL on this parameter, which checks
CanRequestURL and accepts arbitrary HTTPS URLs from any process,
allowing a compromised renderer to inject unvisited cross-origin entries
into the user's browsing history.

This CL strengthens this validation by introducing
`VerifyClientSideRedirectUrl()` to ipc_utils, which verifies via
`ChildProcessSecurityPolicyImpl::HostsOrigin()` that the initiating
process has previously hosted the claimed origin. If validation fails,
the renderer will be killed with a bad message.

Bug: 511820041, 40066983
Change-Id: I5a369f310f2df5b8ba7fdfba105b9aa1ff1149ca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8162305
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Commit-Queue: Liam Brady <lbrady@google.com>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1673720}
---

diff --git a/content/browser/bad_message.h b/content/browser/bad_message.h
index aa702a3..d2c6586 100644
--- a/content/browser/bad_message.h
+++ b/content/browser/bad_message.h
@@ -396,6 +396,7 @@
   PMM_GET_SUBSCRIPTION_IN_FENCED_FRAME = 368,
   BIBI_BIND_VIBRATION_MANAGER_FOR_FENCED_FRAME = 369,
   RFH_BEGIN_NAVIGATION_NO_INITIATOR_TOKENS = 370,
+  RFHI_INVALID_CLIENT_SIDE_REDIRECT_URL = 371,
 
   // Please add new elements here. The naming convention is abbreviated class
   // name (e.g. RenderFrameHost becomes RFH) plus a unique description of the
diff --git a/content/browser/navigation_browsertest.cc b/content/browser/navigation_browsertest.cc
index 69aec3240..f72b543 100644
--- a/content/browser/navigation_browsertest.cc
+++ b/content/browser/navigation_browsertest.cc
@@ -10340,4 +10340,45 @@
   EXPECT_FALSE(policy->CanReadFile(process_c, file_path));
 }
 
+// Verify that navigating an about:blank iframe (which sets
+// client_side_redirect_url to about:blank) succeeds.
+IN_PROC_BROWSER_TEST_F(NavigationBrowserTest,
+                       AboutBlankFrameClientSideRedirectUrl) {
+  GURL start_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), start_url));
+
+  WebContentsImpl* web_contents =
+      static_cast<WebContentsImpl*>(shell()->web_contents());
+  FrameTreeNode* root = web_contents->GetPrimaryFrameTree().root();
+
+  // Create an iframe and navigate it to about:blank so that it is a committed,
+  // non-initial document.
+  EXPECT_TRUE(ExecJs(root->current_frame_host(),
+                     "var frame = document.createElement('iframe'); "
+                     "frame.id = 'child'; "
+                     "document.body.appendChild(frame);"));
+  EXPECT_TRUE(
+      NavigateIframeToURL(web_contents, "child", GURL(url::kAboutBlankURL)));
+
+  // Perform a client-side redirect (replacement navigation) from about:blank.
+  GURL target_url(embedded_test_server()->GetURL("b.com", "/title2.html"));
+  TestNavigationObserver nav_observer(web_contents);
+  EXPECT_TRUE(ExecJs(root->current_frame_host(),
+                     JsReplace("document.getElementById('child').contentWindow."
+                               "location.replace($1);",
+                               target_url)));
+  nav_observer.Wait();
+  EXPECT_TRUE(nav_observer.last_navigation_succeeded());
+
+  // Check that the client side redirect URL (about:blank) is pushed as the
+  // first URL onto the subframe's redirect chain.
+  NavigationEntryImpl* entry =
+      web_contents->GetController().GetLastCommittedEntry();
+  FrameNavigationEntry* frame_entry = entry->GetFrameEntry(root->child_at(0));
+  ASSERT_TRUE(frame_entry);
+  EXPECT_EQ(frame_entry->redirect_chain().size(), 2u);
+  EXPECT_EQ(frame_entry->redirect_chain()[0], GURL(url::kAboutBlankURL));
+  EXPECT_EQ(frame_entry->redirect_chain()[1], target_url);
+}
+
 }  // namespace content
diff --git a/content/browser/renderer_host/ipc_utils.cc b/content/browser/renderer_host/ipc_utils.cc
index fb99c3c..ee707728 100644
--- a/content/browser/renderer_host/ipc_utils.cc
+++ b/content/browser/renderer_host/ipc_utils.cc
@@ -366,6 +366,46 @@
   return true;
 }
 
+bool VerifyClientSideRedirectUrl(const RenderFrameHostImpl& current_rfh,
+                                 GURL* client_side_redirect_url) {
+  CHECK_CURRENTLY_ON(BrowserThread::UI);
+  CHECK(client_side_redirect_url);
+
+  RenderProcessHost* process = current_rfh.GetProcess();
+  CHECK(process);
+
+  if (process->FilterURL(true, client_side_redirect_url) ==
+      RenderProcessHost::FilterURLResult::kBlocked) {
+    bad_message::ReceivedBadMessage(
+        process, bad_message::RFHI_INVALID_CLIENT_SIDE_REDIRECT_URL);
+    return false;
+  }
+
+  // `client_side_redirect_url` is only populated if the navigation's transition
+  // type is a client side redirect. For all other renderer-initiated
+  // navigations, it is intentionally empty.
+  if (client_side_redirect_url->is_empty()) {
+    return true;
+  }
+
+  // Verify that `process` has hosted `redirect_origin` either as a standard
+  // tuple origin or as the precursor of an opaque origin (e.g. when the
+  // redirect is initiated by a sandboxed document).
+  url::Origin redirect_origin = url::Origin::Resolve(
+      *client_side_redirect_url, current_rfh.GetLastCommittedOrigin());
+  auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
+  ChildProcessId process_id = process->GetID();
+  if (!policy->HostsOrigin(process_id.GetUnsafeValue(), redirect_origin) &&
+      !policy->HostsOrigin(process_id.GetUnsafeValue(),
+                           redirect_origin.DeriveNewOpaqueOrigin())) {
+    bad_message::ReceivedBadMessage(
+        process, bad_message::RFHI_INVALID_CLIENT_SIDE_REDIRECT_URL);
+    return false;
+  }
+
+  return true;
+}
+
 bool VerifyCreateNewWindowParams(const RenderFrameHostImpl& current_rfh,
                                  const mojom::CreateNewWindowParams& params) {
   CHECK_CURRENTLY_ON(BrowserThread::UI, base::NotFatalUntil::M154);
diff --git a/content/browser/renderer_host/ipc_utils.h b/content/browser/renderer_host/ipc_utils.h
index b56bf12..b9ebe5d 100644
--- a/content/browser/renderer_host/ipc_utils.h
+++ b/content/browser/renderer_host/ipc_utils.h
@@ -60,6 +60,20 @@
     blink::mojom::CommonNavigationParams* common_params,
     std::optional<blink::LocalFrameToken>& initiator_frame_token);
 
+// Verifies that `client_side_redirect_url` in BeginNavigationParams is valid
+// and can be accessed by `current_rfh`'s process.
+//
+// FilterURL is run on `client_side_redirect_url` as a side effect.
+// Returns true if `client_side_redirect_url` is valid and its origin is hosted
+// by `current_rfh`'s process.
+//
+// Terminates `current_rfh`'s process and returns false if
+// `client_side_redirect_url` is invalid.
+//
+// This function has to be called on the UI thread.
+bool VerifyClientSideRedirectUrl(const RenderFrameHostImpl& current_rfh,
+                                 GURL* client_side_redirect_url);
+
 // Verifies that the CreateNewWindowParams are valid and can be accessed by
 // `current_rfh`'s process.
 //
diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc
index 4653a24e..a484a889 100644
--- a/content/browser/renderer_host/render_frame_host_impl.cc
+++ b/content/browser/renderer_host/render_frame_host_impl.cc
@@ -11691,9 +11691,11 @@
     }
   }
 
-  // TODO(crbug.com/40066983): Consider converting these into renderer kills.
   GetProcess()->FilterURL(true, &begin_params->searchable_form_url);
-  GetProcess()->FilterURL(true, &begin_params->client_side_redirect_url);
+  if (!VerifyClientSideRedirectUrl(*this,
+                                   &begin_params->client_side_redirect_url)) {
+    return;
+  }
 
   // If the request was for a blob URL, but the validated URL is no longer a
   // blob URL, reset the blob_url_token to prevent hitting the ReportBadMessage
diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
index b33ffd48..2089bf0 100644
--- a/content/browser/security_exploit_browsertest.cc
+++ b/content/browser/security_exploit_browsertest.cc
@@ -1875,8 +1875,7 @@
 };
 
 // Verify that a compromised renderer can't poison client_side_redirect_url
-// with a privileged URL, which could then be loaded via a subsequent
-// "request desktop site" operation.
+// with a privileged URL.
 IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
                        BlockIllegalClientSideRedirectUrl) {
   GURL start_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
@@ -1888,36 +1887,100 @@
   GURL webui_url(GetWebUIURL(kChromeUIGpuHost));
   ClientSideRedirectUrlReplacer injector(web_contents, webui_url);

diff --git a/content/browser/navigation_browsertest.cc b/content/browser/navigation_browsertest.cc
index 69aec3240..f72b543 100644
--- a/content/browser/navigation_browsertest.cc
+++ b/content/browser/navigation_browsertest.cc
@@ -10340,4 +10340,45 @@
   EXPECT_FALSE(policy->CanReadFile(process_c, file_path));
 }
 
+// Verify that navigating an about:blank iframe (which sets
+// client_side_redirect_url to about:blank) succeeds.
+IN_PROC_BROWSER_TEST_F(NavigationBrowserTest,
+                       AboutBlankFrameClientSideRedirectUrl) {
+  GURL start_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), start_url));
+
+  WebContentsImpl* web_contents =
+      static_cast<WebContentsImpl*>(shell()->web_contents());
+  FrameTreeNode* root = web_contents->GetPrimaryFrameTree().root();
+
+  // Create an iframe and navigate it to about:blank so that it is a committed,
+  // non-initial document.
+  EXPECT_TRUE(ExecJs(root->current_frame_host(),
+                     "var frame = document.createElement('iframe'); "
+                     "frame.id = 'child'; "
+                     "document.body.appendChild(frame);"));
+  EXPECT_TRUE(
+      NavigateIframeToURL(web_contents, "child", GURL(url::kAboutBlankURL)));
+
+  // Perform a client-side redirect (replacement navigation) from about:blank.
+  GURL target_url(embedded_test_server()->GetURL("b.com", "/title2.html"));
+  TestNavigationObserver nav_observer(web_contents);
+  EXPECT_TRUE(ExecJs(root->current_frame_host(),
+                     JsReplace("document.getElementById('child').contentWindow."
+                               "location.replace($1);",
+                               target_url)));
+  nav_observer.Wait();
+  EXPECT_TRUE(nav_observer.last_navigation_succeeded());
+
+  // Check that the client side redirect URL (about:blank) is pushed as the
+  // first URL onto the subframe's redirect chain.
+  NavigationEntryImpl* entry =
+      web_contents->GetController().GetLastCommittedEntry();
+  FrameNavigationEntry* frame_entry = entry->GetFrameEntry(root->child_at(0));
+  ASSERT_TRUE(frame_entry);
+  EXPECT_EQ(frame_entry->redirect_chain().size(), 2u);
+  EXPECT_EQ(frame_entry->redirect_chain()[0], GURL(url::kAboutBlankURL));
+  EXPECT_EQ(frame_entry->redirect_chain()[1], target_url);
+}
+
 }  // namespace content
diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
index b33ffd48..2089bf0 100644
--- a/content/browser/security_exploit_browsertest.cc
+++ b/content/browser/security_exploit_browsertest.cc
@@ -1875,8 +1875,7 @@
 };
 
 // Verify that a compromised renderer can't poison client_side_redirect_url
-// with a privileged URL, which could then be loaded via a subsequent
-// "request desktop site" operation.
+// with a privileged URL.
 IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
                        BlockIllegalClientSideRedirectUrl) {
   GURL start_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
@@ -1888,36 +1887,100 @@
   GURL webui_url(GetWebUIURL(kChromeUIGpuHost));
   ClientSideRedirectUrlReplacer injector(web_contents, webui_url);
 
+  RenderProcessHostBadIpcMessageWaiter kill_waiter(
+      web_contents->GetPrimaryMainFrame()->GetProcess());
+
   // Setup the interceptor to inject the WebUI URL into the next
   // BeginNavigation's client_side_redirect_url.
   injector.Activate();
 
-  // Trigger a normal renderer-initiated navigation to a benign URL. The
-  // injector will poison the client_side_redirect_url in that IPC's
-  // BeginNavigationParams.
+  // Trigger a normal renderer-initiated navigation. The injector will poison
+  // the client_side_redirect_url in that IPC's BeginNavigationParams.
   GURL next_url(embedded_test_server()->GetURL("b.com", "/title2.html"));
-  TestNavigationManager nav_manager(web_contents, next_url);
-  EXPECT_TRUE(ExecJs(web_contents, JsReplace("location.href = $1;", next_url)));
+  ExecuteScriptAsync(web_contents, JsReplace("location.href = $1;", next_url));
 
-  // Wait for the navigation to finish.
-  ASSERT_TRUE(nav_manager.WaitForNavigationFinished());
-  EXPECT_TRUE(nav_manager.was_successful());
+  EXPECT_EQ(bad_message::RFHI_INVALID_CLIENT_SIDE_REDIRECT_URL,
+            kill_waiter.Wait());
+}
 
-  // At this point, before the fix, the NavigationEntry has saved
-  // the WebUI URL as the OriginalRequestURL.
+// Verify that a compromised renderer can't set client_side_redirect_url to a
+// cross-origin web URL that is not hosted by the process.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
+                       BlockCrossOriginClientSideRedirectUrl) {
+  GURL start_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), start_url));
 
-  // Simulate the user clicking "Request Desktop Site" or similar,
-  // triggering a reload of the original request URL.
-  TestNavigationObserver reload_observer(web_contents);
-  web_contents->GetController().LoadOriginalRequestURL();
-  reload_observer.Wait();
+  WebContentsImpl* web_contents =
+      static_cast<WebContentsImpl*>(shell()->web_contents());
 
-  // Ensure that the browser doesn't navigate to the WebUI URL, which should've
-  // been filtered out when processing the corresponding BeginNavigation IPC.
-  // TODO(crbug.com/40066983): Consider terminating the renderer process
-  // instead.
-  EXPECT_NE(webui_url, web_contents->GetLastCommittedURL());
-  EXPECT_EQ(GURL(kBlockedURL), web_contents->GetLastCommittedURL());
+  GURL cross_origin_url(
+      embedded_test_server()->GetURL("b.com", "/title2.html"));
+  ClientSideRedirectUrlReplacer injector(web_contents, cross_origin_url);
+
+  RenderProcessHostBadIpcMessageWaiter kill_waiter(
+      web_contents->GetPrimaryMainFrame()->GetProcess());
+
+  injector.Activate();
+
+  GURL next_url(embedded_test_server()->GetURL("a.com", "/title3.html"));
+  ExecuteScriptAsync(web_contents, JsReplace("location.href = $1;", next_url));
+
+  EXPECT_EQ(bad_message::RFHI_INVALID_CLIENT_SIDE_REDIRECT_URL,
+            kill_waiter.Wait());
+}
+
+// Verify that a sandboxed frame can legitimately perform client-side redirects
+// without being terminated, but is terminated if it attempts to spoof a
+// client_side_redirect_url that it does not host.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
+                       SandboxedFrameClientSideRedirectUrl) {
+  GURL start_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), start_url));
+
+  WebContentsImpl* web_contents =
+      static_cast<WebContentsImpl*>(shell()->web_contents());
+  FrameTreeNode* root = web_contents->GetPrimaryFrameTree().root();
+
+  // Create a sandboxed child frame at a.com.
+  GURL child_url(embedded_test_server()->GetURL("a.com", "/title2.html"));
+  std::string js_str = base::StringPrintf(
+      "var frame = document.createElement('iframe'); "
+      "frame.id = 'sandboxed_child'; "
+      "frame.sandbox = 'allow-scripts'; "
+      "frame.src = '%s'; "
+      "document.body.appendChild(frame);",
+      child_url.spec().c_str());
+  EXPECT_TRUE(ExecJs(root->current_frame_host(), js_str));
+  ASSERT_TRUE(WaitForLoadStop(web_contents));
+
+  RenderFrameHostImpl* subframe = root->child_at(0)->current_frame_host();
+  ASSERT_TRUE(subframe->IsSandboxed(network::mojom::WebSandboxFlags::kOrigin));
+
+  // Legitimate client-side redirect in sandboxed frame: the sandboxed frame
+  // navigates itself, and Blink populates client_side_redirect_url with
+  // child_url. Verify that the navigation succeeds and the renderer is not
+  // terminated.
+  GURL next_url(embedded_test_server()->GetURL("a.com", "/title3.html"));
+  TestNavigationObserver nav_observer(web_contents);
+  EXPECT_TRUE(ExecJs(subframe, JsReplace("location.href = $1;", next_url)));
+  nav_observer.Wait();
+  EXPECT_TRUE(nav_observer.last_navigation_succeeded());
+
+  // Malicious attempt: a compromised sandboxed renderer attempts to set
+  // client_side_redirect_url to a cross-origin URL.
+  subframe = root->child_at(0)->current_frame_host();
+  GURL cross_origin_url(
+      embedded_test_server()->GetURL("b.com", "/title2.html"));
+  ClientSideRedirectUrlReplacer injector(web_contents, cross_origin_url);
+
+  RenderProcessHostBadIpcMessageWaiter kill_waiter(subframe->GetProcess());
+  injector.Activate();
+
+  GURL final_url(embedded_test_server()->GetURL("a.com", "/empty.html"));
+  ExecuteScriptAsync(subframe, JsReplace("location.href = $1;", final_url));
+
+  EXPECT_EQ(bad_message::RFHI_INVALID_CLIENT_SIDE_REDIRECT_URL,
+            kill_waiter.Wait());
 }
 
 class RemoteFrameHostInterceptor