From 82a5d3f85329f42da85b4cc96bf6fc76e9b9acf2 Mon Sep 17 00:00:00 2001
From: Tom Anderson <thomasanderson@chromium.org>
Date: Fri, 18 Sep 2026 10:23:28 -0700
Subject: [PATCH] [Ozone] Fix UAF when releasing capture to start a drag

DesktopDragDropClientOzone::StartDragAndDrop() releases the global
capture before starting the drag. ReleaseCapture() synchronously
dispatches a capture-changed event, runs WindowDelegate::OnCaptureLost()
and notifies capture observers, any of which may tear down the UI that
owns the drag source. That destroys this object as well as the root and
source windows, but the liveness check was only armed after the release,
so execution resumed on freed memory.

Arm the weak pointer and track both windows with an aura::WindowTracker
before releasing capture, and abort the drag if anything was destroyed.

Also fall back to a default cursor instead of dereferencing the cursor
client unconditionally, since it is null-checked a few lines earlier.

Bug: 553132165
Change-Id: I3585e82fa05aef17834013d167a6fb9edc5fbeeb
Fixed: 553132165
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8427405
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1700916}
---
diff --git a/ui/views/widget/desktop_aura/desktop_drag_drop_client_ozone.cc b/ui/views/widget/desktop_aura/desktop_drag_drop_client_ozone.cc
index 51cbdb37..c1d8b62 100644
--- a/ui/views/widget/desktop_aura/desktop_drag_drop_client_ozone.cc
+++ b/ui/views/widget/desktop_aura/desktop_drag_drop_client_ozone.cc
@@ -15,6 +15,7 @@
 #include "ui/aura/client/cursor_client.h"
 #include "ui/aura/client/drag_drop_client_observer.h"
 #include "ui/aura/window.h"
+#include "ui/aura/window_tracker.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/base/cursor/mojom/cursor_type.mojom-shared.h"
 #include "ui/base/data_transfer_policy/data_transfer_policy_controller.h"
@@ -167,11 +168,29 @@
   DCHECK(!drag_context_);
   drag_context_ = std::make_unique<DragContext>();
 
+  // This object is owned by a DesktopNativeWidgetAura that can be destroyed
+  // while this method is on the stack, which would also destroy this object
+  // and the windows passed in.  So keep track of whether everything is still
+  // alive after each call that may run nested event handlers.
+  auto alive = weak_factory_.GetWeakPtr();
+  aura::WindowTracker window_tracker({root_window, source_window});
+
   if (drag_handler_->ShouldReleaseCaptureForDrag(data.get())) {
     aura::Window* capture_window =
         aura::client::GetCaptureClient(root_window)->GetGlobalCaptureWindow();
     if (capture_window) {
+      // Releasing capture synchronously dispatches a capture-changed event and
+      // notifies capture observers, which may tear down UI, including this
+      // object and the windows involved in the drag.
       capture_window->ReleaseCapture();
+      if (!alive) {
+        return DragOperation::kNone;
+      }
+      if (!window_tracker.Contains(root_window) ||
+          !window_tracker.Contains(source_window)) {
+        drag_context_.reset();
+        return DragOperation::kNone;
+      }
     }
   }
 
@@ -194,13 +213,9 @@
     }
   }
 
