From 7907fe19b3eda099b6b9a78434c56fd6edb637f5 Mon Sep 17 00:00:00 2001
From: Andrew Paseltiner <apaseltiner@chromium.org>
Date: Wed, 29 Apr 2026 08:26:00 -0700
Subject: [PATCH] [Glic] Fix UAF and crashes in actor tools during synchronous frame detachment

This CL fixes Use-After-Free (UAF) vulnerabilities and potential crashes
in several Glic actor tools (Click, Type, and DragAndRelease) that occur
when a page script synchronously detaches a frame or modifies the DOM in
response to an input event.

When these tools dispatch input events (like MouseDown or KeyDown) to
the renderer, the page may execute script that results in the
destruction of the tool object itself or its associated resources. To
handle this safely, we now:

- Use base::WeakPtr to check if the tool object is still alive after
  each synchronous event dispatch.
- Return early if the tool has been destroyed, preventing access to
  freed memory.
- Ensure that the WebWidget is re-validated where necessary.

Regression tests are added in uaf_regression_browsertest.cc using a
nested "ABA" iframe structure to reliably trigger synchronous
detachment. This suite covers the new fixes for Click, Type, and
DragAndRelease.

Bug: 506377279
Change-Id: Ie0753f925b7264a127aa1e37d7e41bcabb25db99
Fixed: 506150628
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7800758
Commit-Queue: Andrew Paseltiner <apaseltiner@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1622471}
---

diff --git a/chrome/browser/actor/BUILD.gn b/chrome/browser/actor/BUILD.gn
index 9bacadda..4b6e449f5 100644
--- a/chrome/browser/actor/BUILD.gn
+++ b/chrome/browser/actor/BUILD.gn
@@ -460,6 +460,7 @@
       "tools/tab_management_tool_browsertest.cc",
       "tools/tool_agnostic_browsertest.cc",
       "tools/type_tool_browsertest.cc",
+      "tools/uaf_regression_browsertest.cc",
       "tools/wait_tool_browsertest.cc",
       "tools/window_management_tool_browsertest.cc",
     ]
diff --git a/chrome/browser/actor/tools/select_tool_browsertest.cc b/chrome/browser/actor/tools/select_tool_browsertest.cc
index 4df0768..e52ff28b 100644
--- a/chrome/browser/actor/tools/select_tool_browsertest.cc
+++ b/chrome/browser/actor/tools/select_tool_browsertest.cc
@@ -408,51 +408,5 @@
             "delta");
 }
 
-// Regression test for UAF in Glic actor tools.
-// See crbug.com/506377279.
-IN_PROC_BROWSER_TEST_F(ActorSelectToolBrowserTest,
-                       SelectTool_HandlesSynchronousFrameDetachment) {
-  GURL outer_url = embedded_https_test_server().GetURL(
-      "a.test", "/actor/select_tool_uaf_outer.html");
-  GURL mid_url = embedded_https_test_server().GetURL(
-      "b.test", "/actor/select_tool_uaf_mid.html");
-  GURL inner_url = embedded_https_test_server().GetURL(
-      "a.test", "/actor/select_tool_uaf_inner.html");
-
-  ASSERT_TRUE(content::NavigateToURL(web_contents(), outer_url));
-  content::WaitForLoadStop(web_contents());
-
-  // Set mid iframe src
-  ASSERT_TRUE(content::ExecJs(
-      web_contents(),
-      content::JsReplace("document.getElementById('mid').src = $1", mid_url)));
-  content::WaitForLoadStop(web_contents());
-
-  content::RenderFrameHost* mid_rfh = content::ChildFrameAt(main_frame(), 0);
-  ASSERT_TRUE(mid_rfh);
-
-  // Set inner iframe src
-  ASSERT_TRUE(content::ExecJs(
-      mid_rfh, content::JsReplace("document.getElementById('inner').src = $1",
-                                  inner_url)));
-  content::WaitForLoadStop(web_contents());
-
-  content::RenderFrameHost* inner_rfh = content::ChildFrameAt(mid_rfh, 0);
-  ASSERT_TRUE(inner_rfh);
-
-  const std::string select_id = "#s";
-  const int32_t select_dom_node_id =
-      GetDOMNodeId(*inner_rfh, select_id).value();
-
-  // Trigger SelectTool on the inner frame.
-  std::unique_ptr<ToolRequest> action =
-      MakeSelectRequest(*inner_rfh, select_dom_node_id, "b");
-  ActResultFuture result;
-  actor_task().Act(ToRequestList(action), result.GetCallback());
-
-  // With the fix, this should not crash the renderer.
-  ASSERT_TRUE(result.Wait());
-}
-
 }  // namespace
 }  // namespace actor
