From 5b5a413ee216bd7eb23ca6776b018034661f4e9a Mon Sep 17 00:00:00 2001
From: Nan Lin <[email protected]>
Date: Fri, 15 May 2026 16:55:50 -0700
Subject: [PATCH] [APC] Implement source verification for popups and avoid populating popup from multiple frames

Currently, the assembly of APC allows multiple frames to report popup
data into a single shared popup_window field. This creates a security
vulnerability where a compromised out-of-process iframe (OOPIF) can
inject malicious nodes into the tab-level singleton.

This CL introduces two complementary defense-in-depth mechanisms:

Source verification: The browser process now acts as the authoritative
"source of truth" for popup ownership. By iterating through
WebContents::GetPopupWidgets(), the browser verifies that the process
reporting popup data actually owns an active popup widget.

Main frame priority policy: The APC stitcher is modified to prioritize
the main frame's popup data. The logic now ensures that the singleton
popup_window is claimed by the first verified reporter and subsequent
claims are ignored. This prevents "merge attacks" where malicious nodes
are combined with legitimate ones and incorrectly attributed to the main
frame.

Fixed: 513028160
Change-Id: I7db47be02a8ba70e99e8fec2603ac1db67c4ac13
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7849278
Commit-Queue: Nan Lin <[email protected]>
Reviewed-by: Andrew Verge <[email protected]>
Reviewed-by: Aaron Leventhal <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1631643}
---

diff --git a/components/optimization_guide/content/browser/page_content_proto_provider.cc b/components/optimization_guide/content/browser/page_content_proto_provider.cc
index fadd644..e97b4b29 100644
--- a/components/optimization_guide/content/browser/page_content_proto_provider.cc
+++ b/components/optimization_guide/content/browser/page_content_proto_provider.cc
@@ -15,6 +15,8 @@
 #include "base/cancelable_callback.h"
 #include "base/check.h"
 #include "base/check_op.h"
+#include "base/feature.h"
+#include "base/feature_list.h"
 #include "base/functional/bind.h"
 #include "base/functional/callback.h"
 #include "base/functional/concurrent_closures.h"
@@ -39,6 +41,8 @@
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/page.h"
 #include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/web_contents.h"
 #include "mojo/public/cpp/bindings/callback_helpers.h"
@@ -53,6 +57,13 @@
 
 namespace optimization_guide {
 
+namespace features {
+// Controls whether or not we verify the process of a frame reporting a popup
+// matches an authorized popup widget in the browser process.
+BASE_FEATURE(kAnnotatedPageContentVerifyPopupProcess,
+             base::FEATURE_ENABLED_BY_DEFAULT);
+}  // namespace features
+
 namespace {
 
 // The maximum limit for computing the metrics.
@@ -333,6 +344,28 @@
   render_frame_info.source_origin = render_frame_host->GetLastCommittedOrigin();
   render_frame_info.url = render_frame_host->GetLastCommittedURL();
   render_frame_info.media_data = ComputeMediaData(render_frame_host);
+
+  if (base::FeatureList::IsEnabled(
+          features::kAnnotatedPageContentVerifyPopupProcess)) {
+    content::WebContents* web_contents =
+        content::WebContents::FromRenderFrameHost(render_frame_host);
+    if (web_contents) {
+      const content::RenderProcessHost* render_process_host =
+          render_frame_host->GetProcess();
+      for (auto* widget_view : web_contents->GetPopupWidgets()) {
+        if (widget_view && widget_view->GetRenderWidgetHost() &&
+            widget_view->GetRenderWidgetHost()->GetProcess() ==
+                render_process_host) {
+          render_frame_info.has_active_popup = true;
+          break;
+        }
+      }
+    }
+  } else {
+    // If verification is disabled, assume all reported popups are authorized.
+    render_frame_info.has_active_popup = true;
+  }
+
   return render_frame_info;
 }
 
diff --git a/components/optimization_guide/content/browser/page_content_proto_util.cc b/components/optimization_guide/content/browser/page_content_proto_util.cc
index 251ec6a8..0d9d277 100644
--- a/components/optimization_guide/content/browser/page_content_proto_util.cc
+++ b/components/optimization_guide/content/browser/page_content_proto_util.cc
@@ -1249,6 +1249,17 @@
       return base::ok();
     }
 
+    if (page_content_proto().has_popup_window()) {
+      return base::ok();
+    }
+
+    if (!opener_frame_info.has_active_popup) {
+      // This could be a race condition where the popup was closed between the
+      // start of extraction and the renderer's response. We skip the popup
+      // but continue with the rest of the page content.
+      return base::ok();
+    }
+
     optimization_guide::proto::PopupWindow* popup_window =
         page_content_proto().mutable_popup_window();
 
@@ -1258,8 +1269,7 @@
 
     // Set the document ID to the frame which opened the popup (might be wrong,
     // because we treat a main page and its same-site iframes as the same
-    // document id). Also we don't need browser-side security check as the data
-    // all come from the same renderer.
+    // document id). We verify that the the popup is owned by the iframe.
     popup_window->mutable_opener_document_id()->set_serialized_token(
         opener_frame_info.serialized_server_token);
 
@@ -1509,6 +1519,13 @@
                       page_content_result);
   converter.AddRendererPasswordRedactionBoxes(*main_frame_page_content);
 
+  // Claim the singleton popup before walking child frames so the main frame
+  // wins if multiple verified frames report popup data.
+  if (main_frame_page_content->frame_data->popup) {
+    RETURN_IF_ERROR(converter.ConvertPopup(
+        *main_frame_page_content->frame_data->popup, *render_frame_info));
+  }
+
   RETURN_IF_ERROR(converter.ConvertNode(
       main_frame_token, *main_frame_page_content->root_node,
       GetAccessibilityFocusedNodeId(*main_frame_page_content->frame_data),
@@ -1549,12 +1566,6 @@
       optimization_guide::proto::ANNOTATED_PAGE_CONTENT_VERSION_1_0);
   page_content_result.proto.set_mode(mode);
 
-  // If the page had a popup open, provide that popup to APC as well.
-  if (main_frame_page_content->frame_data->popup) {
-    RETURN_IF_ERROR(converter.ConvertPopup(
-        *main_frame_page_content->frame_data->popup, *render_frame_info));
-  }
-
   return base::ok();
 }
 
diff --git a/components/optimization_guide/content/browser/page_content_proto_util.h b/components/optimization_guide/content/browser/page_content_proto_util.h
index 96c9a00..54ed71c 100644
--- a/components/optimization_guide/content/browser/page_content_proto_util.h
+++ b/components/optimization_guide/content/browser/page_content_proto_util.h
@@ -53,6 +53,11 @@
   GURL url;
   std::string serialized_server_token;
   std::optional<optimization_guide::proto::MediaData> media_data;
