From 71365f8920c4b1af93544ed85929e3fecf058c44 Mon Sep 17 00:00:00 2001
From: Justin Lulejian <[email protected]>
Date: Thu, 25 Jun 2026 17:52:45 -0700
Subject: [PATCH] [Extensions] Block externally_connectable messaging from error pages.

Previously, a page committing as an error document could still establish
connections to externally_connectable extensions. This was because
MessageService::OpenChannelToExtension validated the connection request
against the frame's GetLastCommittedURL() but did not check if the
document was actually an error page (IsErrorDocument()). Since the last
committed URL of an error page reflects the failed navigation target,
this made it seems like a renderer hosting the error page was actually
hosting the target URL (from the perspective of messaging).

After this change, connection requests from frames that commit as error
documents are rejected.

This is accomplished by adding a check for !IsErrorDocument() when
validating externally_connectable matches in
MessageService::OpenChannelToExtension.

TAG=agy
CONV=e9ac3cb4-8403-4361-9db3-3170b15055e0

Fixed: 516813317
Change-Id: I1fad94b12ffd37d5fb0ff49271869b35966b5f8d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7994200
Auto-Submit: Justin Lulejian <[email protected]>
Reviewed-by: Andrea Orru <[email protected]>
Commit-Queue: Justin Lulejian <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1652841}
---

diff --git a/chrome/browser/extensions/extension_security_exploit_browsertest.cc b/chrome/browser/extensions/extension_security_exploit_browsertest.cc
index 5e85311..ee7528c5 100644
--- a/chrome/browser/extensions/extension_security_exploit_browsertest.cc
+++ b/chrome/browser/extensions/extension_security_exploit_browsertest.cc
@@ -48,6 +48,7 @@
 #include "extensions/common/extension_features.h"
 #include "extensions/common/mojom/event_router.mojom.h"
 #include "extensions/common/mojom/frame.mojom-test-utils.h"
+#include "extensions/common/mojom/message_port.mojom.h"
 #include "extensions/common/mojom/service_worker_host.mojom-test-utils.h"
 #include "extensions/test/extension_test_message_listener.h"
 #include "extensions/test/test_extension_dir.h"
@@ -81,6 +82,36 @@
   return response;
 }
 
+// A mock `mojom::MessagePort` implementation that allows tests to intercept
+// communication from the browser-side `MessageService` (e.g., to verify
+// connection success/failure and capture disconnect errors).
+class TestMessagePort : public mojom::MessagePort {
+ public:
+  explicit TestMessagePort(base::RepeatingClosure quit_closure)
+      : quit_closure_(std::move(quit_closure)) {}
+
+  bool got_message() const { return got_message_; }
+
+  const std::optional<std::string>& disconnect_error() const {
+    return disconnect_error_;
+  }
+
+ private:
+  void DispatchDisconnect(const std::string& error) override {
+    disconnect_error_ = error;
+    quit_closure_.Run();
+  }
+
+  void DeliverMessage(Message message) override {
+    got_message_ = true;
+    quit_closure_.Run();
+  }
+
+  bool got_message_ = false;
+  std::optional<std::string> disconnect_error_;
+  base::RepeatingClosure quit_closure_;
+};
+
 }  // namespace
 
 // ExtensionFrameHostInterceptor is a helper for:
@@ -1896,4 +1927,109 @@
   EXPECT_EQ(api_error, "Cannot call extension APIs from error pages.");
 }
 