diff --git a/chrome/browser/actor/tools/uaf_regression_browsertest.cc b/chrome/browser/actor/tools/uaf_regression_browsertest.cc
new file mode 100644
index 0000000..dd032bba
--- /dev/null
+++ b/chrome/browser/actor/tools/uaf_regression_browsertest.cc
@@ -0,0 +1,153 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/test/test_future.h"
+#include "chrome/browser/actor/actor_test_util.h"
+#include "chrome/browser/actor/tools/tool_request.h"
+#include "chrome/browser/actor/tools/tools_test_util.h"
+#include "chrome/common/actor.mojom.h"
+#include "chrome/common/actor/action_result.h"
+#include "content/public/test/browser_test.h"
+#include "content/public/test/browser_test_utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/gfx/geometry/point.h"
+#include "ui/gfx/geometry/point_conversions.h"
+
+namespace actor {
+
+namespace {
+
+using base::test::TestFuture;
+using content::ChildFrameAt;
+using content::ExecJs;
+using content::GetDOMNodeId;
+using content::RenderFrameHost;
+using ActResultFuture = TestFuture<std::vector<ActionResultWithLatencyInfo>>;
+
+class ActorUafRegressionBrowserTest : public ActorToolsTest {
+ public:
+  void SetUpOnMainThread() override {
+    ActorToolsTest::SetUpOnMainThread();
+    ASSERT_TRUE(embedded_test_server()->Start());
+    embedded_https_test_server().SetSSLConfig(
+        net::EmbeddedTestServer::CERT_TEST_NAMES);
+    ASSERT_TRUE(embedded_https_test_server().Start());
+  }
+
+  void SetupAbaFrames(const std::string& inner_path) {
+    GURL outer_url =
+        embedded_https_test_server().GetURL("a.test", "/actor/uaf_outer.html");
+    GURL mid_url =
+        embedded_https_test_server().GetURL("b.test", "/actor/uaf_mid.html");
+    GURL inner_url = embedded_https_test_server().GetURL("a.test", inner_path);
+
+    ASSERT_TRUE(content::NavigateToURL(web_contents(), outer_url));
+    content::WaitForLoadStop(web_contents());
+
+    // Set mid iframe src
+    ASSERT_TRUE(
+        ExecJs(web_contents(),
+               content::JsReplace("document.getElementById('mid').src = $1",
+                                  mid_url)));
+    content::WaitForLoadStop(web_contents());
+
+    RenderFrameHost* mid_rfh = ChildFrameAt(main_frame(), 0);
+    ASSERT_TRUE(mid_rfh);
+
+    // Set inner iframe src
+    ASSERT_TRUE(ExecJs(
+        mid_rfh, content::JsReplace("document.getElementById('inner').src = $1",
+                                    inner_url)));
+    content::WaitForLoadStop(web_contents());
+  }
+
+  RenderFrameHost* GetInnerRfh() {
+    RenderFrameHost* mid_rfh = ChildFrameAt(main_frame(), 0);
+    if (!mid_rfh) {
+      return nullptr;
+    }
+    return ChildFrameAt(mid_rfh, 0);
+  }
+};
+
+// Regression test for UAF in Glic actor tools.
+// See crbug.com/506150628.
+IN_PROC_BROWSER_TEST_F(ActorUafRegressionBrowserTest,
+                       ClickTool_HandlesSynchronousFrameDetachment) {
+  SetupAbaFrames("/actor/click_tool_uaf_inner.html");
+  RenderFrameHost* inner_rfh = GetInnerRfh();
+  ASSERT_TRUE(inner_rfh);
+
+  const int32_t target_id = GetDOMNodeId(*inner_rfh, "#target").value();
+
+  std::unique_ptr<ToolRequest> action = MakeClickRequest(*inner_rfh, target_id);
+  ActResultFuture result;
+  actor_task().Act(ToRequestList(action), result.GetCallback());
+
+  // This should not crash the renderer.
+  ASSERT_TRUE(result.Wait());
+}
+

diff --git a/chrome/browser/actor/tools/select_tool_browsertest.cc b/chrome/browser/actor/tools/select_tool_browsertest.cc
index 4df0768..e52ff28b 100644
--- a/chrome/browser/actor/tools/select_tool_browsertest.cc
+++ b/chrome/browser/actor/tools/select_tool_browsertest.cc
@@ -408,51 +408,5 @@
             "delta");
 }
 
-// Regression test for UAF in Glic actor tools.
-// See crbug.com/506377279.
-IN_PROC_BROWSER_TEST_F(ActorSelectToolBrowserTest,
-                       SelectTool_HandlesSynchronousFrameDetachment) {
-  GURL outer_url = embedded_https_test_server().GetURL(
-      "a.test", "/actor/select_tool_uaf_outer.html");
-  GURL mid_url = embedded_https_test_server().GetURL(
-      "b.test", "/actor/select_tool_uaf_mid.html");
-  GURL inner_url = embedded_https_test_server().GetURL(
-      "a.test", "/actor/select_tool_uaf_inner.html");
-
-  ASSERT_TRUE(content::NavigateToURL(web_contents(), outer_url));
-  content::WaitForLoadStop(web_contents());
-
-  // Set mid iframe src
-  ASSERT_TRUE(content::ExecJs(
-      web_contents(),
-      content::JsReplace("document.getElementById('mid').src = $1", mid_url)));
-  content::WaitForLoadStop(web_contents());
-
-  content::RenderFrameHost* mid_rfh = content::ChildFrameAt(main_frame(), 0);
-  ASSERT_TRUE(mid_rfh);
-
-  // Set inner iframe src
-  ASSERT_TRUE(content::ExecJs(
-      mid_rfh, content::JsReplace("document.getElementById('inner').src = $1",
-                                  inner_url)));
-  content::WaitForLoadStop(web_contents());
-
-  content::RenderFrameHost* inner_rfh = content::ChildFrameAt(mid_rfh, 0);
-  ASSERT_TRUE(inner_rfh);
-
-  const std::string select_id = "#s";
-  const int32_t select_dom_node_id =
-      GetDOMNodeId(*inner_rfh, select_id).value();
-
-  // Trigger SelectTool on the inner frame.
-  std::unique_ptr<ToolRequest> action =
-      MakeSelectRequest(*inner_rfh, select_dom_node_id, "b");
-  ActResultFuture result;
-  actor_task().Act(ToRequestList(action), result.GetCallback());
-
-  // With the fix, this should not crash the renderer.
-  ASSERT_TRUE(result.Wait());
-}
-
 }  // namespace
 }  // namespace actor
diff --git a/chrome/browser/actor/tools/uaf_regression_browsertest.cc b/chrome/browser/actor/tools/uaf_regression_browsertest.cc
new file mode 100644
index 0000000..dd032bba
--- /dev/null
+++ b/chrome/browser/actor/tools/uaf_regression_browsertest.cc
@@ -0,0 +1,153 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/test/test_future.h"
+#include "chrome/browser/actor/actor_test_util.h"
+#include "chrome/browser/actor/tools/tool_request.h"
+#include "chrome/browser/actor/tools/tools_test_util.h"
+#include "chrome/common/actor.mojom.h"
+#include "chrome/common/actor/action_result.h"
+#include "content/public/test/browser_test.h"
+#include "content/public/test/browser_test_utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/gfx/geometry/point.h"
+#include "ui/gfx/geometry/point_conversions.h"
+
+namespace actor {
+
+namespace {
+
+using base::test::TestFuture;
+using content::ChildFrameAt;
+using content::ExecJs;
+using content::GetDOMNodeId;
+using content::RenderFrameHost;
+using ActResultFuture = TestFuture<std::vector<ActionResultWithLatencyInfo>>;
+
+class ActorUafRegressionBrowserTest : public ActorToolsTest {
+ public:
+  void SetUpOnMainThread() override {
+    ActorToolsTest::SetUpOnMainThread();
+    ASSERT_TRUE(embedded_test_server()->Start());
+    embedded_https_test_server().SetSSLConfig(
+        net::EmbeddedTestServer::CERT_TEST_NAMES);
+    ASSERT_TRUE(embedded_https_test_server().Start());
+  }
+
+  void SetupAbaFrames(const std::string& inner_path) {
+    GURL outer_url =
+        embedded_https_test_server().GetURL("a.test", "/actor/uaf_outer.html");
+    GURL mid_url =
+        embedded_https_test_server().GetURL("b.test", "/actor/uaf_mid.html");
+    GURL inner_url = embedded_https_test_server().GetURL("a.test", inner_path);
+
+    ASSERT_TRUE(content::NavigateToURL(web_contents(), outer_url));
+    content::WaitForLoadStop(web_contents());
+
+    // Set mid iframe src
+    ASSERT_TRUE(
+        ExecJs(web_contents(),
+               content::JsReplace("document.getElementById('mid').src = $1",
+                                  mid_url)));
+    content::WaitForLoadStop(web_contents());
+
+    RenderFrameHost* mid_rfh = ChildFrameAt(main_frame(), 0);
+    ASSERT_TRUE(mid_rfh);
+
+    // Set inner iframe src
+    ASSERT_TRUE(ExecJs(
+        mid_rfh, content::JsReplace("document.getElementById('inner').src = $1",
+                                    inner_url)));
+    content::WaitForLoadStop(web_contents());
+  }
+
+  RenderFrameHost* GetInnerRfh() {
+    RenderFrameHost* mid_rfh = ChildFrameAt(main_frame(), 0);
+    if (!mid_rfh) {
+      return nullptr;
+    }
+    return ChildFrameAt(mid_rfh, 0);
+  }
+};
+
+// Regression test for UAF in Glic actor tools.
+// See crbug.com/506150628.
+IN_PROC_BROWSER_TEST_F(ActorUafRegressionBrowserTest,
+                       ClickTool_HandlesSynchronousFrameDetachment) {
+  SetupAbaFrames("/actor/click_tool_uaf_inner.html");
+  RenderFrameHost* inner_rfh = GetInnerRfh();
+  ASSERT_TRUE(inner_rfh);
+
+  const int32_t target_id = GetDOMNodeId(*inner_rfh, "#target").value();
+
+  std::unique_ptr<ToolRequest> action = MakeClickRequest(*inner_rfh, target_id);
+  ActResultFuture result;
+  actor_task().Act(ToRequestList(action), result.GetCallback());
+
+  // This should not crash the renderer.
+  ASSERT_TRUE(result.Wait());
+}
+
+// Regression test for UAF in Glic actor tools.
+// See crbug.com/506150628.
+IN_PROC_BROWSER_TEST_F(ActorUafRegressionBrowserTest,
+                       TypeTool_HandlesSynchronousFrameDetachment) {
+  SetupAbaFrames("/actor/type_tool_uaf_inner.html");
+  RenderFrameHost* inner_rfh = GetInnerRfh();
+  ASSERT_TRUE(inner_rfh);
+
+  const int32_t target_id = GetDOMNodeId(*inner_rfh, "#target").value();
+
+  std::unique_ptr<ToolRequest> action = MakeTypeRequest(
+      *inner_rfh, target_id, "hello", /* follow_by_enter= */ false);
+  ActResultFuture result;
+  actor_task().Act(ToRequestList(action), result.GetCallback());
+
+  // This should not crash the renderer.
+  ASSERT_TRUE(result.Wait());
+}
+
+// Regression test for UAF in Glic actor tools.
+// See crbug.com/506150628.
+IN_PROC_BROWSER_TEST_F(ActorUafRegressionBrowserTest,
+                       DragAndReleaseTool_HandlesSynchronousFrameDetachment) {
+  SetupAbaFrames("/actor/drag_and_release_tool_uaf_inner.html");
+  RenderFrameHost* inner_rfh = GetInnerRfh();
+  ASSERT_TRUE(inner_rfh);
+
+  gfx::RectF bounds = GetBoundingClientRect(*inner_rfh, "#target");
+  gfx::Point from_point = gfx::ToRoundedPoint(bounds.CenterPoint());
+  gfx::Point to_point = from_point + gfx::Vector2d(100, 100);
+
+  // Drag from target to somewhere else.
+  std::unique_ptr<ToolRequest> action =
+      MakeDragAndReleaseRequest(*active_tab(), from_point, to_point);
+  ActResultFuture result;
+  actor_task().Act(ToRequestList(action), result.GetCallback());
+
+  // This should not crash the renderer.
+  ASSERT_TRUE(result.Wait());
+}
+
+// Regression test for UAF in Glic actor tools.
+// See crbug.com/506377279.
+IN_PROC_BROWSER_TEST_F(ActorUafRegressionBrowserTest,
+                       SelectTool_HandlesSynchronousFrameDetachment) {
+  SetupAbaFrames("/actor/select_tool_uaf_inner.html");
+  RenderFrameHost* inner_rfh = GetInnerRfh();
+  ASSERT_TRUE(inner_rfh);
+
+  const int32_t target_id = GetDOMNodeId(*inner_rfh, "#s").value();
+
+  std::unique_ptr<ToolRequest> action =
+      MakeSelectRequest(*inner_rfh, target_id, "b");
+  ActResultFuture result;
+  actor_task().Act(ToRequestList(action), result.GetCallback());
+
+  // This should not crash the renderer.
+  ASSERT_TRUE(result.Wait());
+}
+
+}  // namespace
+}  // namespace actor
diff --git a/chrome/test/data/actor/click_tool_uaf_inner.html b/chrome/test/data/actor/click_tool_uaf_inner.html
new file mode 100644
index 0000000..86ef17c1
--- /dev/null
+++ b/chrome/test/data/actor/click_tool_uaf_inner.html
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html>
+<body>
+  <div id="target" style="width: 100px; height: 100px; background: red;"></div>
+  <script>
+    const target = document.getElementById('target');
+    target.addEventListener('mousedown', () => {
+      // Remove the mid iframe from the top window.
+      // Since mid is cross-origin, removing it synchronously detaches this frame.
+      parent.parent.document.getElementById('mid').remove();
+    });
+  </script>
+</body>
+</html>
diff --git a/chrome/test/data/actor/drag_and_release_tool_uaf_inner.html b/chrome/test/data/actor/drag_and_release_tool_uaf_inner.html
new file mode 100644
index 0000000..19cfb22
--- /dev/null
+++ b/chrome/test/data/actor/drag_and_release_tool_uaf_inner.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+<body>
+  <div id="target" style="width: 100px; height: 100px; background: blue;"></div>
+  <script>
+    const target = document.getElementById('target');
+    target.addEventListener('mousedown', () => {
+      parent.parent.document.getElementById('mid').remove();
+    });
+  </script>
+</body>
+</html>
diff --git a/chrome/test/data/actor/select_tool_uaf_mid.html b/chrome/test/data/actor/select_tool_uaf_mid.html
deleted file mode 100644
index 39847961..0000000
--- a/chrome/test/data/actor/select_tool_uaf_mid.html
+++ /dev/null
@@ -1,6 +0,0 @@
-<!DOCTYPE html>
-<html>
-<body>
-  <iframe id="inner" width="400" height="400"></iframe>
-</body>
-</html>
diff --git a/chrome/test/data/actor/select_tool_uaf_outer.html b/chrome/test/data/actor/select_tool_uaf_outer.html
deleted file mode 100644
index 8a0f98a..0000000
--- a/chrome/test/data/actor/select_tool_uaf_outer.html
+++ /dev/null
@@ -1,6 +0,0 @@
-<!DOCTYPE html>
-<html>
-<body>
-  <iframe id="mid" width="500" height="500"></iframe>
-</body>
-</html>
diff --git a/chrome/test/data/actor/type_tool_uaf_inner.html b/chrome/test/data/actor/type_tool_uaf_inner.html
new file mode 100644
index 0000000..c55a208b
--- /dev/null
+++ b/chrome/test/data/actor/type_tool_uaf_inner.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+<body>
+  <input id="target" type="text">
+  <script>
+    const target = document.getElementById('target');
+    target.addEventListener('keydown', () => {
+      parent.parent.document.getElementById('mid').remove();
+    });
+  </script>
+</body>
+</html>
diff --git a/chrome/test/data/actor/uaf_mid.html b/chrome/test/data/actor/uaf_mid.html
new file mode 100644
index 0000000..39847961
--- /dev/null
+++ b/chrome/test/data/actor/uaf_mid.html
... (truncated)