From 4162d19bbe0ccd1f3211e1b4077868b25efae210 Mon Sep 17 00:00:00 2001
From: Keren Zhu <[email protected]>
Date: Wed, 26 Aug 2026 13:57:59 -0700
Subject: [PATCH] [mac] Reject drag sessions while a window move loop is active

It is currently possible for a compromised render process to start a
dragging session while the browser process is in a nested run loop (
e.g. during tab dragging). If this happens, the render process can
inject a fake mouse events using -[NSView
beginDraggingSessionWithItems:event:source:], replacing the event that
should follow the cursor.

This CL fixes the loophole by adding a global
CocoaWindowMoveLoop::IsActive() that returns true while a nested run
loop is active and rejecting the render process's dragging request if
so.

Fixed: 520504291
Change-Id: I36638d904a651a32a9edb0fb9494e722d57fc798
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8293981
Reviewed-by: Avi Drissman <[email protected]>
Commit-Queue: Keren Zhu <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1686657}
---
diff --git a/components/remote_cocoa/app_shim/window_move_loop.h b/components/remote_cocoa/app_shim/window_move_loop.h
index 4b5ab0c..9880cbc4 100644
--- a/components/remote_cocoa/app_shim/window_move_loop.h
+++ b/components/remote_cocoa/app_shim/window_move_loop.h
@@ -27,6 +27,11 @@
 
   ~CocoaWindowMoveLoop();
 
+  // Returns true if any window in this process is currently inside Run(). The
+  // move loop tracks the held mouse button, so callers that would otherwise
+  // start a dragging session should refuse to do so while this returns true.
+  static bool IsActive();
+
   // Initiates the drag until a mouse up event is observed, or End() is called.
   // Returns true if a mouse up event ended the loop.
   bool Run();
diff --git a/components/remote_cocoa/app_shim/window_move_loop.mm b/components/remote_cocoa/app_shim/window_move_loop.mm
index 8398901..61b2c90d 100644
--- a/components/remote_cocoa/app_shim/window_move_loop.mm
+++ b/components/remote_cocoa/app_shim/window_move_loop.mm
@@ -9,6 +9,7 @@
 #include <utility>
 #include <vector>
 
+#include "base/auto_reset.h"
 #include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
 #import "components/remote_cocoa/app_shim/native_widget_ns_window_bridge.h"
@@ -94,6 +95,15 @@
 
 namespace remote_cocoa {
 
+namespace {
+bool g_move_loop_active = false;
+}  // namespace
+
+// static
+bool CocoaWindowMoveLoop::IsActive() {
+  return g_move_loop_active;
+}
+
 CocoaWindowMoveLoop::CocoaWindowMoveLoop(NativeWidgetNSWindowBridge* owner,
                                          const NSPoint& initial_mouse_in_screen)
     : owner_(owner),
@@ -111,6 +121,11 @@
 }
 
 bool CocoaWindowMoveLoop::Run() {
+  CHECK(!g_move_loop_active);
+  // |this| may be deleted while the nested run loop below is spinning, so this
+  // must not refer to any member state on destruction.
+  base::AutoReset<bool> active_resetter(&g_move_loop_active, true);
+
   LoopExitReason exit_reason = ENDED_EXTERNALLY;
   exit_reason_ref_ = &exit_reason;
   NSWindow* window = owner_->ns_window();
diff --git a/content/browser/web_contents/web_contents_view_mac.mm b/content/browser/web_contents/web_contents_view_mac.mm
index 6019667f..83db25b5 100644
--- a/content/browser/web_contents/web_contents_view_mac.mm
+++ b/content/browser/web_contents/web_contents_view_mac.mm
@@ -20,6 +20,7 @@
 #include "base/task/current_thread.h"
 #include "base/task/thread_pool.h"
 #include "base/threading/thread_restrictions.h"
+#include "components/remote_cocoa/app_shim/window_move_loop.h"
 #include "components/remote_cocoa/browser/ns_view_ids.h"
 #include "components/remote_cocoa/common/application.mojom.h"
 #include "content/app_shim_remote_cocoa/web_contents_ns_view_bridge.h"
@@ -214,6 +215,12 @@
   if (drag_source_start_rwh_) {
     return;
   }
+  // A window move loop already owns the held mouse button; refuse to start a
+  // dragging session that would interfere with it.
+  if (remote_cocoa::CocoaWindowMoveLoop::IsActive()) {
+    web_contents_->SystemDragEnded(source_rwh);
+    return;
+  }
   url::Origin source_origin = source_rfh.GetLastCommittedOrigin();
   // By allowing nested tasks, the code below also allows Close(),
   // which would deallocate |this|.  The same problem can occur while
diff --git a/ui/views/cocoa/drag_drop_client_mac.mm b/ui/views/cocoa/drag_drop_client_mac.mm
index b7aa80c..2a583d1 100644
--- a/ui/views/cocoa/drag_drop_client_mac.mm
+++ b/ui/views/cocoa/drag_drop_client_mac.mm
@@ -9,6 +9,7 @@
 #include "base/strings/sys_string_conversions.h"
 #import "components/remote_cocoa/app_shim/bridged_content_view.h"
 #import "components/remote_cocoa/app_shim/native_widget_ns_window_bridge.h"
+#include "components/remote_cocoa/app_shim/window_move_loop.h"
 #include "ui/base/dragdrop/drag_drop_types.h"
 #include "ui/base/dragdrop/mojom/drag_drop_types.mojom.h"
 #import "ui/base/dragdrop/os_exchange_data_provider_mac.h"
@@ -31,6 +32,12 @@
     std::unique_ptr<ui::OSExchangeData> data,
     int operation,
     ui::mojom::DragEventSource source) {
+  // A window move loop already owns the held mouse button; starting a dragging
+  // session here would interfere with it.
+  if (remote_cocoa::CocoaWindowMoveLoop::IsActive()) {
+    return;
+  }
+
   exchange_data_ = std::move(data);
   source_operation_ = operation;
   is_drag_source_ = true;
diff --git a/ui/views/cocoa/drag_drop_client_mac_unittest.mm b/ui/views/cocoa/drag_drop_client_mac_unittest.mm
index 9c7bf093..15de877 100644
--- a/ui/views/cocoa/drag_drop_client_mac_unittest.mm
+++ b/ui/views/cocoa/drag_drop_client_mac_unittest.mm
@@ -11,6 +11,7 @@
 #include "base/memory/raw_ptr.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/single_thread_task_runner.h"
+#include "base/test/run_until.h"
 #import "components/remote_cocoa/app_shim/native_widget_ns_window_bridge.h"
 #import "ui/base/clipboard/clipboard_util_mac.h"
 #import "ui/base/dragdrop/drag_drop_types.h"
@@ -26,6 +27,10 @@
 #include "ui/views/widget/native_widget_mac.h"
 #include "ui/views/widget/widget.h"
 
+namespace {
+int g_begin_dragging_session_call_count = 0;
+}  // namespace
+
 @interface NSView (DragSessionTestingDonor)
 @end
 
@@ -34,6 +39,7 @@
                                                  event:(NSEvent*)event
                                                 source:(id<NSDraggingSource>)
                                                            source {
+  ++g_begin_dragging_session_call_count;
   return nil;
 }
 @end
@@ -292,6 +298,45 @@
   EXPECT_FALSE(ns_window_host()->IsMouseCaptureActive());
 }
 
+// A drag should not start while a window move loop is running for any window
+// in the process. The move loop already owns the held mouse button and a
+// concurrent dragging session would interfere with it.
+TEST_F(DragDropClientMacTest, DragRejectedDuringWindowMoveLoop) {
+  std::unique_ptr<OSExchangeData> data = std::make_unique<OSExchangeData>();
+  data->SetString(u"text");
+  data->provider().SetDragImage(gfx::test::CreateImageSkia(100, 100),
+                                gfx::Vector2d());
+
+  // There's no way to cleanly stop NSDraggingSession inside unit tests, so just
+  // don't start it at all.
+  g_begin_dragging_session_call_count = 0;
+  base::apple::ScopedObjCClassSwizzler swizzle(
+      [NSView class], @selector(beginDraggingSessionWithItems:event:source:),
+      @selector(cr_beginDraggingSessionWithItems:event:source:));
+
+  // Attempt to start a drag from inside the move loop and then end the move
+  // loop.
+  bool drag_attempted = false;
+  base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          [](DragDropClientMac* client,
+             remote_cocoa::NativeWidgetNSWindowBridge* bridge,
+             std::unique_ptr<OSExchangeData> data, bool* drag_attempted) {
+            client->StartDragAndDrop(std::move(data),
+                                     ui::DragDropTypes::DRAG_COPY,
+                                     ui::mojom::DragEventSource::kMouse);
+            *drag_attempted = true;
+            bridge->EndMoveLoop();
+          },
+          drag_drop_client(), bridge(), std::move(data), &drag_attempted));
+
+  bridge()->RunMoveLoop(gfx::Vector2d());
+
+  EXPECT_TRUE(base::test::RunUntil([&]() { return drag_attempted; }));
+  EXPECT_EQ(0, g_begin_dragging_session_call_count);
+}
+
 // Tests if the drag and drop target rejects the dropped data with the
 // incorrect format.
 TEST_F(DragDropClientMacTest, InvalidFormatDragDrop) {

diff --git a/ui/views/cocoa/drag_drop_client_mac_unittest.mm b/ui/views/cocoa/drag_drop_client_mac_unittest.mm
index 9c7bf093..15de877 100644
--- a/ui/views/cocoa/drag_drop_client_mac_unittest.mm
+++ b/ui/views/cocoa/drag_drop_client_mac_unittest.mm
@@ -11,6 +11,7 @@
 #include "base/memory/raw_ptr.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/single_thread_task_runner.h"
+#include "base/test/run_until.h"
 #import "components/remote_cocoa/app_shim/native_widget_ns_window_bridge.h"
 #import "ui/base/clipboard/clipboard_util_mac.h"
 #import "ui/base/dragdrop/drag_drop_types.h"
@@ -26,6 +27,10 @@
 #include "ui/views/widget/native_widget_mac.h"
 #include "ui/views/widget/widget.h"
 
+namespace {
+int g_begin_dragging_session_call_count = 0;
+}  // namespace
+
 @interface NSView (DragSessionTestingDonor)
 @end
 
@@ -34,6 +39,7 @@
                                                  event:(NSEvent*)event
                                                 source:(id<NSDraggingSource>)
                                                            source {
+  ++g_begin_dragging_session_call_count;
   return nil;
 }
 @end
@@ -292,6 +298,45 @@
   EXPECT_FALSE(ns_window_host()->IsMouseCaptureActive());
 }
 
+// A drag should not start while a window move loop is running for any window
+// in the process. The move loop already owns the held mouse button and a
+// concurrent dragging session would interfere with it.
+TEST_F(DragDropClientMacTest, DragRejectedDuringWindowMoveLoop) {
+  std::unique_ptr<OSExchangeData> data = std::make_unique<OSExchangeData>();
+  data->SetString(u"text");
+  data->provider().SetDragImage(gfx::test::CreateImageSkia(100, 100),
+                                gfx::Vector2d());
+
+  // There's no way to cleanly stop NSDraggingSession inside unit tests, so just
+  // don't start it at all.
+  g_begin_dragging_session_call_count = 0;
+  base::apple::ScopedObjCClassSwizzler swizzle(
+      [NSView class], @selector(beginDraggingSessionWithItems:event:source:),
+      @selector(cr_beginDraggingSessionWithItems:event:source:));
+
+  // Attempt to start a drag from inside the move loop and then end the move
+  // loop.
+  bool drag_attempted = false;
+  base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          [](DragDropClientMac* client,
+             remote_cocoa::NativeWidgetNSWindowBridge* bridge,
+             std::unique_ptr<OSExchangeData> data, bool* drag_attempted) {
+            client->StartDragAndDrop(std::move(data),
+                                     ui::DragDropTypes::DRAG_COPY,
+                                     ui::mojom::DragEventSource::kMouse);
+            *drag_attempted = true;
+            bridge->EndMoveLoop();
+          },
+          drag_drop_client(), bridge(), std::move(data), &drag_attempted));
+
+  bridge()->RunMoveLoop(gfx::Vector2d());
+
+  EXPECT_TRUE(base::test::RunUntil([&]() { return drag_attempted; }));
+  EXPECT_EQ(0, g_begin_dragging_session_call_count);
+}
+
 // Tests if the drag and drop target rejects the dropped data with the
 // incorrect format.
 TEST_F(DragDropClientMacTest, InvalidFormatDragDrop) {