[go: up one dir, main page]

[Merge to M96] wayland: tabdrag: fix UAF when the dragged window is destroyed

While in a tab drag session, in detached mode, the browser window being
dragged may get suddenly destroyed, eg: programmatically via JS, which
currently may lead to use-after-free issues, such as the one described
in the linked issue. This fixes it by properly handling the dragged
window destruction after quitting the drag loop.

R=​msisov@igalia.com

(cherry picked from commit 7280aa30a74b434c714190479cd02c31a064c4e5)

Bug: 1267791
Change-Id: I22c9b2a8fa06d7d5b50cefae72b24dbd4931f60d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3300755
Commit-Queue: Nick Yamane <nickdiego@igalia.com>
Reviewed-by: Alexander Dunaev <adunaev@igalia.com>
Cr-Original-Commit-Position: refs/heads/main@{#945424}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3304119
Auto-Submit: Nick Yamane <nickdiego@igalia.com>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/branch-heads/4664@{#1179}
Cr-Branched-From: 24dc4ee75e01a29d390d43c9c264372a169273a7-refs/heads/main@{#929512}
diff --git a/ui/ozone/platform/wayland/host/wayland_window_drag_controller.cc b/ui/ozone/platform/wayland/host/wayland_window_drag_controller.cc
index ef4f090..86bafc1b 100644
--- a/ui/ozone/platform/wayland/host/wayland_window_drag_controller.cc
+++ b/ui/ozone/platform/wayland/host/wayland_window_drag_controller.cc
@@ -391,6 +391,11 @@
 void WaylandWindowDragController::OnWindowRemoved(WaylandWindow* window) {
   DCHECK_NE(state_, State::kIdle);
   DCHECK_NE(window, dragged_window_);
+  VLOG(1) << "Window being destroyed. widget=" << window->GetWidget();
+
+  if (window == pointer_grab_owner_)
+    pointer_grab_owner_ = nullptr;
+
   if (window == origin_window_)
     origin_surface_ = origin_window_->TakeWaylandSurface();
 }
@@ -425,18 +430,19 @@
 // about to finish.
 void WaylandWindowDragController::HandleDropAndResetState() {
   DCHECK_EQ(state_, State::kDropped);
-  DCHECK(pointer_grab_owner_);
+  DCHECK(drag_source_);
   VLOG(1) << "Notifying drop. window=" << pointer_grab_owner_;
 
   if (*drag_source_ == DragSource::kMouse) {
-    EventFlags pointer_button = EF_LEFT_MOUSE_BUTTON;
-    pointer_delegate_->OnPointerButtonEvent(ET_MOUSE_RELEASED, pointer_button,
-                                            pointer_grab_owner_);
+    if (pointer_grab_owner_) {
+      pointer_delegate_->OnPointerButtonEvent(
+          ET_MOUSE_RELEASED, EF_LEFT_MOUSE_BUTTON, pointer_grab_owner_);
+    }
   } else {
-    base::TimeTicks timestamp = base::TimeTicks::Now();
     auto touch_pointer_ids = touch_delegate_->GetActiveTouchPointIds();
     DCHECK_EQ(touch_pointer_ids.size(), 1u);
-    touch_delegate_->OnTouchReleaseEvent(timestamp, touch_pointer_ids[0]);
+    touch_delegate_->OnTouchReleaseEvent(base::TimeTicks::Now(),
+                                         touch_pointer_ids[0]);
   }
 
   pointer_grab_owner_ = nullptr;
diff --git a/ui/ozone/platform/wayland/host/wayland_window_drag_controller.h b/ui/ozone/platform/wayland/host/wayland_window_drag_controller.h
index e8f2d5c..fd75f24 100644
--- a/ui/ozone/platform/wayland/host/wayland_window_drag_controller.h
+++ b/ui/ozone/platform/wayland/host/wayland_window_drag_controller.h
@@ -82,6 +82,9 @@
  private:
   class ExtendedDragSource;
 
+  FRIEND_TEST_ALL_PREFIXES(WaylandWindowDragControllerTest,
+                           HandleDraggedWindowDestruction);
+
   // WaylandDataDevice::DragDelegate:
   bool IsDragSource() const override;
   void DrawIcon() override;
diff --git a/ui/ozone/platform/wayland/host/wayland_window_drag_controller_unittest.cc b/ui/ozone/platform/wayland/host/wayland_window_drag_controller_unittest.cc
index 28d84dd..59e0c6b 100644
--- a/ui/ozone/platform/wayland/host/wayland_window_drag_controller_unittest.cc
+++ b/ui/ozone/platform/wayland/host/wayland_window_drag_controller_unittest.cc
@@ -1015,6 +1015,54 @@
   move_loop_handler->RunMoveLoop({});
 }
 
+// Ensure no memory issues happen when the dragged window is destroyed just
+// after quitting the move loop. Regression test for crbug.com/1267791 and
+// should be caught in both regular and ASAN builds, where more details about
+// the actual memory issue is provided.
+TEST_P(WaylandWindowDragControllerTest, HandleDraggedWindowDestruction) {
+  // 1. Ensure there is no window currently focused
+  EXPECT_FALSE(window_manager()->GetCurrentPointerFocusedWindow());
+  SendPointerEnter(window_.get(), &delegate_);
+  SendPointerPress(window_.get(), &delegate_, BTN_LEFT);
+  SendPointerMotion(window_.get(), &delegate_, {10, 10});
+  Mock::VerifyAndClearExpectations(&delegate_);
+
+  // 2. Start the window drag session.
+  auto* wayland_extension = GetWaylandExtension(*window_);
+  wayland_extension->StartWindowDraggingSessionIfNeeded();
+  EXPECT_EQ(State::kAttached, drag_controller()->state());
+
+  auto* move_loop_handler = GetWmMoveLoopHandler(*window_);
+  ASSERT_TRUE(move_loop_handler);
+  ScheduleTestTask(
+      base::BindLambdaForTesting([&]() { move_loop_handler->EndMoveLoop(); }));
+
+  // 3. Run the move loop.
+  move_loop_handler->RunMoveLoop({});
+  Sync();
+
+  // 4. Destroy the dragged window just after quitting move loop.
+  const auto* dangling_window_ptr = window_.get();
+  window_.reset();
+  Sync();
+  EXPECT_NE(dangling_window_ptr, drag_controller()->pointer_grab_owner_);
+  EXPECT_EQ(State::kAttached, drag_controller()->state());
+
+  // 5. Ensure no events are dispatched for drop. Which indirectly means that
+  // drop handling code at window drag controller does not call into the above
+  // destroyed dragged window.
+  EXPECT_CALL(delegate_, DispatchEvent(_)).Times(0);
+  SendDndDrop();
+  Sync();
+  Mock::VerifyAndClearExpectations(&delegate_);
+
+  // 6. Verifies that related state is correctly reset after drop.
+  EXPECT_EQ(State::kIdle, drag_controller()->state());
+  EXPECT_FALSE(window_manager()->GetCurrentPointerFocusedWindow());
+  EXPECT_EQ(gfx::kNullAcceleratedWidget,
+            screen_->GetLocalProcessWidgetAtPoint({20, 20}, {}));
+}
+
 INSTANTIATE_TEST_SUITE_P(XdgVersionStableTest,
                          WaylandWindowDragControllerTest,
                          Values(wl::ServerConfig{