From bb3673b3f6be7e1b2936783f141bd4de2575e509 Mon Sep 17 00:00:00 2001
From: Johann Hofmann <johannhof@chromium.org>
Date: Thu, 18 Jun 2026 13:48:27 -0700
Subject: [PATCH] Prevent iframe sandbox escape in Android Glic popup rewrite

On Android, when a Glic (Actor) task is active on a tab, popup creation
requests are intercepted and rewritten into same-tab top-level
navigations.

However, the Android implementation lacked a sandbox check to verify if the
initiating frame is restricted from performing top-level navigations.
This allows a sandboxed cross-origin iframe with 'allow-popups' (but not
'allow-top-navigation') to bypass its sandbox restrictions and navigate the
top-level page.

This CL adds the missing sandbox check to
TabWebContentsDelegateAndroid::IsWebContentsCreationOverridden, returning
false if the opener is sandboxed with kTopNavigation. This aligns the
Android behavior with Desktop.

Also added a C++ unit test `IsWebContentsCreationOverridden_GlicSandboxCheck`
in `tab_android_unittest.cc` to verify this behavior on Android.

Bug: 516420806
Change-Id: Ifbd89370ae7a56fe064385c45032683d2633eb60
Fixed: 516420806
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7915889
Commit-Queue: Johann Hofmann <johannhof@chromium.org>
Auto-Submit: Johann Hofmann <johannhof@chromium.org>
Reviewed-by: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1649290}
---

diff --git a/chrome/browser/android/tab_android_unittest.cc b/chrome/browser/android/tab_android_unittest.cc
index 61a189d..1260e29 100644
--- a/chrome/browser/android/tab_android_unittest.cc
+++ b/chrome/browser/android/tab_android_unittest.cc
@@ -13,25 +13,63 @@
 #include "base/test/scoped_feature_list.h"
 #include "base/test/task_environment.h"
 #include "chrome/android/chrome_jni_headers/TabAndroidTestHelper_jni.h"
+#include "chrome/browser/actor/actor_keyed_service.h"
+#include "chrome/browser/actor/actor_task.h"
+#include "chrome/browser/actor/actor_test_util.h"
+#include "chrome/browser/actor/actor_util.h"
+#include "chrome/browser/actor/ui/actor_ui_tab_controller_interface.h"
 #include "chrome/browser/android/tab_android.h"
 #include "chrome/browser/android/tab_features.h"
 #include "chrome/browser/android/tab_group_android.h"
 #include "chrome/browser/android/tab_interface_android.h"
+#include "chrome/browser/android/tab_web_contents_delegate_android.h"
 #include "chrome/browser/flags/android/chrome_feature_list.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/android/tab_model/tab_model.h"
 #include "chrome/browser/ui/webui/webui_embedding_context.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chrome/test/base/testing_profile_manager.h"
+#include "components/actor/core/actor_features.h"
 #include "components/tabs/public/pinned_tab_collection.h"
 #include "components/tabs/public/tab_collection.h"
 #include "components/tabs/public/tab_group_tab_collection.h"
+#include "content/public/common/window_container_type.mojom.h"
 #include "content/public/test/browser_task_environment.h"
+#include "content/public/test/navigation_simulator.h"
+#include "content/public/test/test_renderer_host.h"
+#include "net/http/http_response_headers.h"
+#include "services/network/public/mojom/web_sandbox_flags.mojom.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 constexpr int kTabId = 1;
+
+class MockActorUiTabController
+    : public actor::ui::ActorUiTabControllerInterface {
+ public:
+  explicit MockActorUiTabController(tabs::TabInterface& tab)
+      : ActorUiTabControllerInterface(tab) {}
+  ~MockActorUiTabController() override = default;
+
+  // ActorUiTabControllerInterface:
+  void OnUiTabStateChange(const actor::ui::UiTabState& ui_tab_state,
+                          actor::ui::UiResultCallback callback) override {
+    std::move(callback).Run(true);
+  }
+  void SetActorTaskPaused() override {}
+  void SetActorTaskResume() override {}
+  base::WeakPtr<ActorUiTabControllerInterface> GetWeakPtr() override {
+    return weak_ptr_factory_.GetWeakPtr();
+  }
+  actor::ui::UiTabState GetCurrentUiTabState() const override {
+    return actor::ui::UiTabState();
+  }
+
+ private:
+  base::WeakPtrFactory<MockActorUiTabController> weak_ptr_factory_{this};
+};
 }  // namespace
 
 class TabAndroidTest : public testing::Test {
@@ -154,6 +192,92 @@
   EXPECT_EQ(tab.get(), webui::GetTabInterface(raw_web_contents));
 }
 
