From 2b5169f2383cf64a28ea5928c4d5ab756436cbbc Mon Sep 17 00:00:00 2001
From: Alex Moshchuk <alexmos@chromium.org>
Date: Wed, 01 Apr 2026 13:30:28 -0700
Subject: [PATCH] Block inactive frames from updating frame opener via DidChangeOpener

When processing DidChangeOpener IPCs from a renderer, the browser
process resolves the provided opener frame token to a
RenderFrameHostImpl. Previously, this logic did not verify the lifecycle
state of the resolved RenderFrameHost or check whether it is in a
different BrowsingInstance.

A compromised renderer could exploit this by supplying the frame token
of a RenderFrameHost that is in BFCache or pending deletion. This can be
used to trick the browser to establish a cross-BrowsingInstance opener
relationship, if the current RFH of the inactive RenderFrameHost's
FrameTreeNode is in a different BrowsingInstance. This is particularly
bad if the current RFH is hosting a COOP, cross-origin isolated, or
WebUI page, breaking the BrowsingInstance isolation guarantees.

This CL fixes this by updating RenderFrameHostManager::DidChangeOpener
to explicitly ignore the opener change request if the opener's RFH is
not active or if it's in a different BrowsingInstance.

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

diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc
index 2ff622f..da96f61e 100644
--- a/content/browser/renderer_host/render_frame_host_impl.cc
+++ b/content/browser/renderer_host/render_frame_host_impl.cc
@@ -9706,6 +9706,8 @@
     return;
   }
 
+  // Note that this call internally protects against `opener_frame_token`
+  // referring to an inactive frame.
   owner_->GetRenderFrameHostManager().DidChangeOpener(
       opener_frame_token, GetSiteInstance()->group());
 }
diff --git a/content/browser/renderer_host/render_frame_host_manager.cc b/content/browser/renderer_host/render_frame_host_manager.cc
index 79b772e..1e76d25a 100644
--- a/content/browser/renderer_host/render_frame_host_manager.cc
+++ b/content/browser/renderer_host/render_frame_host_manager.cc
@@ -1110,8 +1110,20 @@
         *opener_frame_token);
     // If |opener_rfhi| is null, the opener RFH has already disappeared.  In
     // this case, clear the opener rather than keeping the old opener around.
-    if (opener_rfhi)
+    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.
+      if (opener_rfhi->IsInactiveAndDisallowActivation(
+              DisallowActivationReasonId::kDidChangeOpener) ||
+          !render_frame_host_->GetSiteInstance()
+               ->group()
+               ->IsRelatedSiteInstanceGroup(source_site_instance_group)) {
+        return;
+      }
       opener = opener_rfhi->frame_tree_node();
+    }
   }
 
   if (frame_tree_node_->opener() == opener)
diff --git a/content/browser/renderer_host/render_frame_host_manager_browsertest.cc b/content/browser/renderer_host/render_frame_host_manager_browsertest.cc
index 506bd7f4..95e196f6 100644
--- a/content/browser/renderer_host/render_frame_host_manager_browsertest.cc
+++ b/content/browser/renderer_host/render_frame_host_manager_browsertest.cc
@@ -5957,6 +5957,72 @@
   }
 }
 
+// Tests that a renderer cannot change the opener of a frame across different
+// BrowsingInstances (e.g., when trying to reconnect to a named window that
+// has navigated to a WebUI URL).
+IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest,
+                       CannotChangeOpenerAcrossBrowsingInstances) {
+  StartEmbeddedServer();
+
+  GURL url_a1(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  GURL url_a2(embedded_test_server()->GetURL("a.com", "/title2.html"));
+  GURL url_a3(embedded_test_server()->GetURL("a.com", "/title3.html"));
+  GURL webui_url = GetWebUIURL(kChromeUIGpuHost);
+
+  EXPECT_TRUE(NavigateToURL(shell(), url_a1));
+  WebContentsImpl* web_contents_1 =
+      static_cast<WebContentsImpl*>(shell()->web_contents());
+  FrameTreeNode* ftn_1 = web_contents_1->GetPrimaryFrameTree().root();
+  RenderFrameHostImpl* rfh_1 = ftn_1->current_frame_host();
+
+  // Use window.open() to open a popup from A1 to A2 named "foo", and another
+  // unnamed popup from A1 to A3.
+  Shell* tab_2 = OpenPopup(rfh_1, url_a2, "foo");
+  Shell* tab_3 = OpenPopup(rfh_1, url_a3, "");
+
+  WebContentsImpl* web_contents_2 =
+      static_cast<WebContentsImpl*>(tab_2->web_contents());
+  FrameTreeNode* ftn_2 = web_contents_2->GetPrimaryFrameTree().root();
+  WebContentsImpl* web_contents_3 =
+      static_cast<WebContentsImpl*>(tab_3->web_contents());
+  FrameTreeNode* ftn_3 = web_contents_3->GetPrimaryFrameTree().root();
+
+  // Verify the initial opener relationship.
+  EXPECT_EQ(ftn_1, ftn_2->opener());
+
+  // Verify all tabs are in the same BrowsingInstance.
+  EXPECT_TRUE(rfh_1->GetSiteInstance()->IsRelatedSiteInstance(
+      web_contents_2->GetPrimaryMainFrame()->GetSiteInstance()));
+  EXPECT_TRUE(rfh_1->GetSiteInstance()->IsRelatedSiteInstance(
+      web_contents_3->GetPrimaryMainFrame()->GetSiteInstance()));
+
+  // From Tab 3, discover Tab 2 by name and change its opener to Tab 3. This
+  // should work, as they're in the same BrowsingInstance.
+  EXPECT_TRUE(ExecJs(tab_3, "!!window.open('', 'foo');"));
+  EXPECT_EQ(ftn_3, ftn_2->opener());
+
+  // Navigate Tab 2 to a WebUI URL. This forces a BrowsingInstance swap.
+  EXPECT_TRUE(NavigateToURL(tab_2, webui_url));
+  EXPECT_FALSE(rfh_1->GetSiteInstance()->IsRelatedSiteInstance(
+      web_contents_2->GetPrimaryMainFrame()->GetSiteInstance()));
+
+  // Currently, the WebUI page's opener in the browser process is not severed,
+  // unlike COOP cases. (This allows the opener to still function if we were to
+  // go back to the previous page.) However, window.opener visible to JS should
+  // be null.
+  EXPECT_EQ(ftn_3, ftn_2->opener());
+  EXPECT_TRUE(ExecJs(tab_2, "window.opener == null"));
+
+  // From Tab 1, attempt to discover Tab 2 by name and change its opener to
+  // Tab 1.
+  EXPECT_TRUE(ExecJs(ftn_1, "!!window.open('', 'foo');"));
+
+  // Because those two tabs are now in different BrowsingInstances, this attempt
+  // to change the opener should be rejected. Verify the opener did not change.
+  EXPECT_EQ(ftn_3, ftn_2->opener());
+  EXPECT_TRUE(ExecJs(tab_2, "window.opener == null"));
+}
+
 namespace {
 
 // A helper to post a recurring check that a renderer process is foregrounded.
diff --git a/content/browser/renderer_host/render_frame_proxy_host.cc b/content/browser/renderer_host/render_frame_proxy_host.cc
index bce2b4c..eb03d035 100644
--- a/content/browser/renderer_host/render_frame_proxy_host.cc
+++ b/content/browser/renderer_host/render_frame_proxy_host.cc
@@ -853,6 +853,8 @@
 
 void RenderFrameProxyHost::DidChangeOpener(
     const std::optional<blink::LocalFrameToken>& opener_frame_token) {
+  // Note that this call internally protects against `opener_frame_token`
+  // referring to an inactive frame.
   frame_tree_node_->render_manager()->DidChangeOpener(opener_frame_token,
                                                       site_instance_group());
 }
diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
index a280237d..b5f0e66 100644
--- a/content/browser/security_exploit_browsertest.cc
+++ b/content/browser/security_exploit_browsertest.cc
@@ -1606,6 +1606,121 @@
             kill_waiter.Wait());
 }
 
+// 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.
+//
+// Setup:
+//   Tab 1: A1 in BCG1 -> navigates to WebUI in BCG3, A1 gets bfcached
+//   Tab 2: A2 in BCG2
+//   Tab 3: B in BCG2
+// Attacker controls both A and B, and executes DidChangeOpener on a proxy for
+// tab 3 in A2's SiteInstance, asking the browser process to update tab3's
+// 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) {
+  // This test assumes that back-forward cache is enabled.
+  if (!IsBackForwardCacheEnabled()) {
+    GTEST_SKIP();
+  }
+
+  // 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")));

diff --git a/content/browser/renderer_host/render_frame_host_manager_browsertest.cc b/content/browser/renderer_host/render_frame_host_manager_browsertest.cc
index 506bd7f4..95e196f6 100644
--- a/content/browser/renderer_host/render_frame_host_manager_browsertest.cc
+++ b/content/browser/renderer_host/render_frame_host_manager_browsertest.cc
@@ -5957,6 +5957,72 @@
   }
 }
 
+// Tests that a renderer cannot change the opener of a frame across different
+// BrowsingInstances (e.g., when trying to reconnect to a named window that
+// has navigated to a WebUI URL).
+IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest,
+                       CannotChangeOpenerAcrossBrowsingInstances) {
+  StartEmbeddedServer();
+
+  GURL url_a1(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  GURL url_a2(embedded_test_server()->GetURL("a.com", "/title2.html"));
+  GURL url_a3(embedded_test_server()->GetURL("a.com", "/title3.html"));
+  GURL webui_url = GetWebUIURL(kChromeUIGpuHost);
+
+  EXPECT_TRUE(NavigateToURL(shell(), url_a1));
+  WebContentsImpl* web_contents_1 =
+      static_cast<WebContentsImpl*>(shell()->web_contents());
+  FrameTreeNode* ftn_1 = web_contents_1->GetPrimaryFrameTree().root();
+  RenderFrameHostImpl* rfh_1 = ftn_1->current_frame_host();
+
+  // Use window.open() to open a popup from A1 to A2 named "foo", and another
+  // unnamed popup from A1 to A3.
+  Shell* tab_2 = OpenPopup(rfh_1, url_a2, "foo");
+  Shell* tab_3 = OpenPopup(rfh_1, url_a3, "");
+
+  WebContentsImpl* web_contents_2 =
+      static_cast<WebContentsImpl*>(tab_2->web_contents());
+  FrameTreeNode* ftn_2 = web_contents_2->GetPrimaryFrameTree().root();
+  WebContentsImpl* web_contents_3 =
+      static_cast<WebContentsImpl*>(tab_3->web_contents());
+  FrameTreeNode* ftn_3 = web_contents_3->GetPrimaryFrameTree().root();
+
+  // Verify the initial opener relationship.
+  EXPECT_EQ(ftn_1, ftn_2->opener());
+
+  // Verify all tabs are in the same BrowsingInstance.
+  EXPECT_TRUE(rfh_1->GetSiteInstance()->IsRelatedSiteInstance(
+      web_contents_2->GetPrimaryMainFrame()->GetSiteInstance()));
+  EXPECT_TRUE(rfh_1->GetSiteInstance()->IsRelatedSiteInstance(
+      web_contents_3->GetPrimaryMainFrame()->GetSiteInstance()));
+
+  // From Tab 3, discover Tab 2 by name and change its opener to Tab 3. This
+  // should work, as they're in the same BrowsingInstance.
+  EXPECT_TRUE(ExecJs(tab_3, "!!window.open('', 'foo');"));
+  EXPECT_EQ(ftn_3, ftn_2->opener());
+
+  // Navigate Tab 2 to a WebUI URL. This forces a BrowsingInstance swap.
+  EXPECT_TRUE(NavigateToURL(tab_2, webui_url));
+  EXPECT_FALSE(rfh_1->GetSiteInstance()->IsRelatedSiteInstance(
+      web_contents_2->GetPrimaryMainFrame()->GetSiteInstance()));
+
+  // Currently, the WebUI page's opener in the browser process is not severed,
+  // unlike COOP cases. (This allows the opener to still function if we were to
+  // go back to the previous page.) However, window.opener visible to JS should
+  // be null.
+  EXPECT_EQ(ftn_3, ftn_2->opener());
+  EXPECT_TRUE(ExecJs(tab_2, "window.opener == null"));
+
+  // From Tab 1, attempt to discover Tab 2 by name and change its opener to
+  // Tab 1.
+  EXPECT_TRUE(ExecJs(ftn_1, "!!window.open('', 'foo');"));
+
+  // Because those two tabs are now in different BrowsingInstances, this attempt
+  // to change the opener should be rejected. Verify the opener did not change.
+  EXPECT_EQ(ftn_3, ftn_2->opener());
+  EXPECT_TRUE(ExecJs(tab_2, "window.opener == null"));
+}
+
 namespace {
 
 // A helper to post a recurring check that a renderer process is foregrounded.
diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
index a280237d..b5f0e66 100644
--- a/content/browser/security_exploit_browsertest.cc
+++ b/content/browser/security_exploit_browsertest.cc
@@ -1606,6 +1606,121 @@
             kill_waiter.Wait());
 }
 
+// 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.
+//
+// Setup:
+//   Tab 1: A1 in BCG1 -> navigates to WebUI in BCG3, A1 gets bfcached
+//   Tab 2: A2 in BCG2
+//   Tab 3: B in BCG2
+// Attacker controls both A and B, and executes DidChangeOpener on a proxy for
+// tab 3 in A2's SiteInstance, asking the browser process to update tab3's
+// 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) {
+  // This test assumes that back-forward cache is enabled.
+  if (!IsBackForwardCacheEnabled()) {
+    GTEST_SKIP();
+  }
+
+  // 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 (still controlled by the attacker) in the
+  // same BCG as A2.
+  GURL attacker_cross_site_url(
+      embedded_test_server()->GetURL("b.test", "/title3.html"));
+  Shell* tab3 = OpenPopup(tab2, attacker_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();
+
+  // Navigate A1 to a WebUI URL. This should force a BrowsingInstance swap. The
+  // original RenderFrameHost is placed into BFCache. Note that the popups
+  // created so far don't stop BFCache from working because they're in a
+  // separate BCG.
+  GURL chrome_url(std::string(kChromeUIScheme) + "://" +
+                  std::string(kChromeUIGpuHost));
+  EXPECT_TRUE(NavigateToURL(shell(), chrome_url));
+  RenderFrameHostImpl* tab1_webui_rfh = tab1_root->current_frame_host();
+  EXPECT_NE(tab1_rfh, tab1_webui_rfh);
+  EXPECT_TRUE(tab1_rfh->IsInBackForwardCache());
+  EXPECT_FALSE(tab1_webui_rfh->GetSiteInstance()->IsRelatedSiteInstance(
+      tab2_rfh->GetSiteInstance()));
+  EXPECT_FALSE(tab1_webui_rfh->GetSiteInstance()->IsRelatedSiteInstance(
+      tab3_rfh->GetSiteInstance()));
+
+  // Look up the proxy that represents B in A2's SiteInstance (both
+  // controlled by the attacker). 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 BFCached frame. This goes to the RenderFrameHostManager of
+  // tab3, trying to set its opener to the FTN that's currently showing the
+  // WebUI page.
+  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);
+
+  // Normally, opener updates trigger proxy creation to ensure that the frame
+  // that had its opener updated has access to the new opener chain. Ensure we
+  // didn't create a RenderFrameProxyHost for the WebUI page in tab3's
+  // SiteInstanceGroup, connecting the WebUI page to the attacker-controlled
+  // tab3.
+  RenderFrameProxyHost* webui_to_non_webui_proxy =
+      tab1_webui_rfh->browsing_context_state()->GetRenderFrameProxyHost(
+          tab3_rfh->GetSiteInstance()->group());
+  EXPECT_FALSE(webui_to_non_webui_proxy);
+}
+
 namespace {
 
 // An interceptor class that allows replacing the URL of the commit IPC from