From 52ede47ddb56df2d872d9c411aee15e5a49d3288 Mon Sep 17 00:00:00 2001
From: Jilin Yang <jilinyang@google.com>
Date: Thu, 21 May 2026 21:48:50 -0700
Subject: [PATCH] Gate renderer-supplied OpenUrlInCurrentTab by scheme

A compromised chrome://newtab-footer renderer could call this Mojo
Method with any URL and get a browser-initiated navigation after only
is_valid(). Allow only https://, otherwise report a bad message;
browser-owned fixed chrome:// navigations bypass the gate via a
private OpenUrlInCurrentTabInternal.

Fixed: b:513731890
Change-Id: If1453df6796a4316c6163125ad5044f5f4145638
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7857036
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Jilin Yang <jilinyang@google.com>
Cr-Commit-Position: refs/heads/main@{#1634721}
---

diff --git a/chrome/browser/ui/webui/new_tab_footer/BUILD.gn b/chrome/browser/ui/webui/new_tab_footer/BUILD.gn
index 5ca9b24..8dcf44cf 100644
--- a/chrome/browser/ui/webui/new_tab_footer/BUILD.gn
+++ b/chrome/browser/ui/webui/new_tab_footer/BUILD.gn
@@ -56,6 +56,7 @@
     "//components/search",
     "//content/public/browser",
     "//ui/base",
+    "//url",
   ]
 }
 
@@ -84,6 +85,7 @@
     ":test_support",
     "//chrome/browser/ui/webui:webui_util",
     "//chrome/browser/ui/webui/top_chrome:test_support",
+    "//mojo/public/cpp/test_support:test_utils",
   ]
 }
 
diff --git a/chrome/browser/ui/webui/new_tab_footer/new_tab_footer_handler.cc b/chrome/browser/ui/webui/new_tab_footer/new_tab_footer_handler.cc
index cbdb448..32db2faf 100644
--- a/chrome/browser/ui/webui/new_tab_footer/new_tab_footer_handler.cc
+++ b/chrome/browser/ui/webui/new_tab_footer/new_tab_footer_handler.cc
@@ -36,6 +36,7 @@
 #include "components/prefs/pref_service.h"
 #include "components/strings/grit/components_strings.h"
 #include "content/public/browser/web_contents.h"
+#include "mojo/public/cpp/bindings/message.h"
 #include "net/base/url_util.h"
 #include "ui/base/interaction/element_identifier.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -45,6 +46,7 @@
 #include "ui/color/color_provider.h"
 #include "ui/gfx/image/image_skia_rep_default.h"
 #include "ui/gfx/paint_vector_icon.h"
+#include "url/url_constants.h"
 
 NewTabFooterHandler::NewTabFooterHandler(
     mojo::PendingReceiver<new_tab_footer::mojom::NewTabFooterHandler>
@@ -123,11 +125,11 @@
     options_url = net::AppendOrReplaceQueryParameter(options_url, "id",
                                                      curr_ntp_extension_id_);
   }
-  OpenUrlInCurrentTab(options_url);
+  OpenUrlInCurrentTabInternal(options_url);
 }
 
 void NewTabFooterHandler::OpenManagementPage() {
-  OpenUrlInCurrentTab(GURL(chrome::kChromeUIManagementURL));
+  OpenUrlInCurrentTabInternal(GURL(chrome::kChromeUIManagementURL));
 }
 
 void NewTabFooterHandler::ShowContextMenu(const gfx::Point& point) {
@@ -165,6 +167,16 @@
 }
 
 void NewTabFooterHandler::OpenUrlInCurrentTab(const GURL& url) {
+  // Mojo entry: renderer-supplied URLs become browser-initiated navigations,
+  // so only https:// is allowed; anything else is treated as a bad message.
+  if (!url.SchemeIs(url::kHttpsScheme)) {
+    mojo::ReportBadMessage("OpenUrlInCurrentTab: scheme must be https");
+    return;
+  }
+  OpenUrlInCurrentTabInternal(url);
+}
+
+void NewTabFooterHandler::OpenUrlInCurrentTabInternal(const GURL& url) {
   auto* browser_window = webui::GetBrowserWindowInterface(web_contents_);
   if (!browser_window || !url.is_valid()) {
     return;
diff --git a/chrome/browser/ui/webui/new_tab_footer/new_tab_footer_handler.h b/chrome/browser/ui/webui/new_tab_footer/new_tab_footer_handler.h
index e983fd7..1b173011 100644
--- a/chrome/browser/ui/webui/new_tab_footer/new_tab_footer_handler.h
+++ b/chrome/browser/ui/webui/new_tab_footer/new_tab_footer_handler.h
@@ -71,6 +71,8 @@
   std::string GetManagementNoticeText();
   std::string GetManagementNoticeIconDataUrl();
 
+  void OpenUrlInCurrentTabInternal(const GURL& url);
+
   // extensions::ExtensionRegistryObserver.
   void OnExtensionUnloaded(content::BrowserContext* browser_context,
                            const extensions::Extension* extension,
diff --git a/chrome/browser/ui/webui/new_tab_footer/new_tab_footer_handler_browsertest.cc b/chrome/browser/ui/webui/new_tab_footer/new_tab_footer_handler_browsertest.cc
index 27307b1..4ebb0440a 100644
--- a/chrome/browser/ui/webui/new_tab_footer/new_tab_footer_handler_browsertest.cc
+++ b/chrome/browser/ui/webui/new_tab_footer/new_tab_footer_handler_browsertest.cc
@@ -25,6 +25,8 @@
 #include "content/public/test/browser_test_utils.h"
 #include "extensions/common/extension.h"
 #include "extensions/test/test_extension_dir.h"
+#include "mojo/public/cpp/bindings/remote.h"
+#include "mojo/public/cpp/test_support/test_utils.h"
 #include "net/base/url_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/models/menu_model.h"
@@ -42,19 +44,23 @@
     InProcessBrowserTest::SetUpOnMainThread();
     embedder_ = std::make_unique<TestEmbedder>();
     handler_ = std::make_unique<NewTabFooterHandler>(
-        mojo::PendingReceiver<new_tab_footer::mojom::NewTabFooterHandler>(),
+        handler_remote_.BindNewPipeAndPassReceiver(),
         document_.BindAndGetRemote(), embedder_->GetWeakPtr(),
         NtpCustomBackgroundServiceFactory::GetForProfile(profile()),
         web_contents());
   }
 
   void TearDownOnMainThread() override {
+    handler_remote_.reset();
     handler_.reset();
     InProcessBrowserTest::TearDownOnMainThread();
   }
 
   TestEmbedder& embedder() { return *embedder_; }
   NewTabFooterHandler& handler() { return *handler_; }
+  mojo::Remote<new_tab_footer::mojom::NewTabFooterHandler>& handler_remote() {
+    return handler_remote_;
+  }
   content::WebContents* web_contents() {
     return chrome_test_utils::GetActiveWebContents(this);
   }
@@ -63,6 +69,7 @@
   base::test::ScopedFeatureList feature_list_;
   std::unique_ptr<TestEmbedder> embedder_;
   std::unique_ptr<NewTabFooterHandler> handler_;
+  mojo::Remote<new_tab_footer::mojom::NewTabFooterHandler> handler_remote_;
   testing::NiceMock<MockNewTabFooterDocument> document_;
 };
 
@@ -130,6 +137,40 @@
   EXPECT_EQ(expected_url, web_contents()->GetLastCommittedURL());
 }
 
+// One test per rejected scheme: ReportBadMessage breaks the pipe, so reusing a
+// single Remote across iterations doesn't work.
+IN_PROC_BROWSER_TEST_F(NewTabFooterHandlerBrowserTest,
+                       OpenUrlInCurrentTab_RejectsHttp) {
+  mojo::test::BadMessageObserver bad_message_observer;
+  handler_remote()->OpenUrlInCurrentTab(GURL("http://example.test/"));
+  EXPECT_EQ("OpenUrlInCurrentTab: scheme must be https",
+             bad_message_observer.WaitForBadMessage());
+}
+
+IN_PROC_BROWSER_TEST_F(NewTabFooterHandlerBrowserTest,
+                       OpenUrlInCurrentTab_RejectsWss) {
+  mojo::test::BadMessageObserver bad_message_observer;
+  handler_remote()->OpenUrlInCurrentTab(GURL("wss://example.test/"));
+  EXPECT_EQ("OpenUrlInCurrentTab: scheme must be https",
+             bad_message_observer.WaitForBadMessage());
+}
+
+IN_PROC_BROWSER_TEST_F(NewTabFooterHandlerBrowserTest,
+                       OpenUrlInCurrentTab_RejectsChromeScheme) {
+  mojo::test::BadMessageObserver bad_message_observer;
+  handler_remote()->OpenUrlInCurrentTab(GURL("chrome://version/"));
+  EXPECT_EQ("OpenUrlInCurrentTab: scheme must be https",
+             bad_message_observer.WaitForBadMessage());
+}
+
+IN_PROC_BROWSER_TEST_F(NewTabFooterHandlerBrowserTest,
+                       OpenUrlInCurrentTab_RejectsJavascript) {
+  mojo::test::BadMessageObserver bad_message_observer;
+  handler_remote()->OpenUrlInCurrentTab(GURL("javascript:void(0);"));
+  EXPECT_EQ("OpenUrlInCurrentTab: scheme must be https",
+             bad_message_observer.WaitForBadMessage());
+}
+
 IN_PROC_BROWSER_TEST_F(NewTabFooterHandlerBrowserTest, ShowContextMenu) {
   ASSERT_FALSE(embedder().context_menu_shown());
 

diff --git a/chrome/browser/ui/webui/new_tab_footer/new_tab_footer_handler_browsertest.cc b/chrome/browser/ui/webui/new_tab_footer/new_tab_footer_handler_browsertest.cc
index 27307b1..4ebb0440a 100644
--- a/chrome/browser/ui/webui/new_tab_footer/new_tab_footer_handler_browsertest.cc
+++ b/chrome/browser/ui/webui/new_tab_footer/new_tab_footer_handler_browsertest.cc
@@ -25,6 +25,8 @@
 #include "content/public/test/browser_test_utils.h"
 #include "extensions/common/extension.h"
 #include "extensions/test/test_extension_dir.h"
+#include "mojo/public/cpp/bindings/remote.h"
+#include "mojo/public/cpp/test_support/test_utils.h"
 #include "net/base/url_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/models/menu_model.h"
@@ -42,19 +44,23 @@
     InProcessBrowserTest::SetUpOnMainThread();
     embedder_ = std::make_unique<TestEmbedder>();
     handler_ = std::make_unique<NewTabFooterHandler>(
-        mojo::PendingReceiver<new_tab_footer::mojom::NewTabFooterHandler>(),
+        handler_remote_.BindNewPipeAndPassReceiver(),
         document_.BindAndGetRemote(), embedder_->GetWeakPtr(),
         NtpCustomBackgroundServiceFactory::GetForProfile(profile()),
         web_contents());
   }
 
   void TearDownOnMainThread() override {
+    handler_remote_.reset();
     handler_.reset();
     InProcessBrowserTest::TearDownOnMainThread();
   }
 
   TestEmbedder& embedder() { return *embedder_; }
   NewTabFooterHandler& handler() { return *handler_; }
+  mojo::Remote<new_tab_footer::mojom::NewTabFooterHandler>& handler_remote() {
+    return handler_remote_;
+  }
   content::WebContents* web_contents() {
     return chrome_test_utils::GetActiveWebContents(this);
   }
@@ -63,6 +69,7 @@
   base::test::ScopedFeatureList feature_list_;
   std::unique_ptr<TestEmbedder> embedder_;
   std::unique_ptr<NewTabFooterHandler> handler_;
+  mojo::Remote<new_tab_footer::mojom::NewTabFooterHandler> handler_remote_;
   testing::NiceMock<MockNewTabFooterDocument> document_;
 };
 
@@ -130,6 +137,40 @@
   EXPECT_EQ(expected_url, web_contents()->GetLastCommittedURL());
 }
 
+// One test per rejected scheme: ReportBadMessage breaks the pipe, so reusing a
+// single Remote across iterations doesn't work.
+IN_PROC_BROWSER_TEST_F(NewTabFooterHandlerBrowserTest,
+                       OpenUrlInCurrentTab_RejectsHttp) {
+  mojo::test::BadMessageObserver bad_message_observer;
+  handler_remote()->OpenUrlInCurrentTab(GURL("http://example.test/"));
+  EXPECT_EQ("OpenUrlInCurrentTab: scheme must be https",
+             bad_message_observer.WaitForBadMessage());
+}
+
+IN_PROC_BROWSER_TEST_F(NewTabFooterHandlerBrowserTest,
+                       OpenUrlInCurrentTab_RejectsWss) {
+  mojo::test::BadMessageObserver bad_message_observer;
+  handler_remote()->OpenUrlInCurrentTab(GURL("wss://example.test/"));
+  EXPECT_EQ("OpenUrlInCurrentTab: scheme must be https",
+             bad_message_observer.WaitForBadMessage());
+}
+
+IN_PROC_BROWSER_TEST_F(NewTabFooterHandlerBrowserTest,
+                       OpenUrlInCurrentTab_RejectsChromeScheme) {
+  mojo::test::BadMessageObserver bad_message_observer;
+  handler_remote()->OpenUrlInCurrentTab(GURL("chrome://version/"));
+  EXPECT_EQ("OpenUrlInCurrentTab: scheme must be https",
+             bad_message_observer.WaitForBadMessage());
+}
+
+IN_PROC_BROWSER_TEST_F(NewTabFooterHandlerBrowserTest,
+                       OpenUrlInCurrentTab_RejectsJavascript) {
+  mojo::test::BadMessageObserver bad_message_observer;
+  handler_remote()->OpenUrlInCurrentTab(GURL("javascript:void(0);"));
+  EXPECT_EQ("OpenUrlInCurrentTab: scheme must be https",
+             bad_message_observer.WaitForBadMessage());
+}
+
 IN_PROC_BROWSER_TEST_F(NewTabFooterHandlerBrowserTest, ShowContextMenu) {
   ASSERT_FALSE(embedder().context_menu_shown());