diff --git a/chrome/browser/ui/views/tabs/dragging/tab_drag_controller_interactive_uitest.cc b/chrome/browser/ui/views/tabs/dragging/tab_drag_controller_interactive_uitest.cc index 52a8d1aa4c43c..a016c9890ac5b 100644 --- a/chrome/browser/ui/views/tabs/dragging/tab_drag_controller_interactive_uitest.cc +++ b/chrome/browser/ui/views/tabs/dragging/tab_drag_controller_interactive_uitest.cc @@ -34,6 +34,7 @@ #include "base/test/scoped_feature_list.h" #include "base/test/test_timeouts.h" #include "base/threading/thread_restrictions.h" +#include "base/timer/timer.h" #include "build/build_config.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/browser.h" @@ -99,6 +100,7 @@ #include "ui/views/controls/native/native_view_host.h" #include "ui/views/test/widget_activation_waiter.h" #include "ui/views/view.h" +#include "ui/views/view_tracker.h" #include "ui/views/view_utils.h" #include "ui/views/widget/widget.h" #include "ui/views/window/dialog_delegate.h" @@ -663,6 +665,10 @@ void TabDragControllerTest::SetWindowFinderForTabStrip( GetTabDragController(tab_strip)->window_finder_ = std::move(window_finder); } +bool TabDragControllerTest::IsInMoveLoop(TabDragController* c) { + return c->in_move_loop_; +} + void TabDragControllerTest::HandleGestureEvent(TabStrip* tab_strip, ui::GestureEvent* event) { // Manually dispatch the event to the drag context (which has capture during @@ -815,6 +821,187 @@ IN_PROC_BROWSER_TEST_F(TabDragControllerTest, GestureEndShouldEndDragTest) { EXPECT_FALSE(IsDragSessionActive(tab_strip)); } +namespace { + +// WindowFinder whose return value is controlled by the test. +class ControllableWindowFinder : public WindowFinder { + public: + ControllableWindowFinder() = default; + ~ControllableWindowFinder() override = default; + + gfx::NativeWindow GetLocalProcessWindowAtPoint( + const gfx::Point& screen_point, + const std::set& ignore) override { + return target_; + } + + void set_target(gfx::NativeWindow target) { target_ = target; } + + private: + gfx::NativeWindow target_ = gfx::NativeWindow(); +}; + +} // namespace + +// Regression test: TabDragContextImpl::ContinueDrag returns Liveness based on +// the TabDragController's lifetime instead of `this`'s lifetime. If the source +// window is closed inside the nested RunMoveLoop and the drag re-attaches to a +// third window (so the TDC stays alive), `kAlive` is propagated to the freed +// `this` of OnGestureEvent, and line `tab_strip_->OnGestureEvent(event)` is a +// heap-use-after-free. +// +// This test only makes sense on platforms where Widget::RunMoveLoop spins a +// nestable run loop (e.g. Linux X11). It dispatches a kGestureScrollUpdate +// directly to the source window's TabDragContext (frame F1/F2's `this`), then +// from inside the nested move loop closes the source window and re-attaches +// to a third window so that RunMoveLoop returns Liveness::kAlive. +IN_PROC_BROWSER_TEST_F(TabDragControllerTest, + GestureContinueDragSourceFreedDuringMoveLoop) { + // Browser A: source with 2 tabs (so detach creates a new window C). + AddTabsAndResetBrowser(browser(), 1); + TabStrip* tab_strip_a = GetTabStripForBrowser(browser()); + + // RunMoveLoop must be supported for the bug to fire. + if (!tab_strip_a->GetWidget()->IsMoveLoopSupported()) { + GTEST_SKIP() << "Platform does not support RunMoveLoop"; + } + + // Browser B: re-attach target. + Browser* browser_b = CreateAnotherBrowserAndResize(); + TabStrip* tab_strip_b = GetTabStripForBrowser(browser_b); + Browser* browser_a = browser(); + + TabDragContext* drag_context_a = tab_strip_a->GetDragContext(); + views::ViewTracker drag_context_a_tracker(drag_context_a); + + // Start a touch drag on the selected tab of A. The event location is in + // tab-local coordinates and is forwarded to TabDragController::Init() so + // that start_point_in_screen_ ends up at the tab center. + Tab* dragged_tab = tab_strip_a->tab_at(1); + gfx::Point tab_local_center(dragged_tab->width() / 2, + dragged_tab->height() / 2); + ui::GestureEvent gesture_tap_down( + tab_local_center.x(), tab_local_center.y(), 0, base::TimeTicks(), + ui::GestureEventDetails(ui::EventType::kGestureTapDown)); + tab_strip_a->MaybeStartDrag(dragged_tab, gesture_tap_down, + tab_strip_a->GetSelectionModel()); + ASSERT_TRUE(TabDragController::IsActive()); + + // Install a controllable WindowFinder. Initially returns no window so the + // first Drag() detaches into a new window C and enters RunMoveLoop. + TabDragController* tdc = test::GetTabDragController(tab_strip_a); + ASSERT_TRUE(tdc); + auto owned_finder = std::make_unique(); + ControllableWindowFinder* finder = owned_finder.get(); + SetWindowFinderForTabStrip(tab_strip_a, std::move(owned_finder)); + + // Compute the kGestureScrollUpdate location in drag_context_a's coordinates + // such that the resulting screen point is well outside A's tabstrip + // (>kTouchVerticalDetachMagnetism below) and also far enough from the start + // point to satisfy CanStartDrag(). + gfx::Point detach_point_screen = + ui_test_utils::GetCenterInScreenCoordinates(dragged_tab) + + gfx::Vector2d(0, GetDetachY(tab_strip_a)); + gfx::Point detach_point_in_context = detach_point_screen; + views::View::ConvertPointFromScreen(drag_context_a, &detach_point_in_context); + + bool step2_ran = false; + + // Step 2 runs inside the nested RunMoveLoop. By now the TDC has been + // re-homed to a freshly-created window C, A's IsTabStripCloseable() is true, + // and frames F1 (OnGestureEvent) / F2 (ContinueDrag) have `this` == A's + // (still-live) drag context. + auto step2 = base::BindLambdaForTesting([&]() { + step2_ran = true; + // We must be inside RunMoveLoop with the TDC owned by window C. + ASSERT_TRUE(TabDragController::IsActive()); + ASSERT_FALSE(test::IsDragSessionActive(tab_strip_a)); + ASSERT_EQ(3u, GlobalBrowserCollection::GetInstance()->GetSize()); + + // Close source window A. This is what an attacker-controlled renderer + // (window.close()) or no-permission extension (chrome.windows.remove()) + // would do while the user is mid-drag. A's TabStrip dtor runs + // RemoveChildViewT(drag_context_), freeing the `this` of frames F1/F2. + LOG(ERROR) << "POC: closing source browser A inside nested RunMoveLoop"; + browser_a->window()->Close(); + { + base::RunLoop loop(base::RunLoop::Type::kNestableTasksAllowed); + auto poll = std::make_unique(); + poll->Start(FROM_HERE, base::Milliseconds(10), + base::BindLambdaForTesting([&]() { + if (!drag_context_a_tracker.view()) { + loop.Quit(); + } + })); + loop.Run(); + } + ASSERT_EQ(nullptr, drag_context_a_tracker.view()) + << "POC: source TabDragContextImpl was not freed"; + ASSERT_TRUE(TabDragController::IsActive()); + LOG(ERROR) << "POC: source TabDragContextImpl freed; F1/F2 `this` now " + "dangles. Re-attaching to B so RunMoveLoop returns kAlive."; + + // Now point the WindowFinder at B and call Drag() at B's tab(0) center. + // With current_state_==kDraggingWindow this enters + // DragBrowserToNewTabStrip(B), sets kWaitingToExitRunLoop and calls + // EndMoveLoop(). RunMoveLoop will then return Liveness::kAlive. + StopAnimating(tab_strip_b); + tab_strip_b->GetWidget()->LayoutRootViewIfNecessary(); + finder->set_target(browser_b->window()->GetNativeWindow()); + gfx::Point b_tab_center = + ui_test_utils::GetCenterInScreenCoordinates(tab_strip_b->tab_at(0)); + ASSERT_EQ(TabDragController::Liveness::kAlive, tdc->Drag(b_tab_center)); + LOG(ERROR) << "POC: Drag(B) returned; B drag session active=" + << test::IsDragSessionActive(tab_strip_b) + << " TDC active=" << TabDragController::IsActive(); + }); + + // DetachIntoNewBrowserAndRunMoveLoop may spin a VisibilityWaiter run loop + // (under kInitialWebUI) before reaching Widget::RunMoveLoop. Keep re-posting + // until the TDC is actually inside the move loop (in_move_loop_==true), so + // that step2's tdc->Drag(B) hits DragBrowserToNewTabStrip and EndMoveLoop() + // and is not swallowed by the kWaitingForWindowToShow early-return. + auto wait_for_move_loop_then_step2 = + std::make_unique(); + *wait_for_move_loop_then_step2 = base::BindLambdaForTesting( + [&, ptr = wait_for_move_loop_then_step2.get()]() { + if (!IsInMoveLoop(tdc)) { + base::SequencedTaskRunner::GetCurrentDefault()->PostTask(FROM_HERE, + *ptr); + return; + } + std::move(step2).Run(); + }); + base::SequencedTaskRunner::GetCurrentDefault()->PostTask( + FROM_HERE, *wait_for_move_loop_then_step2); + + // F1: dispatch kGestureScrollUpdate to A's drag context. ContinueDrag (F2) + // calls Drag() which detaches into C and enters RunMoveLoop. When step2 ends + // the move loop with kWaitingToExitRunLoop, RunMoveLoop returns kAlive; F2 + // propagates kAlive; F1's `tabstrip_alive` guard passes; F1 then reads + // `this->tab_strip_` from the freed TabDragContextImpl. ASAN reports a + // heap-use-after-free here. + ui::GestureEvent gesture_scroll_update( + detach_point_in_context.x(), detach_point_in_context.y(), 0, + base::TimeTicks(), + ui::GestureEventDetails(ui::EventType::kGestureScrollUpdate, 0, 0)); + LOG(ERROR) << "POC: dispatching kGestureScrollUpdate to A's drag context " + "(this enters the nested RunMoveLoop)"; + drag_context_a->OnGestureEvent(&gesture_scroll_update); + + // If we get here without ASAN tripping, the bug is fixed. + EXPECT_TRUE(step2_ran); + EXPECT_EQ(nullptr, drag_context_a_tracker.view()); + + // Clean up: end any remaining drag. + if (TabDragController::IsActive()) { + ui::GestureEvent gesture_end( + 0, 0, 0, base::TimeTicks(), + ui::GestureEventDetails(ui::EventType::kGestureEnd)); + tab_strip_b->GetDragContext()->OnGestureEvent(&gesture_end); + } +} + class DetachToBrowserTabDragControllerTest : public TabDragControllerInteractiveTestMixin, public ::testing::WithParamInterface> { diff --git a/chrome/browser/ui/views/tabs/dragging/tab_drag_controller_interactive_uitest.h b/chrome/browser/ui/views/tabs/dragging/tab_drag_controller_interactive_uitest.h index 5b50d9c0bd9b5..13ec0e827c1f5 100644 --- a/chrome/browser/ui/views/tabs/dragging/tab_drag_controller_interactive_uitest.h +++ b/chrome/browser/ui/views/tabs/dragging/tab_drag_controller_interactive_uitest.h @@ -62,6 +62,9 @@ class TabDragControllerTest : public InProcessBrowserTest { bool HasDragStarted(TabStrip* tab_strip) const; + // Exposes TabDragController::in_move_loop_ to derived test bodies. + static bool IsInMoveLoop(TabDragController* c); + void SetTabDragPointResolver(TabDragPointResolver& resolver); // InProcessBrowserTest: