diff --git a/chrome/browser/actor/tools/uaf_regression_browsertest.cc b/chrome/browser/actor/tools/uaf_regression_browsertest.cc index 34086a4e64c31..0aa2befb07e27 100644 --- a/chrome/browser/actor/tools/uaf_regression_browsertest.cc +++ b/chrome/browser/actor/tools/uaf_regression_browsertest.cc @@ -2,12 +2,14 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/test/scoped_feature_list.h" #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 "chrome/common/chrome_features.h" #include "content/public/test/browser_test.h" #include "content/public/test/browser_test_utils.h" #include "testing/gtest/include/gtest/gtest.h" @@ -148,5 +150,40 @@ IN_PROC_BROWSER_TEST_F(ActorUafRegressionBrowserTest, ASSERT_TRUE(result.Wait()); } +// Configures timing so that the browser-side tool timeout fires while the +// renderer is between mouse down and mouse up, exercising the cancellation +// path. +class ActorUafRegressionCancelBrowserTest + : public ActorUafRegressionBrowserTest { + public: + ActorUafRegressionCancelBrowserTest() { + feature_list_.InitWithFeaturesAndParameters( + /*enabled_features=*/ + {{features::kGlicActor, + {{features::kGlicActorClickDelay.name, "30s"}, + {features::kGlicActorPageToolTimeout.name, "1s"}}}}, + /*disabled_features=*/{features::kGlicActorMoveBeforeClick}); + } + + private: + base::test::ScopedFeatureList feature_list_; +}; + +IN_PROC_BROWSER_TEST_F(ActorUafRegressionCancelBrowserTest, + ClickTool_HandlesSynchronousFrameDetachmentOnCancel) { + SetupAbaFrames("/actor/click_tool_cancel_uaf_inner.html"); + RenderFrameHost* inner_rfh = GetInnerRfh(); + ASSERT_TRUE(inner_rfh); + + const int32_t target_id = GetDOMNodeId(*inner_rfh, "#target").value(); + + std::unique_ptr 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()); +} + } // namespace } // namespace actor diff --git a/chrome/renderer/actor/click_tool.cc b/chrome/renderer/actor/click_tool.cc index afc9a8ba20023..aba2471f2094f 100644 --- a/chrome/renderer/actor/click_tool.cc +++ b/chrome/renderer/actor/click_tool.cc @@ -110,7 +110,11 @@ bool ClickTool::SupportsPaintStability() const { void ClickTool::Cancel() { if (click_dispatcher_) { + base::WeakPtr weak_this = weak_ptr_factory_.GetWeakPtr(); click_dispatcher_->Cancel(); + if (!weak_this) { + return; + } click_dispatcher_.reset(); } } diff --git a/chrome/renderer/actor/script_tool.cc b/chrome/renderer/actor/script_tool.cc index a64c31af983eb..bc1dc80414c12 100644 --- a/chrome/renderer/actor/script_tool.cc +++ b/chrome/renderer/actor/script_tool.cc @@ -112,7 +112,11 @@ void ScriptTool::Cancel() { if (!execution_id_.has_value()) { return; } + base::WeakPtr weak_this = weak_ptr_factory_.GetWeakPtr(); frame_->GetWebFrame()->GetDocument().CancelScriptTool(execution_id_.value()); + if (!weak_this) { + return; + } execution_id_.reset(); } diff --git a/chrome/renderer/actor/tool_executor.cc b/chrome/renderer/actor/tool_executor.cc index 9230e74f9441f..247dae795ae63 100644 --- a/chrome/renderer/actor/tool_executor.cc +++ b/chrome/renderer/actor/tool_executor.cc @@ -270,7 +270,11 @@ void ToolExecutor::CancelTool(const actor::TaskId& task_id) { // The browser and renderer should agree on the active tool. CHECK_EQ(tool_->task_id(), task_id); + base::WeakPtr weak_this = weak_ptr_factory_.GetWeakPtr(); tool_->Cancel(); + if (!weak_this) { + return; + } // The result code doesn't matter as it will be ignored by the browser // process. diff --git a/chrome/renderer/actor/type_tool.cc b/chrome/renderer/actor/type_tool.cc index c8aeb1db42b99..52bfaff68ff39 100644 --- a/chrome/renderer/actor/type_tool.cc +++ b/chrome/renderer/actor/type_tool.cc @@ -501,12 +501,19 @@ void TypeTool::Cancel() { // Clicking is completed before key dispatching, so there shouldn't be both. CHECK(!(click_dispatcher_ && key_dispatcher_)); + base::WeakPtr weak_this = weak_ptr_factory_.GetWeakPtr(); if (click_dispatcher_) { click_dispatcher_->Cancel(); + if (!weak_this) { + return; + } click_dispatcher_.reset(); } if (key_dispatcher_) { key_dispatcher_->Cancel(); + if (!weak_this) { + return; + } key_dispatcher_.reset(); } } diff --git a/chrome/test/data/actor/click_tool_cancel_uaf_inner.html b/chrome/test/data/actor/click_tool_cancel_uaf_inner.html new file mode 100644 index 0000000000000..98be51867fd85 --- /dev/null +++ b/chrome/test/data/actor/click_tool_cancel_uaf_inner.html @@ -0,0 +1,14 @@ + + + +
+ + +