+
+  // Whether the browser process has verified that this frame (or its process)
+  // has an active popup widget. This is used for defense-in-depth against
+  // compromised renderers spoofing popups.
+  bool has_active_popup = false;
 };
 
 struct TargetNodeInfo {
diff --git a/components/optimization_guide/content/browser/page_content_proto_util_unittest.cc b/components/optimization_guide/content/browser/page_content_proto_util_unittest.cc
index 4331e4f3..4737e9a 100644
--- a/components/optimization_guide/content/browser/page_content_proto_util_unittest.cc
+++ b/components/optimization_guide/content/browser/page_content_proto_util_unittest.cc
@@ -1579,6 +1579,7 @@
 TEST_F(PageContentProtoUtilTest, ConvertPopup) {
   base::test::ScopedFeatureList feature_list(
       blink::features::kAIPageContentIncludePopupWindows);
+  auto main_frame_token = CreateFrameToken();
   auto mojom_content = CreatePageContent();
 
   blink::mojom::AIPageContentPopupPtr popup =
@@ -1592,10 +1593,30 @@
   popup->opener_dom_node_id = 1;
   mojom_content->frame_data->popup = std::move(popup);
 
-  AIPageContentResult page_content;
+  AIPageContentMap page_content_map;
+  page_content_map[main_frame_token] = std::move(mojom_content);
 
-  EXPECT_TRUE(
-      ConvertAIPageContentToProto(mojom_content, page_content).has_value());
+  auto get_render_frame_info = base::BindLambdaForTesting(
+      [&](int, blink::FrameToken token) -> std::optional<RenderFrameInfo> {
+        if (token == main_frame_token.frame_token) {
+          RenderFrameInfo render_frame_info;
+          render_frame_info.global_frame_token = main_frame_token;

diff --git a/components/optimization_guide/content/browser/page_content_proto_util_unittest.cc b/components/optimization_guide/content/browser/page_content_proto_util_unittest.cc
index 4331e4f3..4737e9a 100644
--- a/components/optimization_guide/content/browser/page_content_proto_util_unittest.cc
+++ b/components/optimization_guide/content/browser/page_content_proto_util_unittest.cc
@@ -1579,6 +1579,7 @@
 TEST_F(PageContentProtoUtilTest, ConvertPopup) {
   base::test::ScopedFeatureList feature_list(
       blink::features::kAIPageContentIncludePopupWindows);
+  auto main_frame_token = CreateFrameToken();
   auto mojom_content = CreatePageContent();
 
   blink::mojom::AIPageContentPopupPtr popup =
@@ -1592,10 +1593,30 @@
   popup->opener_dom_node_id = 1;
   mojom_content->frame_data->popup = std::move(popup);
 
-  AIPageContentResult page_content;
+  AIPageContentMap page_content_map;
+  page_content_map[main_frame_token] = std::move(mojom_content);
 
-  EXPECT_TRUE(
-      ConvertAIPageContentToProto(mojom_content, page_content).has_value());
+  auto get_render_frame_info = base::BindLambdaForTesting(
+      [&](int, blink::FrameToken token) -> std::optional<RenderFrameInfo> {
+        if (token == main_frame_token.frame_token) {
+          RenderFrameInfo render_frame_info;
+          render_frame_info.global_frame_token = main_frame_token;
+          render_frame_info.serialized_server_token = token.ToString();
+          // MOCK: signal that the browser process verified this popup.
+          render_frame_info.has_active_popup = true;
+          return render_frame_info;
+        }
+        return std::nullopt;
+      });
+
+  AIPageContentResult page_content;
+  FrameTokenSet frame_token_set;
+
+  EXPECT_TRUE(ConvertAIPageContentToProto(
+                  blink::mojom::AIPageContentOptions::New(), main_frame_token,
+                  page_content_map, get_render_frame_info, frame_token_set,
+                  page_content)
+                  .has_value());
 
   EXPECT_EQ(page_content.proto.version(),
             optimization_guide::proto::ANNOTATED_PAGE_CONTENT_VERSION_1_0);
@@ -1609,6 +1630,213 @@
       1);
 }
 
+TEST_F(PageContentProtoUtilTest, ConvertPopupNotAuthorized) {
+  base::test::ScopedFeatureList feature_list(
+      blink::features::kAIPageContentIncludePopupWindows);
+  auto main_frame_token = CreateFrameToken();
+  auto mojom_content = CreatePageContent();
+
+  blink::mojom::AIPageContentPopupPtr popup =
+      blink::mojom::AIPageContentPopup::New();
+  popup->root_node =
+      CreateContentNode(blink::mojom::AIPageContentAttributeType::kRoot);
+  popup->opener_dom_node_id = 1;
+  mojom_content->frame_data->popup = std::move(popup);
+
+  AIPageContentMap page_content_map;
+  page_content_map[main_frame_token] = std::move(mojom_content);
+
+  auto get_render_frame_info = base::BindLambdaForTesting(
+      [&](int, blink::FrameToken token) -> std::optional<RenderFrameInfo> {
+        if (token == main_frame_token.frame_token) {
+          RenderFrameInfo render_frame_info;
+          render_frame_info.global_frame_token = main_frame_token;
+          render_frame_info.serialized_server_token = token.ToString();
+          // MOCK: Signal that browser process DID NOT find an active popup.
+          render_frame_info.has_active_popup = false;
+          return render_frame_info;
+        }
+        return std::nullopt;
+      });
+
+  AIPageContentResult page_content;
+  FrameTokenSet frame_token_set;
+
+  // Should succeed but with NO popup data because the frame is not authorized.
+  EXPECT_TRUE(ConvertAIPageContentToProto(
+                  blink::mojom::AIPageContentOptions::New(), main_frame_token,
+                  page_content_map, get_render_frame_info, frame_token_set,
+                  page_content)
+                  .has_value());
+
+  EXPECT_FALSE(page_content.proto.has_popup_window());
+}
+
+TEST_F(PageContentProtoUtilTest, ConvertPopupPriority) {
+  base::test::ScopedFeatureList feature_list(
+      blink::features::kAIPageContentIncludePopupWindows);
+
+  auto main_frame_token = CreateFrameToken();
+  auto root_content = CreatePageContent();
+
+  // Main frame popup
+  blink::mojom::AIPageContentPopupPtr main_popup =
+      blink::mojom::AIPageContentPopup::New();
+  main_popup->root_node =
+      CreateContentNode(blink::mojom::AIPageContentAttributeType::kRoot);
+  main_popup->root_node->children_nodes.push_back(CreateTextNode(
+      "main popup text", blink::mojom::AIPageContentTextSize::kM, false, 0));
+  main_popup->opener_dom_node_id = 1;
+  root_content->frame_data->popup = std::move(main_popup);
+
+  // Add an iframe
+  auto iframe_token = CreateFrameToken();
+  auto iframe_node =
+      CreateContentNode(blink::mojom::AIPageContentAttributeType::kIframe);
+  auto iframe_data = blink::mojom::AIPageContentIframeData::New();
+  iframe_data->frame_token = iframe_token.frame_token;
+
+  auto iframe_frame_data = blink::mojom::AIPageContentFrameData::New();
+  iframe_frame_data->frame_interaction_info =
+      blink::mojom::AIPageContentFrameInteractionInfo::New();
+  // Iframe popup
+  blink::mojom::AIPageContentPopupPtr iframe_popup =
+      blink::mojom::AIPageContentPopup::New();
+  iframe_popup->root_node =
+      CreateContentNode(blink::mojom::AIPageContentAttributeType::kRoot);
+  iframe_popup->root_node->children_nodes.push_back(CreateTextNode(
+      "iframe popup text", blink::mojom::AIPageContentTextSize::kM, false, 0));
+  iframe_popup->opener_dom_node_id = 100;
+  iframe_frame_data->popup = std::move(iframe_popup);
+
+  iframe_data->content =
+      blink::mojom::AIPageContentIframeContent::NewLocalFrameData(
+          std::move(iframe_frame_data));
+
+  iframe_node->content_attributes->iframe_data = std::move(iframe_data);
+  root_content->root_node->children_nodes.push_back(std::move(iframe_node));
+
+  AIPageContentMap page_content_map;
+  page_content_map[main_frame_token] = std::move(root_content);
+
+  auto get_render_frame_info = base::BindLambdaForTesting(
+      [&](int, blink::FrameToken token) -> std::optional<RenderFrameInfo> {
+        RenderFrameInfo render_frame_info;
+        if (token == main_frame_token.frame_token) {
+          render_frame_info.global_frame_token = main_frame_token;
+          render_frame_info.has_active_popup = true;
+        } else if (token == iframe_token.frame_token) {
+          render_frame_info.global_frame_token = iframe_token;
+          render_frame_info.has_active_popup = true;
+        } else {
+          return std::nullopt;
+        }
+        render_frame_info.source_origin =
+            url::Origin::Create(GURL("https://example.com"));
+        render_frame_info.url = GURL("https://example.com");
+        render_frame_info.serialized_server_token = token.ToString();
+        return render_frame_info;
+      });
+
+  AIPageContentResult page_content;
+  FrameTokenSet frame_token_set;
+  EXPECT_TRUE(ConvertAIPageContentToProto(
+                  blink::mojom::AIPageContentOptions::New(), main_frame_token,
+                  page_content_map, get_render_frame_info, frame_token_set,
+                  page_content)
+                  .has_value());
+
+  // Verify only main frame's popup is present.
+  ASSERT_TRUE(page_content.proto.has_popup_window());
+  const auto& popup_window = page_content.proto.popup_window();
+  EXPECT_EQ(popup_window.opener_common_ancestor_dom_node_id(), 1);
+  EXPECT_EQ(popup_window.root_node().children_nodes_size(), 1);
+  EXPECT_EQ(popup_window.root_node()
+                .children_nodes(0)
+                .content_attributes()
+                .text_data()
+                .text_content(),
+            "main popup text");
+}
+
+TEST_F(PageContentProtoUtilTest, ConvertPopupIframeOnly) {
+  base::test::ScopedFeatureList feature_list(
+      blink::features::kAIPageContentIncludePopupWindows);
+
+  auto main_frame_token = CreateFrameToken();
+  auto root_content = CreatePageContent();
+  // No main frame popup.
+
+  // Add an iframe
+  auto iframe_token = CreateFrameToken();
+  auto iframe_node =
+      CreateContentNode(blink::mojom::AIPageContentAttributeType::kIframe);
+  auto iframe_data = blink::mojom::AIPageContentIframeData::New();
+  iframe_data->frame_token = iframe_token.frame_token;
+
+  auto iframe_frame_data = blink::mojom::AIPageContentFrameData::New();
+  iframe_frame_data->frame_interaction_info =
+      blink::mojom::AIPageContentFrameInteractionInfo::New();
+  // Iframe popup
+  blink::mojom::AIPageContentPopupPtr iframe_popup =
+      blink::mojom::AIPageContentPopup::New();
+  iframe_popup->root_node =
+      CreateContentNode(blink::mojom::AIPageContentAttributeType::kRoot);
+  iframe_popup->root_node->children_nodes.push_back(CreateTextNode(
+      "iframe popup text", blink::mojom::AIPageContentTextSize::kM, false, 0));
+  iframe_popup->opener_dom_node_id = 100;
+  iframe_frame_data->popup = std::move(iframe_popup);
+
+  iframe_data->content =
+      blink::mojom::AIPageContentIframeContent::NewLocalFrameData(
+          std::move(iframe_frame_data));
+
+  iframe_node->content_attributes->iframe_data = std::move(iframe_data);
+  root_content->root_node->children_nodes.push_back(std::move(iframe_node));
+
+  AIPageContentMap page_content_map;
+  page_content_map[main_frame_token] = std::move(root_content);
+
+  auto get_render_frame_info = base::BindLambdaForTesting(
+      [&](int, blink::FrameToken token) -> std::optional<RenderFrameInfo> {
+        RenderFrameInfo render_frame_info;
+        if (token == main_frame_token.frame_token) {
+          render_frame_info.global_frame_token = main_frame_token;
+          render_frame_info.has_active_popup = false;
+        } else if (token == iframe_token.frame_token) {
+          render_frame_info.global_frame_token = iframe_token;
+          render_frame_info.has_active_popup = true;
+        } else {
+          return std::nullopt;
+        }
+        render_frame_info.source_origin =
+            url::Origin::Create(GURL("https://example.com"));
+        render_frame_info.url = GURL("https://example.com");
+        render_frame_info.serialized_server_token = token.ToString();
+        return render_frame_info;
+      });
+
+  AIPageContentResult page_content;
+  FrameTokenSet frame_token_set;
+  EXPECT_TRUE(ConvertAIPageContentToProto(
+                  blink::mojom::AIPageContentOptions::New(), main_frame_token,
+                  page_content_map, get_render_frame_info, frame_token_set,
+                  page_content)
+                  .has_value());
+
+  // Verify iframe's popup is present since main frame didn't have one.
+  ASSERT_TRUE(page_content.proto.has_popup_window());
+  const auto& popup_window = page_content.proto.popup_window();
+  EXPECT_EQ(popup_window.opener_common_ancestor_dom_node_id(), 100);
+  EXPECT_EQ(popup_window.root_node().children_nodes_size(), 1);
+  EXPECT_EQ(popup_window.root_node()
+                .children_nodes(0)
+                .content_attributes()
+                .text_data()
+                .text_content(),
+            "iframe popup text");
+}
+
 // Test helper to set the geometry of a ContentNode.
 void SetGeometry(ContentNode* node, const gfx::Rect& rect) {
   auto* geometry = node->mutable_content_attributes()->mutable_geometry();