+// Verifies that a renderer cannot send messages to an `externally_connectable`
+// extension by using a subframe error page.
+// The test navigates to a page with CSP that blocks a subframe, causing it to
+// commit as an error document. It then simulates a renderer attempting to
+// open a channel to the extension on behalf of the subframe.
+// We expect the connection to fail because the subframe is an error document.
+// Regression test for crbug.com/516813317.
+IN_PROC_BROWSER_TEST_F(OpenChannelToExtensionExploitTest,
+                       FromErrorPageSubframe_ExternallyConnectable) {
+  // Install an extension connectable only from `chromewebstore.google.com`.
+  TestExtensionDir extension_dir;
+  extension_dir.WriteManifest(R"({
+    "name": "Connectable Extension",
+    "version": "1.0",
+    "manifest_version": 3,
+    "externally_connectable": {
+      "matches": ["https://chromewebstore.google.com/*"]
+    },
+    "background": {"service_worker": "background.js"}
+  })");
+  extension_dir.WriteFile(FILE_PATH_LITERAL("background.js"), R"(
+    chrome.runtime.onConnectExternal.addListener((port) => {
+      chrome.test.sendMessage("connected");
+      port.postMessage("hello");
+    });
+  )");
+  const Extension* extension = LoadExtension(extension_dir.UnpackedPath());
+  ASSERT_TRUE(extension);
+
+  ExtensionTestMessageListener connect_listener("connected");
+
+  // Navigate to a.com with a CSP blocking a subframe loading
+  // https://chromewebstore.google.com/.
+  const GURL victim_url("https://chromewebstore.google.com/");
+  GURL test_page_url =
+      embedded_test_server()->GetURL("a.com", "/csp_block.html");
+
+  auto* web_contents = GetActiveWebContents();
+  // NavigateToURL() only waits for the main frame navigation to commit. We
+  // must use `TestNavigationObserver` to wait for the subframe navigation to
+  // finish committing as an error document.
+  content::TestNavigationObserver navigation_observer(web_contents);
+  std::ignore = NavigateToURL(web_contents, test_page_url);
+  {
+    SCOPED_TRACE("Waiting for CSP block navigation to complete");
+    navigation_observer.Wait();
+  }
+
+  content::RenderFrameHost* main_frame = web_contents->GetPrimaryMainFrame();
+  content::RenderFrameHost* subframe =
+      content::ChildFrameAt(main_frame, /*index=*/0);
+  ASSERT_TRUE(subframe);
+
+  // Verify the subframe commits as an error document in the parent's process
+  // with `GetLastCommittedURL()` returning
+  // "https://chromewebstore.google.com/".
+  EXPECT_TRUE(subframe->IsErrorDocument());
+  EXPECT_EQ(subframe->GetLastCommittedURL(), victim_url);
+  EXPECT_EQ(main_frame->GetProcess(), subframe->GetProcess());
+
+  // Simulate the renderer opening a channel to the extension on behalf of the
+  // subframe using `MessageService::OpenChannelToExtension`.
+  content::RenderProcessHost* process = subframe->GetProcess();
+  ChannelEndpoint source(profile(), process->GetDeprecatedID(),
+                         PortContext::ForFrame(subframe->GetRoutingID()));
+
+  // Set up the message channel endpoints.
+  PortId source_port_id(base::UnguessableToken::Create(), /*port_number=*/0,
+                        /*is_opener=*/true, mojom::SerializationFormat::kJson);
+
+  mojom::ExternalConnectionInfo info;
+  info.target_id = extension->id();
+  info.source_endpoint = MessagingEndpoint::ForWebPage();
+  info.source_url = test_page_url;
+
+  // Bind the test-side port receiver to capture the connection response.
+  base::RunLoop run_loop;
+  TestMessagePort test_port(run_loop.QuitClosure());
+  mojo::AssociatedReceiver<mojom::MessagePort> port_receiver(&test_port);
+
+  // Create endpoints. Use `BindNewEndpointAndPassDedicatedRemote()` to avoid
+  // Mojo association crashes when simulating the IPC directly.
+  mojo::PendingAssociatedRemote<mojom::MessagePort> port_remote =
+      port_receiver.BindNewEndpointAndPassDedicatedRemote();
+
+  mojo::PendingAssociatedReceiver<mojom::MessagePortHost> port_host_receiver;
+  auto port_host_remote = port_host_receiver.InitWithNewEndpointAndPassRemote();
+
+  // Simulate the renderer calling OpenChannelToExtension directly.
+  MessageService::Get(profile())->OpenChannelToExtension(
+      source, source_port_id, info, mojom::ChannelType::kConnect,
+      "test_channel", std::move(port_remote), std::move(port_host_receiver));
+
+  // We expect the connection to fail since error documents shouldn't be able
+  // to send messages.
+  {
+    SCOPED_TRACE("Waiting for port disconnection");
+    run_loop.Run();
+  }
+  EXPECT_FALSE(connect_listener.was_satisfied());
+  EXPECT_FALSE(test_port.got_message());
+  EXPECT_EQ("Could not establish connection. Receiving end does not exist.",
+            test_port.disconnect_error());
+}
+
 }  // namespace extensions
diff --git a/extensions/browser/api/messaging/message_service.cc b/extensions/browser/api/messaging/message_service.cc
index 2a4b0942..8be9b3fa 100644
--- a/extensions/browser/api/messaging/message_service.cc
+++ b/extensions/browser/api/messaging/message_service.cc
@@ -542,9 +542,13 @@
         DCHECK_EQ(MessagingEndpoint::Relationship::kExternalWebPage,
                   relationship);
 

diff --git a/chrome/browser/extensions/extension_security_exploit_browsertest.cc b/chrome/browser/extensions/extension_security_exploit_browsertest.cc
index 5e85311..ee7528c5 100644
--- a/chrome/browser/extensions/extension_security_exploit_browsertest.cc
+++ b/chrome/browser/extensions/extension_security_exploit_browsertest.cc
@@ -48,6 +48,7 @@
 #include "extensions/common/extension_features.h"
 #include "extensions/common/mojom/event_router.mojom.h"
 #include "extensions/common/mojom/frame.mojom-test-utils.h"
+#include "extensions/common/mojom/message_port.mojom.h"
 #include "extensions/common/mojom/service_worker_host.mojom-test-utils.h"
 #include "extensions/test/extension_test_message_listener.h"
 #include "extensions/test/test_extension_dir.h"
@@ -81,6 +82,36 @@
   return response;
 }
 
+// A mock `mojom::MessagePort` implementation that allows tests to intercept
+// communication from the browser-side `MessageService` (e.g., to verify
+// connection success/failure and capture disconnect errors).
+class TestMessagePort : public mojom::MessagePort {
+ public:
+  explicit TestMessagePort(base::RepeatingClosure quit_closure)
+      : quit_closure_(std::move(quit_closure)) {}
+
+  bool got_message() const { return got_message_; }
+
+  const std::optional<std::string>& disconnect_error() const {
+    return disconnect_error_;
+  }
+
+ private:
+  void DispatchDisconnect(const std::string& error) override {
+    disconnect_error_ = error;
+    quit_closure_.Run();
+  }
+
+  void DeliverMessage(Message message) override {
+    got_message_ = true;
+    quit_closure_.Run();
+  }
+
+  bool got_message_ = false;
+  std::optional<std::string> disconnect_error_;
+  base::RepeatingClosure quit_closure_;
+};
+
 }  // namespace
 
 // ExtensionFrameHostInterceptor is a helper for:
@@ -1896,4 +1927,109 @@
   EXPECT_EQ(api_error, "Cannot call extension APIs from error pages.");
 }
 
+// Verifies that a renderer cannot send messages to an `externally_connectable`
+// extension by using a subframe error page.
+// The test navigates to a page with CSP that blocks a subframe, causing it to
+// commit as an error document. It then simulates a renderer attempting to
+// open a channel to the extension on behalf of the subframe.
+// We expect the connection to fail because the subframe is an error document.
+// Regression test for crbug.com/516813317.
+IN_PROC_BROWSER_TEST_F(OpenChannelToExtensionExploitTest,
+                       FromErrorPageSubframe_ExternallyConnectable) {
+  // Install an extension connectable only from `chromewebstore.google.com`.
+  TestExtensionDir extension_dir;
+  extension_dir.WriteManifest(R"({
+    "name": "Connectable Extension",
+    "version": "1.0",
+    "manifest_version": 3,
+    "externally_connectable": {
+      "matches": ["https://chromewebstore.google.com/*"]
+    },
+    "background": {"service_worker": "background.js"}
+  })");
+  extension_dir.WriteFile(FILE_PATH_LITERAL("background.js"), R"(
+    chrome.runtime.onConnectExternal.addListener((port) => {
+      chrome.test.sendMessage("connected");
+      port.postMessage("hello");
+    });
+  )");
+  const Extension* extension = LoadExtension(extension_dir.UnpackedPath());
+  ASSERT_TRUE(extension);
+
+  ExtensionTestMessageListener connect_listener("connected");
+
+  // Navigate to a.com with a CSP blocking a subframe loading
+  // https://chromewebstore.google.com/.
+  const GURL victim_url("https://chromewebstore.google.com/");
+  GURL test_page_url =
+      embedded_test_server()->GetURL("a.com", "/csp_block.html");
+
+  auto* web_contents = GetActiveWebContents();
+  // NavigateToURL() only waits for the main frame navigation to commit. We
+  // must use `TestNavigationObserver` to wait for the subframe navigation to
+  // finish committing as an error document.
+  content::TestNavigationObserver navigation_observer(web_contents);
+  std::ignore = NavigateToURL(web_contents, test_page_url);
+  {
+    SCOPED_TRACE("Waiting for CSP block navigation to complete");
+    navigation_observer.Wait();
+  }
+
+  content::RenderFrameHost* main_frame = web_contents->GetPrimaryMainFrame();
+  content::RenderFrameHost* subframe =
+      content::ChildFrameAt(main_frame, /*index=*/0);
+  ASSERT_TRUE(subframe);
+
+  // Verify the subframe commits as an error document in the parent's process
+  // with `GetLastCommittedURL()` returning
+  // "https://chromewebstore.google.com/".
+  EXPECT_TRUE(subframe->IsErrorDocument());
+  EXPECT_EQ(subframe->GetLastCommittedURL(), victim_url);
+  EXPECT_EQ(main_frame->GetProcess(), subframe->GetProcess());
+
+  // Simulate the renderer opening a channel to the extension on behalf of the
+  // subframe using `MessageService::OpenChannelToExtension`.
+  content::RenderProcessHost* process = subframe->GetProcess();
+  ChannelEndpoint source(profile(), process->GetDeprecatedID(),
+                         PortContext::ForFrame(subframe->GetRoutingID()));
+
+  // Set up the message channel endpoints.
+  PortId source_port_id(base::UnguessableToken::Create(), /*port_number=*/0,
+                        /*is_opener=*/true, mojom::SerializationFormat::kJson);
+
+  mojom::ExternalConnectionInfo info;
+  info.target_id = extension->id();
+  info.source_endpoint = MessagingEndpoint::ForWebPage();
+  info.source_url = test_page_url;
+
+  // Bind the test-side port receiver to capture the connection response.
+  base::RunLoop run_loop;
+  TestMessagePort test_port(run_loop.QuitClosure());
+  mojo::AssociatedReceiver<mojom::MessagePort> port_receiver(&test_port);
+
+  // Create endpoints. Use `BindNewEndpointAndPassDedicatedRemote()` to avoid
+  // Mojo association crashes when simulating the IPC directly.
+  mojo::PendingAssociatedRemote<mojom::MessagePort> port_remote =
+      port_receiver.BindNewEndpointAndPassDedicatedRemote();
+
+  mojo::PendingAssociatedReceiver<mojom::MessagePortHost> port_host_receiver;
+  auto port_host_remote = port_host_receiver.InitWithNewEndpointAndPassRemote();
+
+  // Simulate the renderer calling OpenChannelToExtension directly.
+  MessageService::Get(profile())->OpenChannelToExtension(
+      source, source_port_id, info, mojom::ChannelType::kConnect,
+      "test_channel", std::move(port_remote), std::move(port_host_receiver));
+
+  // We expect the connection to fail since error documents shouldn't be able
+  // to send messages.
+  {
+    SCOPED_TRACE("Waiting for port disconnection");
+    run_loop.Run();
+  }
+  EXPECT_FALSE(connect_listener.was_satisfied());
+  EXPECT_FALSE(test_port.got_message());
+  EXPECT_EQ("Could not establish connection. Receiving end does not exist.",
+            test_port.disconnect_error());
+}
+
 }  // namespace extensions