[Backport] CVE-2022-1873: Insufficient policy enforcement in COOP.

Manual cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/3532010:
Fix COOP-based opener removal on FrameTreeNodes.

When a page A opens a page B, B can access A via window.opener. If
either of these pages navigate causing a BrowsingInstance swap, the
links need to be severed. Currently it only works well if B navigates.

If A navigates, we do not find the frames that were opened by it and
remove their openers on the browser side. This is now done in the
RenderFrameHostManager.

We also clarify how this information is carried to the renderer, which
was quite obscure and maybe even involuntary. Explains that the
RenderView suppression will trigger an opener clear.

BUG=1305394

Change-Id: I4bb2a9733c523dac78ffb270877ba07aba6984a4
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Cr-Commit-Position: refs/heads/main@{#985876}
Reviewed-by: Michal Klocek <michal.klocek@qt.io>

3 files changed, 39 insertions, 1 deletions

diff --git a/chromium/content/browser/renderer_host/frame_tree_node.cc b/chromium/content/browser/renderer_host/frame_tree_node.cc
index a8fe3877c82..34d0e4c8440 100644
--- a/chromium/content/browser/renderer_host/frame_tree_node.cc
+++ b/chromium/content/browser/renderer_host/frame_tree_node.cc
@@ -54,6 +54,15 @@ class FrameTreeNode::OpenerDestroyedObserver : public FrameTreeNode::Observer {
 
   // FrameTreeNode::Observer
   void OnFrameTreeNodeDestroyed(FrameTreeNode* node) override {
+    NullifyOpener(node);
+  }
+
+  // FrameTreeNode::Observer
+  void OnFrameTreeNodeDisownedOpenee(FrameTreeNode* node) override {
+    NullifyOpener(node);
+  }
+
+  void NullifyOpener(FrameTreeNode* node) {
     if (observing_original_opener_) {
       // The "original owner" is special. It's used for attribution, and clients
       // walk down the original owner chain. Therefore, if a link in the chain
@@ -875,4 +884,13 @@ bool FrameTreeNode::IsInFencedFrameTree() const {
   }
 }
 
+void FrameTreeNode::ClearOpenerReferences() {
+  // Simulate the FrameTreeNode being dead to opener observers. They will
+  // nullify their opener.
+  // Note: observers remove themselves from observers_, no need to take care of
+  // that manually.
+  for (auto& observer : observers_)
+    observer.OnFrameTreeNodeDisownedOpenee(this);
+}
+
 }  // namespace content
diff --git a/chromium/content/browser/renderer_host/frame_tree_node.h b/chromium/content/browser/renderer_host/frame_tree_node.h
index e3f14b23925..daef2e0b0d0 100644
--- a/chromium/content/browser/renderer_host/frame_tree_node.h
+++ b/chromium/content/browser/renderer_host/frame_tree_node.h
@@ -62,6 +62,10 @@ class CONTENT_EXPORT FrameTreeNode {
     // Invoked when a FrameTreeNode becomes focused.
     virtual void OnFrameTreeNodeFocused(FrameTreeNode* node) {}
 
+    // Invoked when a FrameTreeNode moves to a different BrowsingInstance and
+    // the popups it opened should be disowned.
+    virtual void OnFrameTreeNodeDisownedOpenee(FrameTreeNode* node) {}
+
     virtual ~Observer() = default;
   };
 
@@ -513,6 +517,10 @@ class CONTENT_EXPORT FrameTreeNode {
     replication_state_->name = name;
   }
 
+  // Clears the opener property of popups referencing this FrameTreeNode as
+  // their opener.
+  void ClearOpenerReferences();
+
  private:
   FRIEND_TEST_ALL_PREFIXES(SitePerProcessPermissionsPolicyBrowserTest,
                            ContainerPolicyDynamic);
diff --git a/chromium/content/browser/renderer_host/render_frame_host_manager.cc b/chromium/content/browser/renderer_host/render_frame_host_manager.cc
index d44fe1f7dbe..1c45b777c94 100644
--- a/chromium/content/browser/renderer_host/render_frame_host_manager.cc
+++ b/chromium/content/browser/renderer_host/render_frame_host_manager.cc
@@ -3477,9 +3477,19 @@ void RenderFrameHostManager::CommitPending(
 
   // If this is a top-level frame, and COOP triggered a BrowsingInstance swap,
   // make sure all relationships with the previous BrowsingInstance are severed
-  // by removing the opener and proxies with unrelated SiteInstances.
+  // by removing the opener, the openee's opener, and the proxies with unrelated
+  // SiteInstances.
   if (clear_proxies_on_commit) {
     DCHECK(frame_tree_node_->IsMainFrame());
+
+    // If this frame has opened popups, we need to clear the opened popup's
+    // opener. This is done here on the browser side. A similar mechanism occurs
+    // in the renderer process when the RenderView of this frame is destroyed,
+    // via blink::OpenedFrameTracker.
+    frame_tree_node_->ClearOpenerReferences();
+
+    // We've just cleared other frames' "opener" referencing this frame, we now
+    // clear this frame's "opener".
     if (frame_tree_node_->opener() &&
         !render_frame_host_->GetSiteInstance()->IsRelatedSiteInstance(
             frame_tree_node_->opener()
@@ -3491,6 +3501,8 @@ void RenderFrameHostManager::CommitPending(
       // after it is not necessary in this particuliar case.
     }
 
+    // Now that opener references are gone in both direction, we can clear the
+    // underlying proxies that were used for that purpose.
     std::vector<RenderFrameProxyHost*> removed_proxies;
     for (auto& it : proxy_hosts_) {
       const auto& proxy = it.second;