From e310c53457aabfff077bdd670be1cf89aa8924ca Mon Sep 17 00:00:00 2001
From: Tom Anderson <[email protected]>
Date: Tue, 14 Jul 2026 09:32:57 -0700
Subject: [PATCH] [X11] Fix potential use-after-free in X11WSML::EndMoveLoop

EndMoveLoop() invokes OnMoveLoopEnded() on its delegate. Under some
conditions (e.g., drag-and-drop operations on Linux/X11), this callback
can synchronously destroy the owning window and the move loop instance
itself. Upon returning to EndMoveLoop(), accessing any members on a
freed 'this' causes a use-after-free (UAF) crash.

This CL introduces a WeakPtr guard immediately after the delegate
invocation to check if the loop is still alive before executing the
remaining cleanup/teardown code. An automated unit test is added to
prevent regressions.

Change-Id: Ia4ea812661a1a67313af90e994eed842d5ebc913
Fixed: 534582496
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8092749
Reviewed-by: Lei Zhang <[email protected]>
Commit-Queue: Thomas Anderson <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1661927}
---

diff --git a/ui/base/x/BUILD.gn b/ui/base/x/BUILD.gn
index 190c5c0..dac89b38 100644
--- a/ui/base/x/BUILD.gn
+++ b/ui/base/x/BUILD.gn
@@ -167,6 +167,7 @@
     "x11_cursor_loader_unittest.cc",
     "x11_desktop_window_move_client_unittest.cc",
     "x11_display_util_unittest.cc",
+    "x11_whole_screen_move_loop_unittest.cc",
   ]
   deps = [
     ":x",
@@ -176,6 +177,7 @@
     "//testing/gtest",
     "//ui/base/cursor",
     "//ui/base/cursor/mojom:cursor_type_shared",
+    "//ui/events/platform",
     "//ui/gfx/geometry",
     "//ui/gfx/x",
   ]
diff --git a/ui/base/x/x11_whole_screen_move_loop.cc b/ui/base/x/x11_whole_screen_move_loop.cc
index 71ec09e..e200895 100644
--- a/ui/base/x/x11_whole_screen_move_loop.cc
+++ b/ui/base/x/x11_whole_screen_move_loop.cc
@@ -237,7 +237,11 @@
 
   // Restore the previous dispatcher.
   nested_dispatcher_.reset();
+  base::WeakPtr<X11WholeScreenMoveLoop> alive(weak_factory_.GetWeakPtr());
   delegate_->OnMoveLoopEnded();
+  if (!alive) {
+    return;
+  }
   grab_input_window_events_.Reset();
   connection->DestroyWindow({grab_input_window_});
   grab_input_window_ = x11::Window::None;
diff --git a/ui/base/x/x11_whole_screen_move_loop_unittest.cc b/ui/base/x/x11_whole_screen_move_loop_unittest.cc
new file mode 100644
index 0000000..99d649d
--- /dev/null
+++ b/ui/base/x/x11_whole_screen_move_loop_unittest.cc
@@ -0,0 +1,82 @@
+// 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 "ui/base/x/x11_whole_screen_move_loop.h"
+
+#include <memory>
+
+#include "base/functional/bind.h"
+#include "base/functional/callback.h"
+#include "base/functional/callback_helpers.h"
+#include "base/memory/weak_ptr.h"
+#include "base/run_loop.h"
+#include "base/task/single_thread_task_runner.h"
+#include "base/test/task_environment.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/x/x11_cursor.h"
+#include "ui/events/platform/platform_event_source.h"
+#include "ui/gfx/geometry/point.h"
+
+namespace ui {
+
+namespace {
+
+class TestPlatformEventSource : public PlatformEventSource {
+ public:
+  TestPlatformEventSource() = default;
+  ~TestPlatformEventSource() override = default;
+};
+
+class TestMoveLoopDelegate : public X11MoveLoopDelegate {
+ public:
+  TestMoveLoopDelegate() = default;
+  ~TestMoveLoopDelegate() override = default;
+
+  // X11MoveLoopDelegate:
+  void OnMouseMovement(const gfx::Point& screen_point,
+                       int flags,
+                       base::TimeTicks event_time) override {}
+  void OnMouseReleased() override {}
+  void OnMoveLoopEnded() override {
+    loop_.reset();
+  }
+
+  void Init(std::unique_ptr<X11WholeScreenMoveLoop> loop) {
+    loop_ = std::move(loop);
+  }
+
+  bool has_loop() const { return !!loop_; }
+
+ private:
+  std::unique_ptr<X11WholeScreenMoveLoop> loop_;
+};
+
+}  // namespace
+
+TEST(X11WholeScreenMoveLoopTest, EndMoveLoopSurvivesSelfDeletion) {
+  base::test::SingleThreadTaskEnvironment task_environment(
+      base::test::SingleThreadTaskEnvironment::MainThreadType::UI);
+
+  TestPlatformEventSource event_source;
+
+  TestMoveLoopDelegate delegate;
+  auto move_loop = std::make_unique<X11WholeScreenMoveLoop>(&delegate);
+  X11WholeScreenMoveLoop* move_loop_ptr = move_loop.get();
+  delegate.Init(std::move(move_loop));
+
+  // Post a task to end the move loop. This task will run after the nested
+  // run loop starts, which synchronously invokes delegate->OnMoveLoopEnded()
+  // and deletes the loop.
+  base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
+      FROM_HERE, base::BindOnce(&X11WholeScreenMoveLoop::EndMoveLoop,
+                                base::Unretained(move_loop_ptr)));
+
+  bool run_result = move_loop_ptr->RunMoveLoop(
+      /*can_grab_pointer=*/false, nullptr, nullptr, base::DoNothing());
+
+  EXPECT_FALSE(delegate.has_loop());
+  EXPECT_FALSE(run_result);
+}
+
+}  // namespace ui

diff --git a/ui/base/x/x11_whole_screen_move_loop_unittest.cc b/ui/base/x/x11_whole_screen_move_loop_unittest.cc
new file mode 100644
index 0000000..99d649d
--- /dev/null
+++ b/ui/base/x/x11_whole_screen_move_loop_unittest.cc
@@ -0,0 +1,82 @@
+// 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 "ui/base/x/x11_whole_screen_move_loop.h"
+
+#include <memory>
+
+#include "base/functional/bind.h"
+#include "base/functional/callback.h"
+#include "base/functional/callback_helpers.h"
+#include "base/memory/weak_ptr.h"
+#include "base/run_loop.h"
+#include "base/task/single_thread_task_runner.h"
+#include "base/test/task_environment.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/x/x11_cursor.h"
+#include "ui/events/platform/platform_event_source.h"
+#include "ui/gfx/geometry/point.h"
+
+namespace ui {
+
+namespace {
+
+class TestPlatformEventSource : public PlatformEventSource {
+ public:
+  TestPlatformEventSource() = default;
+  ~TestPlatformEventSource() override = default;
+};
+
+class TestMoveLoopDelegate : public X11MoveLoopDelegate {
+ public:
+  TestMoveLoopDelegate() = default;
+  ~TestMoveLoopDelegate() override = default;
+
+  // X11MoveLoopDelegate:
+  void OnMouseMovement(const gfx::Point& screen_point,
+                       int flags,
+                       base::TimeTicks event_time) override {}
+  void OnMouseReleased() override {}
+  void OnMoveLoopEnded() override {
+    loop_.reset();
+  }
+
+  void Init(std::unique_ptr<X11WholeScreenMoveLoop> loop) {
+    loop_ = std::move(loop);
+  }
+
+  bool has_loop() const { return !!loop_; }
+
+ private:
+  std::unique_ptr<X11WholeScreenMoveLoop> loop_;
+};
+
+}  // namespace
+
+TEST(X11WholeScreenMoveLoopTest, EndMoveLoopSurvivesSelfDeletion) {
+  base::test::SingleThreadTaskEnvironment task_environment(
+      base::test::SingleThreadTaskEnvironment::MainThreadType::UI);
+
+  TestPlatformEventSource event_source;
+
+  TestMoveLoopDelegate delegate;
+  auto move_loop = std::make_unique<X11WholeScreenMoveLoop>(&delegate);
+  X11WholeScreenMoveLoop* move_loop_ptr = move_loop.get();
+  delegate.Init(std::move(move_loop));
+
+  // Post a task to end the move loop. This task will run after the nested
+  // run loop starts, which synchronously invokes delegate->OnMoveLoopEnded()
+  // and deletes the loop.
+  base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
+      FROM_HERE, base::BindOnce(&X11WholeScreenMoveLoop::EndMoveLoop,
+                                base::Unretained(move_loop_ptr)));
+
+  bool run_result = move_loop_ptr->RunMoveLoop(
+      /*can_grab_pointer=*/false, nullptr, nullptr, base::DoNothing());
+
+  EXPECT_FALSE(delegate.has_loop());
+  EXPECT_FALSE(run_result);
+}
+
+}  // namespace ui