-  // This object is owned by a DesktopNativeWidgetAura that can be destroyed
-  // during the drag loop, which will also destroy this object.  So keep track
-  // of whether we are still alive after the drag ends.
-  auto alive = weak_factory_.GetWeakPtr();
-
   const bool drag_succeeded = drag_handler_->StartDrag(
-      *data.get(), allowed_operations, source, cursor_client->GetCursor(),
+      *data.get(), allowed_operations, source,
+      cursor_client ? cursor_client->GetCursor() : gfx::NativeCursor(),
       !source_window->HasCapture(),
       base::BindOnce(&DesktopDragDropClientOzone::OnDragStarted,
                      weak_factory_.GetWeakPtr()),
diff --git a/ui/views/widget/desktop_aura/desktop_drag_drop_client_ozone_unittest.cc b/ui/views/widget/desktop_aura/desktop_drag_drop_client_ozone_unittest.cc
index 3e263ab8..9486759 100644
--- a/ui/views/widget/desktop_aura/desktop_drag_drop_client_ozone_unittest.cc
+++ b/ui/views/widget/desktop_aura/desktop_drag_drop_client_ozone_unittest.cc
@@ -19,6 +19,7 @@
 #include "base/task/single_thread_task_runner.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "ui/aura/client/drag_drop_delegate.h"
+#include "ui/aura/test/test_window_delegate.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/base/cursor/platform_cursor.h"
@@ -27,6 +28,7 @@
 #include "ui/base/dragdrop/drag_drop_types.h"
 #include "ui/base/dragdrop/mojom/drag_drop_types.mojom.h"
 #include "ui/base/dragdrop/os_exchange_data.h"
+#include "ui/compositor/layer_type.h"
 #include "ui/platform_window/platform_window.h"
 #include "ui/platform_window/wm/wm_drag_handler.h"
 #include "ui/platform_window/wm/wm_drop_handler.h"
@@ -291,6 +293,13 @@
     client_.reset();
   }
 
+  // Destroys the drag-drop client along with the widget that hosts the drag
+  // source, emulating the teardown of a DesktopNativeWidgetAura.
+  void DestroyDragSource() {
+    ResetClient();
+    widget_.reset();
+  }
+
   base::WeakPtr<DesktopDragDropClientOzoneTest> GetWeakPtr() {
     return weak_factory_.GetWeakPtr();
   }
@@ -644,4 +653,57 @@
   EXPECT_EQ(1, dragdrop_delegate_->num_drops());
 }
 
+namespace {
+
+// A window delegate that runs a closure when the window loses capture.
+class CaptureLostDelegate : public aura::test::TestWindowDelegate {
+ public:
+  explicit CaptureLostDelegate(base::OnceClosure on_capture_lost)
+      : on_capture_lost_(std::move(on_capture_lost)) {}
+
+  CaptureLostDelegate(const CaptureLostDelegate&) = delete;
+  CaptureLostDelegate& operator=(const CaptureLostDelegate&) = delete;
+
+  ~CaptureLostDelegate() override = default;
+
+  // aura::test::TestWindowDelegate:
+  void OnCaptureLost() override {
+    if (on_capture_lost_) {
+      std::move(on_capture_lost_).Run();
+    }
+  }
+
+ private:
+  base::OnceClosure on_capture_lost_;
+};
+
+}  // namespace
+
+// Starting a drag releases the capture, which synchronously dispatches a
+// capture-changed event.  A handler of that event may tear down the UI that
+// owns the drag source, including this client.  Verify that the drag is
+// rejected instead of using the freed objects.
+TEST_F(DesktopDragDropClientOzoneTest, CaptureReleaseDestroysDragSource) {
+  // Create a second top-level widget that holds the capture, the way an open
+  // menu or a tab drag does.
+  auto capture_widget = std::make_unique<Widget>();
+  Widget::InitParams params(Widget::InitParams::CLIENT_OWNS_WIDGET,
+                            Widget::InitParams::TYPE_WINDOW);
+  params.bounds = gfx::Rect(100, 100);
+  capture_widget->Init(std::move(params));
+  capture_widget->Show();
+
+  CaptureLostDelegate capture_lost_delegate(base::BindOnce(
+      &DesktopDragDropClientOzoneTest::DestroyDragSource, GetWeakPtr()));
+  aura::Window capture_window(&capture_lost_delegate);
+  capture_window.Init(ui::LAYER_NOT_DRAWN);
+  capture_widget->GetNativeWindow()->AddChild(&capture_window);
+  capture_window.Show();
+  capture_window.SetCapture();
+  ASSERT_TRUE(capture_window.HasCapture());
+
+  EXPECT_EQ(DragOperation::kNone,
+            StartDragAndDrop(ui::DragDropTypes::DRAG_COPY));
+}
+
 }  // namespace views

diff --git a/ui/views/widget/desktop_aura/desktop_drag_drop_client_ozone_unittest.cc b/ui/views/widget/desktop_aura/desktop_drag_drop_client_ozone_unittest.cc
index 3e263ab8..9486759 100644
--- a/ui/views/widget/desktop_aura/desktop_drag_drop_client_ozone_unittest.cc
+++ b/ui/views/widget/desktop_aura/desktop_drag_drop_client_ozone_unittest.cc
@@ -19,6 +19,7 @@
 #include "base/task/single_thread_task_runner.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "ui/aura/client/drag_drop_delegate.h"
+#include "ui/aura/test/test_window_delegate.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/base/cursor/platform_cursor.h"
@@ -27,6 +28,7 @@
 #include "ui/base/dragdrop/drag_drop_types.h"
 #include "ui/base/dragdrop/mojom/drag_drop_types.mojom.h"
 #include "ui/base/dragdrop/os_exchange_data.h"
+#include "ui/compositor/layer_type.h"
 #include "ui/platform_window/platform_window.h"
 #include "ui/platform_window/wm/wm_drag_handler.h"
 #include "ui/platform_window/wm/wm_drop_handler.h"
@@ -291,6 +293,13 @@
     client_.reset();
   }
 
+  // Destroys the drag-drop client along with the widget that hosts the drag
+  // source, emulating the teardown of a DesktopNativeWidgetAura.
+  void DestroyDragSource() {
+    ResetClient();
+    widget_.reset();
+  }
+
   base::WeakPtr<DesktopDragDropClientOzoneTest> GetWeakPtr() {
     return weak_factory_.GetWeakPtr();
   }
@@ -644,4 +653,57 @@
   EXPECT_EQ(1, dragdrop_delegate_->num_drops());
 }
 
+namespace {
+
+// A window delegate that runs a closure when the window loses capture.
+class CaptureLostDelegate : public aura::test::TestWindowDelegate {
+ public:
+  explicit CaptureLostDelegate(base::OnceClosure on_capture_lost)
+      : on_capture_lost_(std::move(on_capture_lost)) {}
+
+  CaptureLostDelegate(const CaptureLostDelegate&) = delete;
+  CaptureLostDelegate& operator=(const CaptureLostDelegate&) = delete;
+
+  ~CaptureLostDelegate() override = default;
+
+  // aura::test::TestWindowDelegate:
+  void OnCaptureLost() override {
+    if (on_capture_lost_) {
+      std::move(on_capture_lost_).Run();
+    }
+  }
+
+ private:
+  base::OnceClosure on_capture_lost_;
+};
+
+}  // namespace
+
+// Starting a drag releases the capture, which synchronously dispatches a
+// capture-changed event.  A handler of that event may tear down the UI that
+// owns the drag source, including this client.  Verify that the drag is
+// rejected instead of using the freed objects.
+TEST_F(DesktopDragDropClientOzoneTest, CaptureReleaseDestroysDragSource) {
+  // Create a second top-level widget that holds the capture, the way an open
+  // menu or a tab drag does.
+  auto capture_widget = std::make_unique<Widget>();
+  Widget::InitParams params(Widget::InitParams::CLIENT_OWNS_WIDGET,
+                            Widget::InitParams::TYPE_WINDOW);
+  params.bounds = gfx::Rect(100, 100);
+  capture_widget->Init(std::move(params));
+  capture_widget->Show();
+
+  CaptureLostDelegate capture_lost_delegate(base::BindOnce(
+      &DesktopDragDropClientOzoneTest::DestroyDragSource, GetWeakPtr()));
+  aura::Window capture_window(&capture_lost_delegate);
+  capture_window.Init(ui::LAYER_NOT_DRAWN);
+  capture_widget->GetNativeWindow()->AddChild(&capture_window);
+  capture_window.Show();
+  capture_window.SetCapture();
+  ASSERT_TRUE(capture_window.HasCapture());
+
+  EXPECT_EQ(DragOperation::kNone,
+            StartDragAndDrop(ui::DragDropTypes::DRAG_COPY));
+}
+
 }  // namespace views