From 59cf8131947d105468cfb47814a8c1ead94f72c3 Mon Sep 17 00:00:00 2001
From: Alex Moshchuk <[email protected]>
Date: Tue, 31 Mar 2026 00:14:48 -0700
Subject: [PATCH] Filter client_side_redirect_url in BeginNavigation IPC

Unlike other navigation parameters (e.g., common_params.url,
searchable_form_url), the client_side_redirect_url in
blink::mojom::BeginNavigationParams was passed through
RenderFrameHostImpl::BeginNavigation without being validated or
filtered (e.g., by RenderProcessHost::FilterURL). This CL fixes that.

Fixed: 497365545
Change-Id: Ia0c6ad1bb493789657c4a4ec734f90cfb43805da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7713438
Commit-Queue: Alex Moshchuk <[email protected]>
Reviewed-by: Charlie Reis <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1607681}
---

diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc
index c5646213..c402f90 100644
--- a/content/browser/renderer_host/render_frame_host_impl.cc
+++ b/content/browser/renderer_host/render_frame_host_impl.cc
@@ -11565,7 +11565,9 @@
     }
   }
 
+  // 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 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 b500c32..a280237d 100644
--- a/content/browser/security_exploit_browsertest.cc
+++ b/content/browser/security_exploit_browsertest.cc
@@ -1030,6 +1030,85 @@
             child->current_frame_host()->GetLastCommittedURL());
 }
 
+// Helper class to replace a particular URL as the client_side_redirect_url
+// in BeginNavigationParams.
+class ClientSideRedirectUrlReplacer : public FrameHostInterceptor {
+ public:
+  ClientSideRedirectUrlReplacer(WebContents* web_contents,
+                                const GURL& url_to_inject)
+      : FrameHostInterceptor(web_contents), url_to_inject_(url_to_inject) {}
+
+  ClientSideRedirectUrlReplacer(const ClientSideRedirectUrlReplacer&) = delete;
+  ClientSideRedirectUrlReplacer& operator=(
+      const ClientSideRedirectUrlReplacer&) = delete;
+
+  bool WillDispatchBeginNavigation(
+      RenderFrameHost* render_frame_host,
+      blink::mojom::CommonNavigationParamsPtr* common_params,
+      blink::mojom::BeginNavigationParamsPtr* begin_params,
+      mojo::PendingRemote<blink::mojom::BlobURLToken>* blob_url_token,
+      mojo::PendingAssociatedRemote<mojom::NavigationClient>* navigation_client)
+      override {
+    if (is_activated_) {
+      (*begin_params)->client_side_redirect_url = url_to_inject_;
+      is_activated_ = false;
+    }
+    return true;
+  }
+
+  void Activate() { is_activated_ = true; }
+
+ private:
+  GURL url_to_inject_;
+  bool is_activated_ = false;
+};
+
+// 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.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
+                       BlockIllegalClientSideRedirectUrl) {
+  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());
+
+  GURL webui_url(GetWebUIURL(kChromeUIGpuHost));
+  ClientSideRedirectUrlReplacer injector(web_contents, webui_url);
+
+  // 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.
+  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)));
+
+  // Wait for the navigation to finish.
+  ASSERT_TRUE(nav_manager.WaitForNavigationFinished());
+  EXPECT_TRUE(nav_manager.was_successful());
+
+  // At this point, before the fix, the NavigationEntry has saved
+  // the WebUI URL as the OriginalRequestURL.
+
+  // 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();
+
+  // 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());
+}
+
 class RemoteFrameHostInterceptor
     : public blink::mojom::RemoteFrameHostInterceptorForTesting {
  public:

diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
index b500c32..a280237d 100644
--- a/content/browser/security_exploit_browsertest.cc
+++ b/content/browser/security_exploit_browsertest.cc
@@ -1030,6 +1030,85 @@
             child->current_frame_host()->GetLastCommittedURL());
 }
 
+// Helper class to replace a particular URL as the client_side_redirect_url
+// in BeginNavigationParams.
+class ClientSideRedirectUrlReplacer : public FrameHostInterceptor {
+ public:
+  ClientSideRedirectUrlReplacer(WebContents* web_contents,
+                                const GURL& url_to_inject)
+      : FrameHostInterceptor(web_contents), url_to_inject_(url_to_inject) {}
+
+  ClientSideRedirectUrlReplacer(const ClientSideRedirectUrlReplacer&) = delete;
+  ClientSideRedirectUrlReplacer& operator=(
+      const ClientSideRedirectUrlReplacer&) = delete;
+
+  bool WillDispatchBeginNavigation(
+      RenderFrameHost* render_frame_host,
+      blink::mojom::CommonNavigationParamsPtr* common_params,
+      blink::mojom::BeginNavigationParamsPtr* begin_params,
+      mojo::PendingRemote<blink::mojom::BlobURLToken>* blob_url_token,
+      mojo::PendingAssociatedRemote<mojom::NavigationClient>* navigation_client)
+      override {
+    if (is_activated_) {
+      (*begin_params)->client_side_redirect_url = url_to_inject_;
+      is_activated_ = false;
+    }
+    return true;
+  }
+
+  void Activate() { is_activated_ = true; }
+
+ private:
+  GURL url_to_inject_;
+  bool is_activated_ = false;
+};
+
+// 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.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
+                       BlockIllegalClientSideRedirectUrl) {
+  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());
+
+  GURL webui_url(GetWebUIURL(kChromeUIGpuHost));
+  ClientSideRedirectUrlReplacer injector(web_contents, webui_url);
+
+  // 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.
+  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)));
+
+  // Wait for the navigation to finish.
+  ASSERT_TRUE(nav_manager.WaitForNavigationFinished());
+  EXPECT_TRUE(nav_manager.was_successful());
+
+  // At this point, before the fix, the NavigationEntry has saved
+  // the WebUI URL as the OriginalRequestURL.
+
+  // 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();
+
+  // 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());
+}
+
 class RemoteFrameHostInterceptor
     : public blink::mojom::RemoteFrameHostInterceptorForTesting {
  public: