From ce7e429fedeaa330340d59bd0c5f5892c3c44897 Mon Sep 17 00:00:00 2001
From: Alex Moshchuk <alexmos@chromium.org>
Date: Thu, 10 Sep 2026 12:58:18 -0700
Subject: [PATCH] Ensure that speculative RFHs cannot update frame name or opener

When a cross-site navigation begins, a speculative RenderFrameHost is
created that shares the FrameTreeNode's BrowsingContextState with the
current active document. Prior to this CL, DidChangeName and
DidChangeOpener did not gate replication updates using
CanApplyFrameReplicationUpdate(). As a result, a compromised renderer
hosting a speculative frame could invoke DidChangeName or
DidChangeOpener to modify the replicated frame name or modify/sever
the opener of the active cross-origin frame before navigation
committed.

This CL:

- Updates RenderFrameHostImpl::DidChangeName to replace the existing
  redundant bfcache / pending-deletion checks with
  CanApplyFrameReplicationUpdate(..., kRfhDidChangeName). This blocks
  speculative frames while ensuring cached frames are properly evicted
  from the back/forward cache with the appropriate reason.

- Updates RenderFrameHostImpl::DidChangeOpener to check
  CanApplyFrameReplicationUpdate(..., kRfhDidChangeOpener), preventing
  speculative frames from modifying or severing an active frame's
  opener.

- Adds kRfhDidChangeName and kRfhDidChangeOpener to
  BackForwardCache::NotRestoredReason along with corresponding
  histogram, trace proto, and DevTools mappings.

- Adds a regression browser test in SecurityExploitBrowserTest.

Bug: 554348119
Change-Id: If2379efa2889c08d9603304bea1b9dfcbb18c466
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8373423
Reviewed-by: Stephen Nusko <nuskos@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1695594}
---
diff --git a/base/tracing/protos/chrome_track_event.proto b/base/tracing/protos/chrome_track_event.proto
index d2a7f9e74..a533e91 100644
--- a/base/tracing/protos/chrome_track_event.proto
+++ b/base/tracing/protos/chrome_track_event.proto
@@ -750,6 +750,8 @@
     RFH_ENFORCE_INSECURE_NAVIGATIONS_SET = 73;
     RFH_HAD_STICKY_USER_ACTIVATION_BEFORE_NAVIGATION_CHANGED = 74;
     RFH_UPDATE_AD_FRAME_STATUS = 75;
+    RFH_DID_CHANGE_NAME = 76;
+    RFH_DID_CHANGE_OPENER = 77;
   }
 
   optional BackForwardCacheNotRestoredReason
diff --git a/content/browser/back_forward_cache/back_forward_cache_can_store_document_result.cc b/content/browser/back_forward_cache/back_forward_cache_can_store_document_result.cc
index d82faec..ada5df93 100644
--- a/content/browser/back_forward_cache/back_forward_cache_can_store_document_result.cc
+++ b/content/browser/back_forward_cache/back_forward_cache_can_store_document_result.cc
@@ -219,6 +219,10 @@
           RFH_HAD_STICKY_USER_ACTIVATION_BEFORE_NAVIGATION_CHANGED;
     case Reason::kRfhUpdateAdFrameStatus:
       return ProtoEnum::RFH_UPDATE_AD_FRAME_STATUS;
+    case Reason::kRfhDidChangeName:
+      return ProtoEnum::RFH_DID_CHANGE_NAME;
+    case Reason::kRfhDidChangeOpener:
+      return ProtoEnum::RFH_DID_CHANGE_OPENER;
   }
   NOTREACHED();
 }
@@ -493,6 +497,10 @@
       return "Rfh: HadStickyUserActivationBeforeNavigationChanged";
     case Reason::kRfhUpdateAdFrameStatus:
       return "Rfh: UpdateAdFrameStatus";
+    case Reason::kRfhDidChangeName:
+      return "Rfh: DidChangeName";
+    case Reason::kRfhDidChangeOpener:
+      return "Rfh: DidChangeOpener";
   }
 }
 
@@ -624,6 +632,8 @@
     case Reason::kRfhEnforceInsecureRequestPolicy:
     case Reason::kRfhHadStickyUserActivationBeforeNavigationChanged:
     case Reason::kRfhUpdateAdFrameStatus:
+    case Reason::kRfhDidChangeName:
+    case Reason::kRfhDidChangeOpener:
     case Reason::kUnknown:
       return "masked";
   }
diff --git a/content/browser/devtools/protocol/page_handler.cc b/content/browser/devtools/protocol/page_handler.cc
index 45fe5097..34f1a439 100644
--- a/content/browser/devtools/protocol/page_handler.cc
+++ b/content/browser/devtools/protocol/page_handler.cc
@@ -2182,6 +2182,8 @@
     case Reason::kRfhEnforceInsecureRequestPolicy:
     case Reason::kRfhHadStickyUserActivationBeforeNavigationChanged:
     case Reason::kRfhUpdateAdFrameStatus:
+    case Reason::kRfhDidChangeName:
+    case Reason::kRfhDidChangeOpener:
       return Page::BackForwardCacheNotRestoredReasonEnum::Unknown;
     case Reason::kCacheControlNoStoreDeviceBoundSessionTerminated:
       return Page::BackForwardCacheNotRestoredReasonEnum::
@@ -2512,6 +2514,8 @@
     case Reason::kRfhEnforceInsecureRequestPolicy:
     case Reason::kRfhHadStickyUserActivationBeforeNavigationChanged:
     case Reason::kRfhUpdateAdFrameStatus:
+    case Reason::kRfhDidChangeName:
+    case Reason::kRfhDidChangeOpener:
     case Reason::kUnknown:
       return Page::BackForwardCacheNotRestoredReasonTypeEnum::SupportPending;
     case Reason::kBlocklistedFeatures:
diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc
index da8ff951..9a3e6d4 100644
--- a/content/browser/renderer_host/render_frame_host_impl.cc
+++ b/content/browser/renderer_host/render_frame_host_impl.cc
@@ -8560,11 +8560,13 @@
 
 void RenderFrameHostImpl::DidChangeName(const std::string& name,
                                         const std::string& unique_name) {
-  // Frame name updates used to occur in the FrameTreeNode; however, as they
-  // now occur in RenderFrameHostImpl (and by extension, BrowsingContextState),
-  // ensure that invalid updates (i.e. when in the BackForwardCache or in a
-  // pending deletion state) are not applied.
-  if (IsInBackForwardCache() || IsPendingDeletion()) {
+  // Frame name updates used to occur in the FrameTreeNode; however, as they now
+  // occur in RenderFrameHostImpl (and by extension, BrowsingContextState),
+  // ensure that invalid updates (i.e. when in speculative, pending deletion, or
+  // back/forward cached state) are not applied.
+  if (!CanApplyFrameReplicationUpdate(
+          this,
+          BackForwardCacheMetrics::NotRestoredReason::kRfhDidChangeName)) {
     return;
   }
   if (GetParent() != nullptr) {
@@ -10123,9 +10125,9 @@
 
 void RenderFrameHostImpl::DidChangeOpener(
     const std::optional<blink::LocalFrameToken>& opener_frame_token) {
-  // `owner_` could be null when we get this message asynchronously from the
-  // renderer in pending deletion state.
-  if (!owner_) {
+  if (!CanApplyFrameReplicationUpdate(
+          this,
+          BackForwardCacheMetrics::NotRestoredReason::kRfhDidChangeOpener)) {
     return;
   }
 
diff --git a/content/public/browser/back_forward_cache.h b/content/public/browser/back_forward_cache.h
index 8484d20..6b0f6cd 100644
--- a/content/public/browser/back_forward_cache.h
+++ b/content/public/browser/back_forward_cache.h
@@ -127,7 +127,9 @@
     kRfhEnforceInsecureRequestPolicy = 77,
     kRfhHadStickyUserActivationBeforeNavigationChanged = 78,
     kRfhUpdateAdFrameStatus = 79,
-    kMaxValue = kRfhUpdateAdFrameStatus,
+    kRfhDidChangeName = 80,
+    kRfhDidChangeOpener = 81,
+    kMaxValue = kRfhDidChangeOpener,
   };
   // LINT.ThenChange(//tools/metrics/histograms/metadata/navigation/enums.xml:BackForwardCacheNotRestoredReason)
 
diff --git a/tools/metrics/histograms/metadata/navigation/enums.xml b/tools/metrics/histograms/metadata/navigation/enums.xml
index c6ebdca6..6555a847 100644
--- a/tools/metrics/histograms/metadata/navigation/enums.xml
+++ b/tools/metrics/histograms/metadata/navigation/enums.xml
@@ -148,6 +148,8 @@
   <int value="77" label="Rfh EnforceInsecureRequestPolicy"/>
   <int value="78" label="Rfh HadStickyUserActivationBeforeNavigationChanged"/>
   <int value="79" label="Rfh UpdateAdFrameStatus"/>
+  <int value="80" label="Rfh DidChangeName"/>
+  <int value="81" label="Rfh DidChangeOpener"/>
 </enum>
 
 <!-- LINT.ThenChange(//content/public/browser/back_forward_cache.h:NotRestoredReason) -->
diff --git a/content/browser/renderer_host/render_frame_host_impl_browsertest.cc b/content/browser/renderer_host/render_frame_host_impl_browsertest.cc
index b8a4fa2b..0c8428f9 100644
--- a/content/browser/renderer_host/render_frame_host_impl_browsertest.cc
+++ b/content/browser/renderer_host/render_frame_host_impl_browsertest.cc
@@ -9153,26 +9153,27 @@
             "");
 
   // Update the name using a pagehide handler to ensure that it occurs while the
-  // RenderFrameHost is in the BackForwardCache. This typically shouldn't occur
-  // and the name update should therefore be blocked.
+  // RenderFrameHost is in the BackForwardCache. The name update should be
+  // blocked, and the RenderFrameHost should be evicted from the
+  // BackForwardCache.
   EXPECT_TRUE(ExecJs(
       render_frame_host,
       "window.onpagehide = function() { window.name = 'unused_name'; }"));
 
-  // Navigate so that the current RenderFrameHost is cached.
-  EXPECT_TRUE(NavigateToURL(shell(), url_b));
-  EXPECT_TRUE(render_frame_host->IsInBackForwardCache());
+  scoped_refptr<BrowsingContextState> browsing_context_state =
+      render_frame_host->browsing_context_state();
+  RenderFrameDeletedObserver delete_observer(render_frame_host);
 
-  std::string frame_name =
-      render_frame_host->browsing_context_state()->frame_name();
-  std::string unique_name = render_frame_host->browsing_context_state()
-                                ->current_replication_state()
-                                .unique_name;
+  // Navigate so that the current RenderFrameHost is cached and subsequently
+  // evicted due to the name update in pagehide.
+  EXPECT_TRUE(NavigateToURL(shell(), url_b));
+  delete_observer.WaitUntilDeleted();
 
   // Verify that the frame name and unique name haven't been changed, even
   // though a name change was triggered by the Javascript.
-  EXPECT_EQ(frame_name, "page_name");
-  EXPECT_EQ(unique_name, "");
+  EXPECT_EQ(browsing_context_state->frame_name(), "page_name");
+  EXPECT_EQ(browsing_context_state->current_replication_state().unique_name,
+            "");
 }
 
 // Test that, when the RenderFrameHostImpl is in a pending delete state, the
diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
index 5d90eec..abaefda2 100644
--- a/content/browser/security_exploit_browsertest.cc
+++ b/content/browser/security_exploit_browsertest.cc
@@ -2077,6 +2077,64 @@
   EXPECT_EQ(initial_ad_frame_status, replication_state.ad_frame_status);
 }
 
+// Verify that a speculative RenderFrameHost cannot update the frame name or
+// opener in the BrowsingContextState/FrameTreeNode that it shares with the
+// current cross-origin document. Regression test for crbug.com/554348119.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
+                       SpeculativeFrameCannotUpdateNameOrOpener) {
+  IsolateAllSitesForTesting(base::CommandLine::ForCurrentProcess());
+
+  GURL main_url(embedded_test_server()->GetURL(
+      "a.com", "/cross_site_iframe_factory.html?a(b)"));
+  EXPECT_TRUE(NavigateToURL(shell(), main_url));
+
+  WebContentsImpl* web_contents =
+      static_cast<WebContentsImpl*>(shell()->web_contents());
+  FrameTreeNode* root = web_contents->GetPrimaryFrameTree().root();
+  FrameTreeNode* child = root->child_at(0);
+  RenderFrameHostImpl* current_rfh = child->current_frame_host();
+
+  const std::string initial_name = child->current_replication_state().name;
+  const std::string initial_unique_name =
+      child->current_replication_state().unique_name;
+
+  // Set an opener on the child frame so we can verify a speculative RFH
+  // cannot sever it.
+  child->SetOpener(root);
+  ASSERT_EQ(root, child->opener());
+
+  // Start a cross-process navigation in the subframe so that a speculative
+  // RenderFrameHost is created. The speculative RenderFrameHost shares the
+  // FrameTreeNode's BrowsingContextState with the current document.
+  GURL cross_site_url(embedded_test_server()->GetURL("c.com", "/title1.html"));
+  TestNavigationManager nav_manager(web_contents, cross_site_url);
+  EXPECT_TRUE(BeginNavigateToURLFromRenderer(child, cross_site_url));
+  nav_manager.WaitForSpeculativeRenderFrameHostCreation();
+
+  RenderFrameHostImpl* speculative_rfh =
+      child->render_manager()->speculative_frame_host();
+  ASSERT_TRUE(speculative_rfh);
+  ASSERT_EQ(RenderFrameHostImpl::LifecycleStateImpl::kSpeculative,
+            speculative_rfh->lifecycle_state());
+  EXPECT_NE(speculative_rfh->GetProcess(), current_rfh->GetProcess());
+  ASSERT_EQ(speculative_rfh->browsing_context_state().get(),
+            current_rfh->browsing_context_state().get());
+
+  // Simulate a compromised renderer sending DidChangeName and DidChangeOpener
+  // from the speculative RenderFrameHost before the navigation has committed.
+  // These should be ignored and must not modify the name or opener of the
+  // still-current cross-origin document.
+  static_cast<mojom::FrameHost*>(speculative_rfh)
+      ->DidChangeName("evil_name", "evil_unique_name");
+  static_cast<blink::mojom::LocalFrameHost*>(speculative_rfh)
+      ->DidChangeOpener(std::nullopt);
+
+  EXPECT_EQ(initial_name, child->current_replication_state().name);
+  EXPECT_EQ(initial_unique_name,
+            child->current_replication_state().unique_name);
+  EXPECT_EQ(root, child->opener());
+}
+
 class RemoteFrameHostInterceptor
     : public blink::mojom::RemoteFrameHostInterceptorForTesting {
  public:

diff --git a/content/browser/renderer_host/render_frame_host_impl_browsertest.cc b/content/browser/renderer_host/render_frame_host_impl_browsertest.cc
index b8a4fa2b..0c8428f9 100644
--- a/content/browser/renderer_host/render_frame_host_impl_browsertest.cc
+++ b/content/browser/renderer_host/render_frame_host_impl_browsertest.cc
@@ -9153,26 +9153,27 @@
             "");
 
   // Update the name using a pagehide handler to ensure that it occurs while the
-  // RenderFrameHost is in the BackForwardCache. This typically shouldn't occur
-  // and the name update should therefore be blocked.
+  // RenderFrameHost is in the BackForwardCache. The name update should be
+  // blocked, and the RenderFrameHost should be evicted from the
+  // BackForwardCache.
   EXPECT_TRUE(ExecJs(
       render_frame_host,
       "window.onpagehide = function() { window.name = 'unused_name'; }"));
 
-  // Navigate so that the current RenderFrameHost is cached.
-  EXPECT_TRUE(NavigateToURL(shell(), url_b));
-  EXPECT_TRUE(render_frame_host->IsInBackForwardCache());
+  scoped_refptr<BrowsingContextState> browsing_context_state =
+      render_frame_host->browsing_context_state();
+  RenderFrameDeletedObserver delete_observer(render_frame_host);
 
-  std::string frame_name =
-      render_frame_host->browsing_context_state()->frame_name();
-  std::string unique_name = render_frame_host->browsing_context_state()
-                                ->current_replication_state()
-                                .unique_name;
+  // Navigate so that the current RenderFrameHost is cached and subsequently
+  // evicted due to the name update in pagehide.
+  EXPECT_TRUE(NavigateToURL(shell(), url_b));
+  delete_observer.WaitUntilDeleted();
 
   // Verify that the frame name and unique name haven't been changed, even
   // though a name change was triggered by the Javascript.
-  EXPECT_EQ(frame_name, "page_name");
-  EXPECT_EQ(unique_name, "");
+  EXPECT_EQ(browsing_context_state->frame_name(), "page_name");
+  EXPECT_EQ(browsing_context_state->current_replication_state().unique_name,
+            "");
 }
 
 // Test that, when the RenderFrameHostImpl is in a pending delete state, the
diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
index 5d90eec..abaefda2 100644
--- a/content/browser/security_exploit_browsertest.cc
+++ b/content/browser/security_exploit_browsertest.cc
@@ -2077,6 +2077,64 @@
   EXPECT_EQ(initial_ad_frame_status, replication_state.ad_frame_status);
 }
 
+// Verify that a speculative RenderFrameHost cannot update the frame name or
+// opener in the BrowsingContextState/FrameTreeNode that it shares with the
+// current cross-origin document. Regression test for crbug.com/554348119.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
+                       SpeculativeFrameCannotUpdateNameOrOpener) {
+  IsolateAllSitesForTesting(base::CommandLine::ForCurrentProcess());
+
+  GURL main_url(embedded_test_server()->GetURL(
+      "a.com", "/cross_site_iframe_factory.html?a(b)"));
+  EXPECT_TRUE(NavigateToURL(shell(), main_url));
+
+  WebContentsImpl* web_contents =
+      static_cast<WebContentsImpl*>(shell()->web_contents());
+  FrameTreeNode* root = web_contents->GetPrimaryFrameTree().root();
+  FrameTreeNode* child = root->child_at(0);
+  RenderFrameHostImpl* current_rfh = child->current_frame_host();
+
+  const std::string initial_name = child->current_replication_state().name;
+  const std::string initial_unique_name =
+      child->current_replication_state().unique_name;
+
+  // Set an opener on the child frame so we can verify a speculative RFH
+  // cannot sever it.
+  child->SetOpener(root);
+  ASSERT_EQ(root, child->opener());
+
+  // Start a cross-process navigation in the subframe so that a speculative
+  // RenderFrameHost is created. The speculative RenderFrameHost shares the
+  // FrameTreeNode's BrowsingContextState with the current document.
+  GURL cross_site_url(embedded_test_server()->GetURL("c.com", "/title1.html"));
+  TestNavigationManager nav_manager(web_contents, cross_site_url);
+  EXPECT_TRUE(BeginNavigateToURLFromRenderer(child, cross_site_url));
+  nav_manager.WaitForSpeculativeRenderFrameHostCreation();
+
+  RenderFrameHostImpl* speculative_rfh =
+      child->render_manager()->speculative_frame_host();
+  ASSERT_TRUE(speculative_rfh);
+  ASSERT_EQ(RenderFrameHostImpl::LifecycleStateImpl::kSpeculative,
+            speculative_rfh->lifecycle_state());
+  EXPECT_NE(speculative_rfh->GetProcess(), current_rfh->GetProcess());
+  ASSERT_EQ(speculative_rfh->browsing_context_state().get(),
+            current_rfh->browsing_context_state().get());
+
+  // Simulate a compromised renderer sending DidChangeName and DidChangeOpener
+  // from the speculative RenderFrameHost before the navigation has committed.
+  // These should be ignored and must not modify the name or opener of the
+  // still-current cross-origin document.
+  static_cast<mojom::FrameHost*>(speculative_rfh)
+      ->DidChangeName("evil_name", "evil_unique_name");
+  static_cast<blink::mojom::LocalFrameHost*>(speculative_rfh)
+      ->DidChangeOpener(std::nullopt);
+
+  EXPECT_EQ(initial_name, child->current_replication_state().name);
+  EXPECT_EQ(initial_unique_name,
+            child->current_replication_state().unique_name);
+  EXPECT_EQ(root, child->opener());
+}
+
 class RemoteFrameHostInterceptor
     : public blink::mojom::RemoteFrameHostInterceptorForTesting {
  public: