From f72d1060ae3b9a755cf6760dc116a42cb04e9989 Mon Sep 17 00:00:00 2001
From: Charlie Reis <creis@chromium.org>
Date: Fri, 24 Jul 2026 08:54:51 -0700
Subject: [PATCH] Prevent non-current and cross-BCG openers in DidChangeOpener.

This CL fixes the opener BCG comparison in DidChangeOpener and resolves
a corner case from IsInactiveAndDisallowActivation, with test coverage
for both.

Fixed: 516899248
Change-Id: Ied80d9ccba9eb2cafdecd405af97fb39bdf60a4b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8139403
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1667891}
---

diff --git a/content/browser/renderer_host/render_frame_host_manager.cc b/content/browser/renderer_host/render_frame_host_manager.cc
index 80f0533..f09e7143 100644
--- a/content/browser/renderer_host/render_frame_host_manager.cc
+++ b/content/browser/renderer_host/render_frame_host_manager.cc
@@ -1198,13 +1198,21 @@
     if (opener_rfhi) {
       // Ignore this message if |opener_rfhi| is inactive (e.g., in BFCache or
       // pending deletion), or if the FrameTreeNode's current RenderFrameHost
-      // is in a different BrowsingInstance, as it would be incorrect to
-      // establish an opener relationship in those cases.
+      // is in a different BrowsingInstance from the new opener, as it would be
+      // incorrect to establish an opener relationship in those cases.
+      //
+      // Note that `IsInactiveAndDisallowActivation` may return false for
+      // speculative and pending commit RFHs (while killing their renderer
+      // process), because this case is unexpected except for compromised
+      // renderers. As a result, also confirm that opener_rfhi is the current
+      // frame of its FTN, to catch speculative and pending commit cases.
       if (opener_rfhi->IsInactiveAndDisallowActivation(
               DisallowActivationReasonId::kDidChangeOpener) ||
+          opener_rfhi != opener_rfhi->frame_tree_node()->current_frame_host() ||
           !render_frame_host_->GetSiteInstance()
                ->group()
-               ->IsRelatedSiteInstanceGroup(source_site_instance_group)) {
+               ->IsRelatedSiteInstanceGroup(
+                   opener_rfhi->GetSiteInstance()->group())) {
         return;
       }
       opener = opener_rfhi->frame_tree_node();
diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
index c76fe77..36f23d8 100644
--- a/content/browser/security_exploit_browsertest.cc
+++ b/content/browser/security_exploit_browsertest.cc
@@ -2497,6 +2497,78 @@
             kill_waiter.Wait());
 }
 
+// Ensure that DidChangeOpener IPC messages do not succeed if they attempt to
+// assign an opener from a different Browsing Context Group (BCG).
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
+                       OpenerChangeDisallowedAcrossBrowsingInstances) {
+  // Explicitly isolate a.test to ensure that this test is applicable on
+  // platforms without site-per-process.
+  IsolateOrigin("a.test");
+
+  // Force process reuse so tab 1 and tab 2 are in the same renderer process.
+  // This guarantees that the opener token (A1) is valid in the same process as
+  // the proxy in A2's SiteInstance that will be used to send the spoofed
+  // DidChangeOpener IPC later.
+  RenderProcessHost::SetMaxRendererProcessCount(1);
+
+  // Navigate to A1.
+  EXPECT_TRUE(NavigateToURL(
+      shell(), embedded_test_server()->GetURL("a.test", "/title1.html")));
+
+  WebContentsImpl* web_contents =
+      static_cast<WebContentsImpl*>(shell()->web_contents());
+  RenderFrameHostImpl* tab1_rfh = web_contents->GetPrimaryMainFrame();
+  FrameTreeNode* tab1_root = tab1_rfh->frame_tree_node();
+
+  Shell* tab2 = CreateBrowser();
+  // Create an unrelated window and navigate it to A2. Workaround: navigate
+  // `tab2` to a random site before navigating to attacker's site to ensure that
+  // `tab2` shares the process with tab1, rather than staying in its initial
+  // process.
+  ASSERT_TRUE(NavigateToURL(
+      tab2, embedded_test_server()->GetURL("d.test", "/title3.html")));
+  ASSERT_TRUE(NavigateToURL(
+      tab2, embedded_test_server()->GetURL("a.test", "/title2.html")));
+  RenderFrameHostImpl* tab2_rfh = static_cast<RenderFrameHostImpl*>(
+      tab2->web_contents()->GetPrimaryMainFrame());
+  FrameTreeNode* tab2_root = tab2_rfh->frame_tree_node();
+  EXPECT_FALSE(tab1_rfh->GetSiteInstance()->IsRelatedSiteInstance(
+      tab2_rfh->GetSiteInstance()));
+  EXPECT_EQ(tab1_rfh->GetProcess(), tab2_rfh->GetProcess());
+
+  // Open a cross-site page B in the same BCG as A2.
+  GURL cross_site_url(embedded_test_server()->GetURL("b.test", "/title3.html"));
+  Shell* tab3 = OpenPopup(tab2, cross_site_url, "foo");
+  RenderFrameHostImpl* tab3_rfh = static_cast<RenderFrameHostImpl*>(
+      tab3->web_contents()->GetPrimaryMainFrame());
+  FrameTreeNode* tab3_root = tab3_rfh->frame_tree_node();
+  EXPECT_EQ(tab3_root->opener(), tab2_root);
+  EXPECT_TRUE(tab2_rfh->GetSiteInstance()->IsRelatedSiteInstance(
+      tab3_rfh->GetSiteInstance()));
+
+  // Grab the token of the A1 document that will be set as the new opener.
+  blink::LocalFrameToken opener_token = tab1_rfh->GetFrameToken();
+
+  // Look up the proxy that represents B in A2's SiteInstance. This is the
+  // target of the malicious DidChangeOpener IPC. This should exist as long as
+  // a.test and b.test are process-isolated from each other.
+  RenderFrameProxyHost* proxy =
+      tab3_rfh->browsing_context_state()->GetRenderFrameProxyHost(
+          tab2_rfh->GetSiteInstance()->group());
+  ASSERT_TRUE(proxy);
+
+  // Simulate the vulnerability by sending the DidChangeOpener IPC using the
+  // token of the frame in the wrong BCG. This goes to the
+  // RenderFrameHostManager of tab3, trying to set its opener to the FTN that's
+  // in the other BCG.
+  proxy->DidChangeOpener(opener_token);
+
+  // The opener should *not* be updated to tab1_root. It should stay the same,
+  // still pointing to tab2.
+  EXPECT_NE(tab3_root->opener(), tab1_root);
+  EXPECT_EQ(tab3_root->opener(), tab2_root);
+}
+
 // Verify that a compromised renderer can't use a BFCached page and a rogue
 // DidChangeOpener IPC to establish an opener relationship with a
 // cross-BrowsingInstance page.
@@ -2510,7 +2582,7 @@
 // opener to A1. A1 is bfcached, and the test ensures that the opener isn't set
 // to the WebUI page which is the current RFH in A1's FTN.
 IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
-                       OpenerChangeDisallowedAcrossBrowsingInstances) {
+                       OpenerChangeDisallowedAcrossBrowsingInstances_Bfcache) {
   // This test assumes that back-forward cache is enabled.
   if (!IsBackForwardCacheEnabled()) {
     GTEST_SKIP();
@@ -2612,6 +2684,85 @@
   EXPECT_FALSE(webui_to_non_webui_proxy);
 }
 
+// Similar to OpenerChangeDisallowedAcrossBrowsingInstances_Bfcache, but
+// exercises the case where the supplied opener token resolves to a speculative
+// RenderFrameHost (which happens to be in the popup tab itself). This should
+// fail even within the same BCG because the new opener is still in the
+// speculative state and not the current frame of its FrameTreeNode.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
+                       OpenerChangeDisallowedViaSpeculativeFrame) {
+  // Explicitly isolate a.test to ensure that this test is applicable on
+  // platforms without site-per-process.
+  IsolateOrigin("a.test");
+
+  // Navigate to A1.
+  EXPECT_TRUE(NavigateToURL(
+      shell(), embedded_test_server()->GetURL("a.test", "/title1.html")));
+
+  WebContentsImpl* web_contents =
+      static_cast<WebContentsImpl*>(shell()->web_contents());
+  RenderFrameHostImpl* tab1_rfh = web_contents->GetPrimaryMainFrame();
+  FrameTreeNode* tab1_root = tab1_rfh->frame_tree_node();
+
+  // Open A2 in the same BCG as A1.
+  GURL popup_url(embedded_test_server()->GetURL("a.test", "/title2.html"));
+  Shell* tab2 = OpenPopup(shell(), popup_url, "foo");
+  RenderFrameHostImpl* tab2_rfh = static_cast<RenderFrameHostImpl*>(
+      tab2->web_contents()->GetPrimaryMainFrame());
+  FrameTreeNode* tab2_root = tab2_rfh->frame_tree_node();
+  EXPECT_EQ(tab2_root->opener(), tab1_root);
+  EXPECT_TRUE(tab1_rfh->GetSiteInstance()->IsRelatedSiteInstance(
+      tab2_rfh->GetSiteInstance()));
+
+  // Navigate tab 2 to B in the same BCG.
+  GURL b_url(embedded_test_server()->GetURL("b.test", "/title1.html"));
+  {
+    TestNavigationObserver nav_observer(tab2->web_contents(), 1);
+    EXPECT_TRUE(ExecJs(tab2, JsReplace("location.href = $1;", b_url)));
+    nav_observer.Wait();
+  }
+  tab2_rfh = static_cast<RenderFrameHostImpl*>(
+      tab2->web_contents()->GetPrimaryMainFrame());
+  EXPECT_EQ(tab2_root->opener(), tab1_root);
+  EXPECT_TRUE(tab1_rfh->GetSiteInstance()->IsRelatedSiteInstance(
+      tab2_rfh->GetSiteInstance()));
+
+  // Start a cross-process navigation in tab 2 back to A, and pause it once the
+  // speculative RenderFrameHost has been created.
+  GURL dest_url(embedded_test_server()->GetURL("a.test", "/title3.html"));
+  TestNavigationManager nav_manager(tab2->web_contents(), dest_url);
+  EXPECT_TRUE(ExecJs(tab2, JsReplace("location.href = $1;", dest_url)));
+  nav_manager.WaitForSpeculativeRenderFrameHostCreation();
+  RenderFrameHostImpl* speculative_rfh =
+      tab2_root->render_manager()->speculative_frame_host();
+  ASSERT_TRUE(speculative_rfh);
+  ASSERT_EQ(speculative_rfh->GetProcess(), tab1_rfh->GetProcess());
+  ASSERT_EQ(tab2_root->current_frame_host(), tab2_rfh);
+
+  // Find SiteInstance A's proxy for tab 2, as the target for the IPC.
+  RenderFrameProxyHost* proxy =
+      tab2_rfh->browsing_context_state()->GetRenderFrameProxyHost(

diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
index c76fe77..36f23d8 100644
--- a/content/browser/security_exploit_browsertest.cc
+++ b/content/browser/security_exploit_browsertest.cc
@@ -2497,6 +2497,78 @@
             kill_waiter.Wait());
 }
 
+// Ensure that DidChangeOpener IPC messages do not succeed if they attempt to
+// assign an opener from a different Browsing Context Group (BCG).
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
+                       OpenerChangeDisallowedAcrossBrowsingInstances) {
+  // Explicitly isolate a.test to ensure that this test is applicable on
+  // platforms without site-per-process.
+  IsolateOrigin("a.test");
+
+  // Force process reuse so tab 1 and tab 2 are in the same renderer process.
+  // This guarantees that the opener token (A1) is valid in the same process as
+  // the proxy in A2's SiteInstance that will be used to send the spoofed
+  // DidChangeOpener IPC later.
+  RenderProcessHost::SetMaxRendererProcessCount(1);
+
+  // Navigate to A1.
+  EXPECT_TRUE(NavigateToURL(
+      shell(), embedded_test_server()->GetURL("a.test", "/title1.html")));
+
+  WebContentsImpl* web_contents =
+      static_cast<WebContentsImpl*>(shell()->web_contents());
+  RenderFrameHostImpl* tab1_rfh = web_contents->GetPrimaryMainFrame();
+  FrameTreeNode* tab1_root = tab1_rfh->frame_tree_node();
+
+  Shell* tab2 = CreateBrowser();
+  // Create an unrelated window and navigate it to A2. Workaround: navigate
+  // `tab2` to a random site before navigating to attacker's site to ensure that
+  // `tab2` shares the process with tab1, rather than staying in its initial
+  // process.
+  ASSERT_TRUE(NavigateToURL(
+      tab2, embedded_test_server()->GetURL("d.test", "/title3.html")));
+  ASSERT_TRUE(NavigateToURL(
+      tab2, embedded_test_server()->GetURL("a.test", "/title2.html")));
+  RenderFrameHostImpl* tab2_rfh = static_cast<RenderFrameHostImpl*>(
+      tab2->web_contents()->GetPrimaryMainFrame());
+  FrameTreeNode* tab2_root = tab2_rfh->frame_tree_node();
+  EXPECT_FALSE(tab1_rfh->GetSiteInstance()->IsRelatedSiteInstance(
+      tab2_rfh->GetSiteInstance()));
+  EXPECT_EQ(tab1_rfh->GetProcess(), tab2_rfh->GetProcess());
+
+  // Open a cross-site page B in the same BCG as A2.
+  GURL cross_site_url(embedded_test_server()->GetURL("b.test", "/title3.html"));
+  Shell* tab3 = OpenPopup(tab2, cross_site_url, "foo");
+  RenderFrameHostImpl* tab3_rfh = static_cast<RenderFrameHostImpl*>(
+      tab3->web_contents()->GetPrimaryMainFrame());
+  FrameTreeNode* tab3_root = tab3_rfh->frame_tree_node();
+  EXPECT_EQ(tab3_root->opener(), tab2_root);
+  EXPECT_TRUE(tab2_rfh->GetSiteInstance()->IsRelatedSiteInstance(
+      tab3_rfh->GetSiteInstance()));
+
+  // Grab the token of the A1 document that will be set as the new opener.
+  blink::LocalFrameToken opener_token = tab1_rfh->GetFrameToken();
+
+  // Look up the proxy that represents B in A2's SiteInstance. This is the
+  // target of the malicious DidChangeOpener IPC. This should exist as long as
+  // a.test and b.test are process-isolated from each other.
+  RenderFrameProxyHost* proxy =
+      tab3_rfh->browsing_context_state()->GetRenderFrameProxyHost(
+          tab2_rfh->GetSiteInstance()->group());
+  ASSERT_TRUE(proxy);
+
+  // Simulate the vulnerability by sending the DidChangeOpener IPC using the
+  // token of the frame in the wrong BCG. This goes to the
+  // RenderFrameHostManager of tab3, trying to set its opener to the FTN that's
+  // in the other BCG.
+  proxy->DidChangeOpener(opener_token);
+
+  // The opener should *not* be updated to tab1_root. It should stay the same,
+  // still pointing to tab2.
+  EXPECT_NE(tab3_root->opener(), tab1_root);
+  EXPECT_EQ(tab3_root->opener(), tab2_root);
+}
+
 // Verify that a compromised renderer can't use a BFCached page and a rogue
 // DidChangeOpener IPC to establish an opener relationship with a
 // cross-BrowsingInstance page.
@@ -2510,7 +2582,7 @@
 // opener to A1. A1 is bfcached, and the test ensures that the opener isn't set
 // to the WebUI page which is the current RFH in A1's FTN.
 IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
-                       OpenerChangeDisallowedAcrossBrowsingInstances) {
+                       OpenerChangeDisallowedAcrossBrowsingInstances_Bfcache) {
   // This test assumes that back-forward cache is enabled.
   if (!IsBackForwardCacheEnabled()) {
     GTEST_SKIP();
@@ -2612,6 +2684,85 @@
   EXPECT_FALSE(webui_to_non_webui_proxy);
 }
 
+// Similar to OpenerChangeDisallowedAcrossBrowsingInstances_Bfcache, but
+// exercises the case where the supplied opener token resolves to a speculative
+// RenderFrameHost (which happens to be in the popup tab itself). This should
+// fail even within the same BCG because the new opener is still in the
+// speculative state and not the current frame of its FrameTreeNode.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
+                       OpenerChangeDisallowedViaSpeculativeFrame) {
+  // Explicitly isolate a.test to ensure that this test is applicable on
+  // platforms without site-per-process.
+  IsolateOrigin("a.test");
+
+  // Navigate to A1.
+  EXPECT_TRUE(NavigateToURL(
+      shell(), embedded_test_server()->GetURL("a.test", "/title1.html")));
+
+  WebContentsImpl* web_contents =
+      static_cast<WebContentsImpl*>(shell()->web_contents());
+  RenderFrameHostImpl* tab1_rfh = web_contents->GetPrimaryMainFrame();
+  FrameTreeNode* tab1_root = tab1_rfh->frame_tree_node();
+
+  // Open A2 in the same BCG as A1.
+  GURL popup_url(embedded_test_server()->GetURL("a.test", "/title2.html"));
+  Shell* tab2 = OpenPopup(shell(), popup_url, "foo");
+  RenderFrameHostImpl* tab2_rfh = static_cast<RenderFrameHostImpl*>(
+      tab2->web_contents()->GetPrimaryMainFrame());
+  FrameTreeNode* tab2_root = tab2_rfh->frame_tree_node();
+  EXPECT_EQ(tab2_root->opener(), tab1_root);
+  EXPECT_TRUE(tab1_rfh->GetSiteInstance()->IsRelatedSiteInstance(
+      tab2_rfh->GetSiteInstance()));
+
+  // Navigate tab 2 to B in the same BCG.
+  GURL b_url(embedded_test_server()->GetURL("b.test", "/title1.html"));
+  {
+    TestNavigationObserver nav_observer(tab2->web_contents(), 1);
+    EXPECT_TRUE(ExecJs(tab2, JsReplace("location.href = $1;", b_url)));
+    nav_observer.Wait();
+  }
+  tab2_rfh = static_cast<RenderFrameHostImpl*>(
+      tab2->web_contents()->GetPrimaryMainFrame());
+  EXPECT_EQ(tab2_root->opener(), tab1_root);
+  EXPECT_TRUE(tab1_rfh->GetSiteInstance()->IsRelatedSiteInstance(
+      tab2_rfh->GetSiteInstance()));
+
+  // Start a cross-process navigation in tab 2 back to A, and pause it once the
+  // speculative RenderFrameHost has been created.
+  GURL dest_url(embedded_test_server()->GetURL("a.test", "/title3.html"));
+  TestNavigationManager nav_manager(tab2->web_contents(), dest_url);
+  EXPECT_TRUE(ExecJs(tab2, JsReplace("location.href = $1;", dest_url)));
+  nav_manager.WaitForSpeculativeRenderFrameHostCreation();
+  RenderFrameHostImpl* speculative_rfh =
+      tab2_root->render_manager()->speculative_frame_host();
+  ASSERT_TRUE(speculative_rfh);
+  ASSERT_EQ(speculative_rfh->GetProcess(), tab1_rfh->GetProcess());
+  ASSERT_EQ(tab2_root->current_frame_host(), tab2_rfh);
+
+  // Find SiteInstance A's proxy for tab 2, as the target for the IPC.
+  RenderFrameProxyHost* proxy =
+      tab2_rfh->browsing_context_state()->GetRenderFrameProxyHost(
+          tab1_rfh->GetSiteInstance()->group());
+  ASSERT_TRUE(proxy);
+
+  // Grab the speculative RenderFrameHost's frame token and simulate a rogue
+  // DidChangeOpener IPC that supplies it as the new opener for tab 2, which
+  // would (legally) create a cycle if it succeeds, but should fail because the
+  // token is for a speculative RFH.
+  blink::LocalFrameToken opener_token = speculative_rfh->GetFrameToken();
+  RenderProcessHostBadIpcMessageWaiter kill_waiter(tab1_rfh->GetProcess());
+  proxy->DidChangeOpener(opener_token);
+
+  // The opener should *not* be updated to tab2_root, since the frame token is
+  // for a speculative RFH.
+  EXPECT_EQ(tab2_root->opener(), tab1_root);
+
+  // The renderer that supplied a speculative frame's token as opener should
+  // be terminated.
+  EXPECT_EQ(bad_message::RFH_INACTIVE_CHECK_FROM_SPECULATIVE_RFH,
+            kill_waiter.Wait());
+}
+
 namespace {
 
 // An interceptor class that allows replacing the URL of the commit IPC from