+class GlicTabAndroidTest : public TabAndroidTest {
+ public:
+  GlicTabAndroidTest() {
+    scoped_feature_list_.InitWithFeatures(
+        /*enabled_features=*/{features::kGlic, features::kGlicActor},
+        /*disabled_features=*/{});
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+TEST_F(GlicTabAndroidTest, IsWebContentsCreationOverridden_GlicSandboxCheck) {
+  content::RenderViewHostTestEnabler rvh_test_enabler;
+
+  // Create a WebContents.
+  std::unique_ptr<content::WebContents> web_contents =
+      content::WebContents::Create(
+          content::WebContents::CreateParams(profile_.get()));
+  content::WebContents* raw_web_contents = web_contents.get();
+
+  // Create TabAndroid for testing.
+  std::unique_ptr<TabAndroid> tab = TabAndroid::CreateForTesting(
+      profile_.get(), kTabId + 1, std::move(web_contents));
+
+  // Register the tab lookup helper.
+  tabs::TabLookupFromWebContents::CreateForWebContents(raw_web_contents,
+                                                       tab.get());
+
+  // Create the delegate with a null Java reference.
+  auto delegate = std::make_unique<android::TabWebContentsDelegateAndroid>(
+      env_, base::android::ScopedJavaLocalRef<jobject>());
+
+  // Set the delegate on the web contents.
+  raw_web_contents->SetDelegate(delegate.get());
+
+  content::RenderFrameHost* main_frame =
+      raw_web_contents->GetPrimaryMainFrame();
+
+  // 1. Without Glic Actor active, it should return false.
+  EXPECT_FALSE(delegate->IsWebContentsCreationOverridden(
+      main_frame, nullptr, content::mojom::WindowContainerType::NORMAL, GURL(),
+      "", GURL()));
+
+  // 2. Start Glic Actor task and attach the tab to it.
+  actor::ActorKeyedService* service =
+      actor::ActorKeyedService::Get(profile_.get());
+  ASSERT_NE(nullptr, service);
+  actor::TaskId task_id = service->CreateTask(
+      actor::TestTaskSourceInfo(), actor::NoEnterprisePolicyChecker());
+  actor::ActorTask* task = service->GetTask(task_id);
+  ASSERT_NE(nullptr, task);
+
+  // Add the tab to the task.
+  MockActorUiTabController mock_controller(*tab);
+  actor::AddTabToTask(*tab, *task);
+
+  // Ensure HasActorTaskPreventingNewWebContents returns true.
+  ASSERT_TRUE(actor::HasActorTaskPreventingNewWebContents(main_frame));
+
+  // 3. Under Glic Actor control, if the frame is NOT sandboxed, it should
+  // override creation.
+  EXPECT_TRUE(delegate->IsWebContentsCreationOverridden(
+      main_frame, nullptr, content::mojom::WindowContainerType::NORMAL, GURL(),
+      "", GURL()));
+
+  // 4. Under Glic Actor control, if the frame IS sandboxed with kTopNavigation,
+  // it should NOT override creation.
+  auto simulator = content::NavigationSimulator::CreateBrowserInitiated(
+      GURL("https://sandboxed.example.com"), raw_web_contents);
+  auto headers =
+      base::MakeRefCounted<net::HttpResponseHeaders>("HTTP/1.1 200 OK");
+  headers->AddHeader("Content-Security-Policy", "sandbox allow-popups");
+  simulator->SetResponseHeaders(headers);
+  simulator->Commit();
+
+  content::RenderFrameHost* sandboxed_frame =
+      raw_web_contents->GetPrimaryMainFrame();
+  ASSERT_TRUE(sandboxed_frame->IsSandboxed(
+      network::mojom::WebSandboxFlags::kTopNavigation));
+
+  EXPECT_FALSE(delegate->IsWebContentsCreationOverridden(
+      sandboxed_frame, nullptr, content::mojom::WindowContainerType::NORMAL,
+      GURL(), "", GURL()));
+}
+
 TEST_F(TabAndroidTest, Getters) {
   TabInterfaceAndroid tab_interface(tab_android_);
   EXPECT_EQ(u"about:blank", tab_interface.GetTitle());
diff --git a/chrome/browser/android/tab_web_contents_delegate_android.cc b/chrome/browser/android/tab_web_contents_delegate_android.cc
index d72ab20..483f52a 100644
--- a/chrome/browser/android/tab_web_contents_delegate_android.cc
+++ b/chrome/browser/android/tab_web_contents_delegate_android.cc
@@ -77,6 +77,7 @@
 #include "content/public/browser/render_widget_host_view.h"

diff --git a/chrome/browser/android/tab_android_unittest.cc b/chrome/browser/android/tab_android_unittest.cc
index 61a189d..1260e29 100644
--- a/chrome/browser/android/tab_android_unittest.cc
+++ b/chrome/browser/android/tab_android_unittest.cc
@@ -13,25 +13,63 @@
 #include "base/test/scoped_feature_list.h"
 #include "base/test/task_environment.h"
 #include "chrome/android/chrome_jni_headers/TabAndroidTestHelper_jni.h"
+#include "chrome/browser/actor/actor_keyed_service.h"
+#include "chrome/browser/actor/actor_task.h"
+#include "chrome/browser/actor/actor_test_util.h"
+#include "chrome/browser/actor/actor_util.h"
+#include "chrome/browser/actor/ui/actor_ui_tab_controller_interface.h"
 #include "chrome/browser/android/tab_android.h"
 #include "chrome/browser/android/tab_features.h"
 #include "chrome/browser/android/tab_group_android.h"
 #include "chrome/browser/android/tab_interface_android.h"
+#include "chrome/browser/android/tab_web_contents_delegate_android.h"
 #include "chrome/browser/flags/android/chrome_feature_list.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/android/tab_model/tab_model.h"
 #include "chrome/browser/ui/webui/webui_embedding_context.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chrome/test/base/testing_profile_manager.h"
+#include "components/actor/core/actor_features.h"
 #include "components/tabs/public/pinned_tab_collection.h"
 #include "components/tabs/public/tab_collection.h"
 #include "components/tabs/public/tab_group_tab_collection.h"
+#include "content/public/common/window_container_type.mojom.h"
 #include "content/public/test/browser_task_environment.h"
+#include "content/public/test/navigation_simulator.h"
+#include "content/public/test/test_renderer_host.h"
+#include "net/http/http_response_headers.h"
+#include "services/network/public/mojom/web_sandbox_flags.mojom.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 constexpr int kTabId = 1;
+
+class MockActorUiTabController
+    : public actor::ui::ActorUiTabControllerInterface {
+ public:
+  explicit MockActorUiTabController(tabs::TabInterface& tab)
+      : ActorUiTabControllerInterface(tab) {}
+  ~MockActorUiTabController() override = default;
+
+  // ActorUiTabControllerInterface:
+  void OnUiTabStateChange(const actor::ui::UiTabState& ui_tab_state,
+                          actor::ui::UiResultCallback callback) override {
+    std::move(callback).Run(true);
+  }
+  void SetActorTaskPaused() override {}
+  void SetActorTaskResume() override {}
+  base::WeakPtr<ActorUiTabControllerInterface> GetWeakPtr() override {
+    return weak_ptr_factory_.GetWeakPtr();
+  }
+  actor::ui::UiTabState GetCurrentUiTabState() const override {
+    return actor::ui::UiTabState();
+  }
+
+ private:
+  base::WeakPtrFactory<MockActorUiTabController> weak_ptr_factory_{this};
+};
 }  // namespace
 
 class TabAndroidTest : public testing::Test {
@@ -154,6 +192,92 @@
   EXPECT_EQ(tab.get(), webui::GetTabInterface(raw_web_contents));
 }
 
+class GlicTabAndroidTest : public TabAndroidTest {
+ public:
+  GlicTabAndroidTest() {
+    scoped_feature_list_.InitWithFeatures(
+        /*enabled_features=*/{features::kGlic, features::kGlicActor},
+        /*disabled_features=*/{});
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+TEST_F(GlicTabAndroidTest, IsWebContentsCreationOverridden_GlicSandboxCheck) {
+  content::RenderViewHostTestEnabler rvh_test_enabler;
+
+  // Create a WebContents.
+  std::unique_ptr<content::WebContents> web_contents =
+      content::WebContents::Create(
+          content::WebContents::CreateParams(profile_.get()));
+  content::WebContents* raw_web_contents = web_contents.get();
+
+  // Create TabAndroid for testing.
+  std::unique_ptr<TabAndroid> tab = TabAndroid::CreateForTesting(
+      profile_.get(), kTabId + 1, std::move(web_contents));
+
+  // Register the tab lookup helper.
+  tabs::TabLookupFromWebContents::CreateForWebContents(raw_web_contents,
+                                                       tab.get());
+
+  // Create the delegate with a null Java reference.
+  auto delegate = std::make_unique<android::TabWebContentsDelegateAndroid>(
+      env_, base::android::ScopedJavaLocalRef<jobject>());
+
+  // Set the delegate on the web contents.
+  raw_web_contents->SetDelegate(delegate.get());
+
+  content::RenderFrameHost* main_frame =
+      raw_web_contents->GetPrimaryMainFrame();
+
+  // 1. Without Glic Actor active, it should return false.
+  EXPECT_FALSE(delegate->IsWebContentsCreationOverridden(
+      main_frame, nullptr, content::mojom::WindowContainerType::NORMAL, GURL(),
+      "", GURL()));
+
+  // 2. Start Glic Actor task and attach the tab to it.
+  actor::ActorKeyedService* service =
+      actor::ActorKeyedService::Get(profile_.get());
+  ASSERT_NE(nullptr, service);
+  actor::TaskId task_id = service->CreateTask(
+      actor::TestTaskSourceInfo(), actor::NoEnterprisePolicyChecker());
+  actor::ActorTask* task = service->GetTask(task_id);
+  ASSERT_NE(nullptr, task);
+
+  // Add the tab to the task.
+  MockActorUiTabController mock_controller(*tab);
+  actor::AddTabToTask(*tab, *task);
+
+  // Ensure HasActorTaskPreventingNewWebContents returns true.
+  ASSERT_TRUE(actor::HasActorTaskPreventingNewWebContents(main_frame));
+
+  // 3. Under Glic Actor control, if the frame is NOT sandboxed, it should
+  // override creation.
+  EXPECT_TRUE(delegate->IsWebContentsCreationOverridden(
+      main_frame, nullptr, content::mojom::WindowContainerType::NORMAL, GURL(),
+      "", GURL()));
+
+  // 4. Under Glic Actor control, if the frame IS sandboxed with kTopNavigation,
+  // it should NOT override creation.
+  auto simulator = content::NavigationSimulator::CreateBrowserInitiated(
+      GURL("https://sandboxed.example.com"), raw_web_contents);
+  auto headers =
+      base::MakeRefCounted<net::HttpResponseHeaders>("HTTP/1.1 200 OK");
+  headers->AddHeader("Content-Security-Policy", "sandbox allow-popups");
+  simulator->SetResponseHeaders(headers);
+  simulator->Commit();
+
+  content::RenderFrameHost* sandboxed_frame =
+      raw_web_contents->GetPrimaryMainFrame();
+  ASSERT_TRUE(sandboxed_frame->IsSandboxed(
+      network::mojom::WebSandboxFlags::kTopNavigation));
+
+  EXPECT_FALSE(delegate->IsWebContentsCreationOverridden(
+      sandboxed_frame, nullptr, content::mojom::WindowContainerType::NORMAL,
+      GURL(), "", GURL()));
+}
+
 TEST_F(TabAndroidTest, Getters) {
   TabInterfaceAndroid tab_interface(tab_android_);
   EXPECT_EQ(u"about:blank", tab_interface.GetTitle());