Overview

High
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactUse after free in TabStrip
DescriptionUse after free in TabStrip
ComponentTabStrip
Sub-componentUI>Browser>TopChrome>TabStrip
Bug ClassUAF
Tracker562002095
Fix commit0baac4accd09 (chromium/src) +238/-16
CISA KEVNot listed
CreditedGoogle
Disclosed2026-10-06

Changed Functions

FunctionChangeNotes
if
chrome/browser/ui/views/tabs/dragging/tab_drag_controller.cc
modified
BindLambdaForTesting
chrome/browser/ui/views/tabs/dragging/tab_drag_controller_interactive_uitest.cc
modified
if
chrome/browser/ui/views/tabs/dragging/tab_drag_controller_interactive_uitest.cc
modified
TargetWindowFinder
chrome/browser/ui/views/tabs/dragging/tab_drag_controller_interactive_uitest.cc
modified
TestActivationDelegate
chrome/browser/ui/views/tabs/dragging/tab_drag_controller_interactive_uitest.cc
modified

Files Changed

  • chrome/browser/ui/views/tabs/dragging/tab_drag_controller.cc
  • chrome/browser/ui/views/tabs/dragging/tab_drag_controller_interactive_uitest.cc
From 0baac4accd092eb754988eb044f0f1b636e7ed4d Mon Sep 17 00:00:00 2001
From: David Pennington <dpenning@chromium.org>
Date: Tue, 29 Sep 2026 08:01:39 -0700
Subject: [PATCH] Guard against reentrant destruction during tab drag window operations

In TabDragController::DetachIntoNewBrowserAndRunMoveLoop, native window
operations such as Show(), SetBounds(), Wait(), and Activate() can
dispatch platform events that synchronously close the detached browser
window or end the drag session. Because BrowserView::Show() absorbs
window destruction internally via a WeakPtr and returns void, the caller
previously resumed execution on freed stack locals (dragged_widget,
browser) and a freed TabDragController (this).

This change:
- Adds liveness checks in DetachIntoNewBrowserAndRunMoveLoop for the
  controller, browser, widget, and attached context after every native
  window dispatch.
- Checks weak_this after Activate() in DragBrowserToNewTabStrip.
- Updates RestoreAttachedWindowForDrag to return Liveness and checks
  WeakPtrs across Restore() and SetSize().
- Guards EndDragImpl, RevertDrag, CompleteDrag, and
  MaximizeAttachedWindow with WeakPtr checks and WeakAutoReset, and
  clears tab_drag_data_ before Activate() during drag revert.
- Adds interactive UI tests verifying window destruction and drag
  cancellation during Show() and Activate().

Bug: 562002095
Change-Id: I03c934f4f8c31999afccc9f0731b6a07ef56c60d
Fixed: 562002095
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8421710
Reviewed-by: Kaan Alsan <alsan@chromium.org>
Commit-Queue: David Pennington <dpenning@chromium.org>
Auto-Submit: David Pennington <dpenning@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1707084}
---
diff --git a/chrome/browser/ui/views/tabs/dragging/tab_drag_controller.cc b/chrome/browser/ui/views/tabs/dragging/tab_drag_controller.cc
index 6e1a493..ffc222d0 100644
--- a/chrome/browser/ui/views/tabs/dragging/tab_drag_controller.cc
+++ b/chrome/browser/ui/views/tabs/dragging/tab_drag_controller.cc
@@ -1124,7 +1124,7 @@
   TRACE_EVENT1("views", "TabDragController::DragBrowserToNewTabStrip",
                "point_in_screen", point_in_screen.ToString());
 
-  base::WeakPtr<TabDragController> weak_this = weak_factory_.GetWeakPtr();
+  base::WeakPtr<TabDragController> ref = weak_factory_.GetWeakPtr();
 
   dragging_tabs_session_ = nullptr;
 
@@ -1140,7 +1140,7 @@
       attached_native_view, target_context->GetWidget()->GetNativeView(),
       ui::TransferTouchesBehavior::kDontCancel);
 
-  CHECK(weak_this);
+  CHECK(ref);
 #endif
 
   if (current_state_ == DragState::kDraggingWindow) {
@@ -1176,7 +1176,7 @@
     // control returns to RunMoveLoop().
     VLOG(1) << "EndMoveLoop in DragBrowserToNewTabStrip";
     browser_widget->EndMoveLoop();
-    CHECK(weak_this);
+    CHECK(ref);
 
     // Ideally we would always swap the tabs now, but on non-ash Windows, it
     // seems that running the move loop implicitly activates the window when
@@ -1192,7 +1192,7 @@
       // capture.
       DetachAndAttachToNewContext(ReleaseCapture::kDontReleaseCapture,
                                   target_context);
-      CHECK(weak_this);
+      CHECK(ref);
 
       // Enter kWaitingToExitRunLoop until we actually have exited the nested
       // run loop. Otherwise, we might attempt to start another nested run loop,
@@ -1201,7 +1201,12 @@
 
       // Move the tabs into position.
       StartDraggingTabsSession(false, point_in_screen);
+      // Activate() may trigger a focus loss or window event that ends the drag
+      // and destroys `this`.
       attached_context_->GetWidget()->Activate();
+      if (!ref) {
+        return Liveness::kDeleted;
+      }
     }
 
     return Liveness::kAlive;
@@ -1222,10 +1227,15 @@
   // behaviour.
   DetachAndAttachToNewContext(ReleaseCapture::kDontReleaseCapture,
                               target_context);
-  CHECK(weak_this);
+  CHECK(ref);
 
   StartDraggingTabsSession(false, point_in_screen);
+  // Activate() may trigger a focus loss or window event that ends the drag and
+  // destroys `this`.
   attached_context_->GetWidget()->Activate();
+  if (!ref) {
+    return Liveness::kDeleted;
+  }
   return Liveness::kAlive;
 }
 
@@ -1848,6 +1858,7 @@
   }
 
   const gfx::Vector2d drag_offset = CalculateWindowDragOffset();
+
 #if (!BUILDFLAG(IS_MAC))
   // Set the window origin before making it visible, to avoid flicker on
   // Windows. See https://crbug.com/394529650
@@ -1857,8 +1868,14 @@
   const gfx::Size widget_size = dragged_widget->GetSize();
 #endif
 
+  base::WeakPtr<TabDragController> ref(weak_factory_.GetWeakPtr());
   dragged_widget->SetVisibilityChangedAnimationsEnabled(false);
+  // Show() may trigger platform event handlers that close the detached browser
+  // window or end the drag session, synchronously destroying `this`.
   browser->GetWindow()->Show();
+  if (!ref) {
+    return Liveness::kDeleted;
+  }
   dragged_widget->SetVisibilityChangedAnimationsEnabled(true);
 
   // When InitialWebUI is enabled, the asynchronous loading of WebUI might cause
@@ -1878,7 +1895,8 @@
       auto suppress_data_drag =
           views::DesktopDragDropClientOzone::ScopedSuppressForWindowMove();
 #endif
-      base::WeakPtr<TabDragController> ref(weak_factory_.GetWeakPtr());
+      // Wait() runs a nested message loop that may process events closing the
+      // window or ending the drag, destroying `this`.
       waiter.Wait();
       if (!ref) {
         return Liveness::kDeleted;
@@ -1896,13 +1914,11 @@
       display::Screen::Get()->GetDisplayNearestPoint(point_in_screen).id();
 #endif
 
-  // Activate may trigger a focus loss, destroying us.
-  {
-    base::WeakPtr<TabDragController> ref(weak_factory_.GetWeakPtr());
-    browser->GetWindow()->Activate();
-    if (!ref) {
-      return Liveness::kDeleted;
-    }
+  // Activate() may trigger a focus loss or window event that ends the drag,
+  // destroying `this`.
+  browser->GetWindow()->Activate();
+  if (!ref) {
+    return Liveness::kDeleted;
   }
   return RunMoveLoop(point_in_screen, drag_offset);
 }
@@ -2037,6 +2053,7 @@
   VLOG(1) << __func__ << " type=" << static_cast<int>(type)
           << " state=" << static_cast<int>(current_state_);
 
+  base::WeakPtr<TabDragController> ref(weak_factory_.GetWeakPtr());
   DragState previous_state = current_state_;
   current_state_ = DragState::kStopped;
   attached_context_tabs_closed_tracker_.reset();
@@ -2080,8 +2097,13 @@
           }
 #endif  // defined(USE_AURA)
 
-          // Make the hidden window containing the dragged tabs visible.
+          // Make the hidden window containing the dragged tabs visible. Show()
+          // may trigger platform event handlers that synchronously close the
+          // window and destroy `this`.
           GetAttachedBrowserWidget()->Show();
+          if (!ref) {
+            return;
+          }
         }
         CompleteDrag();
       }
@@ -2093,6 +2115,12 @@
     }
   }  // else case the only tab we were dragging was deleted. Nothing to do.
 
+  // RevertDrag() or CompleteDrag() may have triggered window activation or
+  // closure events that synchronously destroyed the owning window and `this`.
+  if (!ref) {
+    return;
+  }
+
   // Clear out drag data so we don't attempt to do anything with it.
   drag_data_.tab_drag_data_.clear();
   if (current_drag_target_) {
@@ -2120,8 +2148,13 @@
   // Otherwise, the group will get emptied out as we revert all the tabs.
   MaybePauseTrackingSavedTabGroup();
 
-  base::AutoReset<bool> is_mutating_setter(&is_mutating_, true);
-  base::AutoReset<bool> is_removing_last_tab_setter(&is_moving_last_tab_, true);
+  // Activate() at the end of RevertDrag() may synchronously destroy `this`; use
+  // WeakAutoReset so unwinding stack frames do not write to a freed object.
+  base::WeakPtr<TabDragController> ref(weak_factory_.GetWeakPtr());
+  base::WeakAutoReset<TabDragController, bool> is_mutating_setter(
+      ref, &TabDragController::is_mutating_, true);
+  base::WeakAutoReset<TabDragController, bool> is_removing_last_tab_setter(
+      ref, &TabDragController::is_moving_last_tab_, true);
 
   // If we support a mixture of dragging between VT and HT, then update this
   // metric accordingly.
@@ -2195,6 +2228,9 @@
                          initial_selection_model_);
   }
 
+  // Clear tab drag data before Activate(), which may synchronously close the
+  // window and destroy its WebContents.
+  drag_data_.tab_drag_data_.clear();
   source_context_->GetWidget()->Activate();
 }
 
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 8104a878..00471f6 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
@@ -105,7 +105,9 @@
 #include "ui/views/test/widget_activation_waiter.h"
 #include "ui/views/view.h"
 #include "ui/views/view_utils.h"
+#include "ui/views/widget/any_widget_observer.h"
 #include "ui/views/widget/widget.h"
+#include "ui/views/widget/widget_activation_delegate.h"
 #include "ui/views/window/dialog_delegate.h"
 
 #if BUILDFLAG(IS_OZONE)
@@ -857,6 +859,190 @@
   EXPECT_FALSE(IsDragSessionActive(tab_strip));
 }
 
+IN_PROC_BROWSER_TEST_F(TabDragControllerTest,
+                       DetachToNewBrowserWindowDestroyedDuringShow) {
+  TabStrip* tab_strip = GetTabStripForBrowser(browser());
+  if (!tab_strip->GetWidget()->IsMoveLoopSupported()) {
+    return;
+  }
+
+  AddTabsAndResetBrowser(browser(), 1);
+  Tab* tab1 = tab_strip->tab_at(1);
+  gfx::Point tab_1_center = GetCenterInScreenCoordinates(tab1);
+
+  ui::MouseEvent press_event(ui::EventType::kMousePressed,
+                             tab1->GetLocalBounds().CenterPoint(), tab_1_center,
+                             base::TimeTicks::Now(), ui::EF_LEFT_MOUSE_BUTTON,
+                             ui::EF_LEFT_MOUSE_BUTTON);
+  tab_strip->MaybeStartDrag(tab1, press_event, tab_strip->GetSelectionModel());
+  ASSERT_TRUE(TabDragController::IsActive());
+
+  TabDragController* controller =
+      tab_strip->GetDragContext()->GetDragController();
+  ASSERT_NE(controller, nullptr);
+
+  // First drag movement starts the drag session.
+  ASSERT_EQ(controller->Drag(tab_1_center + gfx::Vector2d(20, 0)),
+            TabDragController::Liveness::kAlive);
+
+  views::Widget* source_widget = tab_strip->GetWidget();
+  views::AnyWidgetObserver observer(views::test::AnyWidgetTestPasskey{});
+  bool closed_during_show = false;
+  observer.set_shown_callback(
+      base::BindLambdaForTesting([&](views::Widget* widget) {
+        if (widget != source_widget && !closed_during_show) {
+          closed_during_show = true;
+          widget->CloseNow();
+        }
+      }));
+
+  // Drag vertically enough to detach into a new browser window.
+  EXPECT_EQ(
+      controller->Drag(tab_1_center + gfx::Vector2d(0, GetDetachY(tab_strip))),
+      TabDragController::Liveness::kDeleted);
+  EXPECT_TRUE(closed_during_show);
+  EXPECT_FALSE(TabDragController::IsActive());
+}
+
+IN_PROC_BROWSER_TEST_F(TabDragControllerTest,
+                       DetachToNewBrowserDragEndedDuringShow) {
+  TabStrip* tab_strip = GetTabStripForBrowser(browser());
+  if (!tab_strip->GetWidget()->IsMoveLoopSupported()) {
+    return;
+  }
+
+  AddTabsAndResetBrowser(browser(), 1);
+  Tab* tab1 = tab_strip->tab_at(1);
+  gfx::Point tab_1_center = GetCenterInScreenCoordinates(tab1);
+
+  ui::MouseEvent press_event(ui::EventType::kMousePressed,
+                             tab1->GetLocalBounds().CenterPoint(), tab_1_center,
+                             base::TimeTicks::Now(), ui::EF_LEFT_MOUSE_BUTTON,
+                             ui::EF_LEFT_MOUSE_BUTTON);
+  tab_strip->MaybeStartDrag(tab1, press_event, tab_strip->GetSelectionModel());
+  ASSERT_TRUE(TabDragController::IsActive());
+
+  TabDragController* controller =
+      tab_strip->GetDragContext()->GetDragController();
+  ASSERT_NE(controller, nullptr);
+
+  ASSERT_EQ(controller->Drag(tab_1_center + gfx::Vector2d(20, 0)),
+            TabDragController::Liveness::kAlive);
+
+  views::Widget* source_widget = tab_strip->GetWidget();
+  views::AnyWidgetObserver observer(views::test::AnyWidgetTestPasskey{});
+  bool ended_during_show = false;
+  observer.set_shown_callback(
+      base::BindLambdaForTesting([&](views::Widget* widget) {
+        if (widget != source_widget && !ended_during_show) {
+          ended_during_show = true;
+          controller->EndDrag(EndDragReason::kCaptureLost);
+        }
+      }));
+
+  EXPECT_EQ(
+      controller->Drag(tab_1_center + gfx::Vector2d(0, GetDetachY(tab_strip))),
+      TabDragController::Liveness::kDeleted);
+  EXPECT_TRUE(ended_during_show);
+  EXPECT_FALSE(TabDragController::IsActive());
+}
+
+IN_PROC_BROWSER_TEST_F(TabDragControllerTest,
+                       DragBrowserToNewTabStripDragEndedDuringActivate) {
+  AddTabsAndResetBrowser(browser(), 1);
+  TabStrip* tab_strip = GetTabStripForBrowser(browser());
+
+  BrowserWindowInterface* browser2 = CreateBrowser(browser()->GetProfile());
+  AddTabsAndResetBrowser(browser2, 1);
+  TabStrip* tab_strip2 = GetTabStripForBrowser(browser2);
+
+  views::Widget* source_widget = tab_strip->GetWidget();
+  views::Widget* target_widget = tab_strip2->GetWidget();
+  source_widget->Activate();
+  target_widget->Deactivate();
+
+  Tab* tab1 = tab_strip->tab_at(1);
+  gfx::Point tab_1_center = GetCenterInScreenCoordinates(tab1);
+
+  ui::MouseEvent press_event(ui::EventType::kMousePressed,
+                             tab1->GetLocalBounds().CenterPoint(), tab_1_center,
+                             base::TimeTicks::Now(), ui::EF_LEFT_MOUSE_BUTTON,
+                             ui::EF_LEFT_MOUSE_BUTTON);
+  tab_strip->MaybeStartDrag(tab1, press_event, tab_strip->GetSelectionModel());
+  ASSERT_TRUE(TabDragController::IsActive());
+
+  TabDragController* controller =
+      tab_strip->GetDragContext()->GetDragController();
+  ASSERT_NE(controller, nullptr);
+
+  ASSERT_EQ(controller->Drag(tab_1_center + gfx::Vector2d(20, 0)),
+            TabDragController::Liveness::kAlive);
+
+  class TargetWindowFinder : public WindowFinder {
+   public:
+    explicit TargetWindowFinder(gfx::NativeWindow target_window)
+        : target_window_(target_window) {}
+
+    gfx::NativeWindow GetLocalProcessWindowAtPoint(
+        const gfx::Point& screen_point,
+        const std::set<gfx::NativeWindow>& ignore) override {
+      return ignore.contains(target_window_) ? gfx::NativeWindow()
+                                             : target_window_;
+    }
+
+   private:
+    gfx::NativeWindow target_window_;
+  };
+  SetWindowFinderForTabStrip(tab_strip, std::make_unique<TargetWindowFinder>(
+                                            target_widget->GetNativeWindow()));
+
+  bool ended_during_activate = false;
+  auto end_drag_on_activate = [&]() {
+    if (!ended_during_activate) {
+      ended_during_activate = true;
+      controller->EndDrag(EndDragReason::kCancel);
+    }
+  };
+
+  class TestActivationDelegate : public views::WidgetActivationDelegate {
+   public:
+    TestActivationDelegate(views::Widget* target,
+                           base::RepeatingClosure on_activate)
+        : target_(target), on_activate_(std::move(on_activate)) {}
+
+    void MaybeActivate(views::Widget* widget, bool activate) override {
+      if (widget == target_ && activate) {
+        on_activate_.Run();
+      }
+    }
+
+    void Deactivate(views::Widget* widget) override {}
+
+    bool IsActive(const views::Widget* widget) override { return false; }
+
+   private:
+    raw_ptr<views::Widget> target_;
... (file diff truncated)
Loading diff…

Regression Test / PoC

shipped with the fix
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 8104a878..00471f6 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
@@ -105,7 +105,9 @@
 #include "ui/views/test/widget_activation_waiter.h"
 #include "ui/views/view.h"
 #include "ui/views/view_utils.h"
+#include "ui/views/widget/any_widget_observer.h"
 #include "ui/views/widget/widget.h"
+#include "ui/views/widget/widget_activation_delegate.h"
 #include "ui/views/window/dialog_delegate.h"
 
 #if BUILDFLAG(IS_OZONE)
@@ -857,6 +859,190 @@
   EXPECT_FALSE(IsDragSessionActive(tab_strip));
 }
 
+IN_PROC_BROWSER_TEST_F(TabDragControllerTest,
+                       DetachToNewBrowserWindowDestroyedDuringShow) {
+  TabStrip* tab_strip = GetTabStripForBrowser(browser());
+  if (!tab_strip->GetWidget()->IsMoveLoopSupported()) {
+    return;
+  }
+
+  AddTabsAndResetBrowser(browser(), 1);
+  Tab* tab1 = tab_strip->tab_at(1);
+  gfx::Point tab_1_center = GetCenterInScreenCoordinates(tab1);
+
+  ui::MouseEvent press_event(ui::EventType::kMousePressed,
+                             tab1->GetLocalBounds().CenterPoint(), tab_1_center,
+                             base::TimeTicks::Now(), ui::EF_LEFT_MOUSE_BUTTON,
+                             ui::EF_LEFT_MOUSE_BUTTON);
+  tab_strip->MaybeStartDrag(tab1, press_event, tab_strip->GetSelectionModel());
+  ASSERT_TRUE(TabDragController::IsActive());
+
+  TabDragController* controller =
+      tab_strip->GetDragContext()->GetDragController();
+  ASSERT_NE(controller, nullptr);
+
+  // First drag movement starts the drag session.
+  ASSERT_EQ(controller->Drag(tab_1_center + gfx::Vector2d(20, 0)),
+            TabDragController::Liveness::kAlive);
+
+  views::Widget* source_widget = tab_strip->GetWidget();
+  views::AnyWidgetObserver observer(views::test::AnyWidgetTestPasskey{});
+  bool closed_during_show = false;
+  observer.set_shown_callback(
+      base::BindLambdaForTesting([&](views::Widget* widget) {
+        if (widget != source_widget && !closed_during_show) {
+          closed_during_show = true;
+          widget->CloseNow();
+        }
+      }));
+
+  // Drag vertically enough to detach into a new browser window.
+  EXPECT_EQ(
+      controller->Drag(tab_1_center + gfx::Vector2d(0, GetDetachY(tab_strip))),
+      TabDragController::Liveness::kDeleted);
+  EXPECT_TRUE(closed_during_show);
+  EXPECT_FALSE(TabDragController::IsActive());
+}
+
+IN_PROC_BROWSER_TEST_F(TabDragControllerTest,
+                       DetachToNewBrowserDragEndedDuringShow) {
+  TabStrip* tab_strip = GetTabStripForBrowser(browser());
+  if (!tab_strip->GetWidget()->IsMoveLoopSupported()) {
+    return;
+  }
+
+  AddTabsAndResetBrowser(browser(), 1);
+  Tab* tab1 = tab_strip->tab_at(1);
+  gfx::Point tab_1_center = GetCenterInScreenCoordinates(tab1);
+
+  ui::MouseEvent press_event(ui::EventType::kMousePressed,
+                             tab1->GetLocalBounds().CenterPoint(), tab_1_center,
+                             base::TimeTicks::Now(), ui::EF_LEFT_MOUSE_BUTTON,
+                             ui::EF_LEFT_MOUSE_BUTTON);
+  tab_strip->MaybeStartDrag(tab1, press_event, tab_strip->GetSelectionModel());
+  ASSERT_TRUE(TabDragController::IsActive());
+
+  TabDragController* controller =
+      tab_strip->GetDragContext()->GetDragController();
+  ASSERT_NE(controller, nullptr);
+
+  ASSERT_EQ(controller->Drag(tab_1_center + gfx::Vector2d(20, 0)),
+            TabDragController::Liveness::kAlive);
+
+  views::Widget* source_widget = tab_strip->GetWidget();
+  views::AnyWidgetObserver observer(views::test::AnyWidgetTestPasskey{});
+  bool ended_during_show = false;
+  observer.set_shown_callback(
+      base::BindLambdaForTesting([&](views::Widget* widget) {
+        if (widget != source_widget && !ended_during_show) {
+          ended_during_show = true;
+          controller->EndDrag(EndDragReason::kCaptureLost);
+        }
+      }));
+
+  EXPECT_EQ(
+      controller->Drag(tab_1_center + gfx::Vector2d(0, GetDetachY(tab_strip))),
+      TabDragController::Liveness::kDeleted);
+  EXPECT_TRUE(ended_during_show);
+  EXPECT_FALSE(TabDragController::IsActive());
+}
+
+IN_PROC_BROWSER_TEST_F(TabDragControllerTest,
+                       DragBrowserToNewTabStripDragEndedDuringActivate) {
+  AddTabsAndResetBrowser(browser(), 1);
+  TabStrip* tab_strip = GetTabStripForBrowser(browser());
+
+  BrowserWindowInterface* browser2 = CreateBrowser(browser()->GetProfile());
+  AddTabsAndResetBrowser(browser2, 1);
+  TabStrip* tab_strip2 = GetTabStripForBrowser(browser2);
+
+  views::Widget* source_widget = tab_strip->GetWidget();
+  views::Widget* target_widget = tab_strip2->GetWidget();
+  source_widget->Activate();
+  target_widget->Deactivate();
+
+  Tab* tab1 = tab_strip->tab_at(1);
+  gfx::Point tab_1_center = GetCenterInScreenCoordinates(tab1);
+
+  ui::MouseEvent press_event(ui::EventType::kMousePressed,
+                             tab1->GetLocalBounds().CenterPoint(), tab_1_center,
+                             base::TimeTicks::Now(), ui::EF_LEFT_MOUSE_BUTTON,
+                             ui::EF_LEFT_MOUSE_BUTTON);
+  tab_strip->MaybeStartDrag(tab1, press_event, tab_strip->GetSelectionModel());
+  ASSERT_TRUE(TabDragController::IsActive());
+
+  TabDragController* controller =
+      tab_strip->GetDragContext()->GetDragController();
+  ASSERT_NE(controller, nullptr);
+
+  ASSERT_EQ(controller->Drag(tab_1_center + gfx::Vector2d(20, 0)),
+            TabDragController::Liveness::kAlive);
+
+  class TargetWindowFinder : public WindowFinder {
+   public:
+    explicit TargetWindowFinder(gfx::NativeWindow target_window)
+        : target_window_(target_window) {}
+
+    gfx::NativeWindow GetLocalProcessWindowAtPoint(
+        const gfx::Point& screen_point,
+        const std::set<gfx::NativeWindow>& ignore) override {
+      return ignore.contains(target_window_) ? gfx::NativeWindow()
+                                             : target_window_;
+    }
+
+   private:
+    gfx::NativeWindow target_window_;
+  };
+  SetWindowFinderForTabStrip(tab_strip, std::make_unique<TargetWindowFinder>(
+                                            target_widget->GetNativeWindow()));
+
+  bool ended_during_activate = false;
+  auto end_drag_on_activate = [&]() {
+    if (!ended_during_activate) {
+      ended_during_activate = true;
+      controller->EndDrag(EndDragReason::kCancel);
+    }
+  };
+
+  class TestActivationDelegate : public views::WidgetActivationDelegate {
+   public:
+    TestActivationDelegate(views::Widget* target,
+                           base::RepeatingClosure on_activate)
+        : target_(target), on_activate_(std::move(on_activate)) {}
+
+    void MaybeActivate(views::Widget* widget, bool activate) override {
+      if (widget == target_ && activate) {
+        on_activate_.Run();
+      }
+    }
+
+    void Deactivate(views::Widget* widget) override {}
+
+    bool IsActive(const views::Widget* widget) override { return false; }
+
+   private:
+    raw_ptr<views::Widget> target_;
+    base::RepeatingClosure on_activate_;
+  };
+
+  TestActivationDelegate activation_delegate(
+      target_widget, base::BindLambdaForTesting(end_drag_on_activate));
+
+  views::AnyWidgetObserver observer(views::test::AnyWidgetTestPasskey{});
+  observer.set_activated_callback(
+      base::BindLambdaForTesting([&](views::Widget* widget) {
+        if (widget == target_widget) {
+          end_drag_on_activate();
+        }
+      }));
+
+  gfx::Point target_point = GetCenterInScreenCoordinates(tab_strip2->tab_at(0));
+  EXPECT_EQ(controller->Drag(target_point),
+            TabDragController::Liveness::kDeleted);
+  EXPECT_TRUE(ended_during_activate);
+  EXPECT_FALSE(TabDragController::IsActive());
+}
+
 class DetachToBrowserTabDragControllerTest
     : public TabDragControllerInteractiveTestMixin<TabDragControllerTest>,
       public ::testing::WithParamInterface<testing::tuple<bool, const char*>> {
Loading diff…

Original Bug Report

reported by vm...@google.com

UAF in TabDragController::DetachIntoNewBrowserAndRunMoveLoop after Show()

Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.

Overview: In TabDragController::DetachIntoNewBrowserAndRunMoveLoop (the tab tear-off path), the first Show() of the newly created dragged browser window is deletion-capable but has no liveness guard. If a platform event handler during the nested Show() dispatch destroys the new window or ends the drag, BrowserView::Show()’s internal weak-ptr guard silently swallows the destruction, and the caller resumes on freed objects: the bare stack locals dragged_widget and browser, and the freed TabDragController (this). The result is a use-after-free in the unsandboxed browser process — including virtual dispatch through freed memory — while the dragged tab renders attacker-controlled web content. Both destruction variants were reproduced with ASan on Linux/X11 at the exact sites below.

Affected files:

Estimated timestamp from git blame: 2012-01-09

Summary

During a tab tear-off drag, DetachIntoNewBrowserAndRunMoveLoop creates a hidden browser for the dragged tabs, captures bare stack pointers to it (tab_drag_controller.cc:1713-1718), and moves ownership of the drag controller into the new browser’s tab strip (:1760 → TabDragContext::OwnDragController). It then performs the window’s first Show() at :1786 with no liveness check for this, browser, or dragged_widget.

That Show() is a nested platform dispatch (first native show + activation, since views::Widget::Show() shows with WindowShowState::kNormal, ui/views/widget/widget.cc:1113-1143). The tree already treats destruction inside this dispatch as a real, crash-driven hazard three times over:

When destruction lands inside the Show() dispatch, execution resumes at :1787 on freed memory. ASan runs confirm both variants, with MiraclePtr Status: NOT PROTECTED (bare stack locals and this are not BackupRefPtr-protected).


Root Cause Analysis

  1. Bare locals captured for the new browser: tab_drag_controller.cc:1713-1718 — no weak references, no re-derivation after the hazard:
    // tab_drag_controller.cc:1713-1718
    BrowserWindowInterface* browser =
        CreateBrowserForDrag(attached_context_, new_size);
    BrowserView* const dragged_browser_view =
        BrowserView::GetBrowserViewForBrowser(browser);
    views::Widget* const dragged_widget = dragged_browser_view->GetWidget();
    
  2. this becomes owned by the new hidden browser: tab_drag_controller.cc:1760 — DetachAndAttachToNewContext ends in attached_context_->OwnDragController(std::move(controller)); the new TabStrip holds the controller in a unique_ptr and DestroyDragController() is a synchronous reset (chrome/browser/ui/views/tabs/tab_strip.cc:447-461).
  3. Unguarded deletion-capable first Show(): tab_drag_controller.cc:1785-1787 —
    // tab_drag_controller.cc:1785-1787
    dragged_widget->SetVisibilityChangedAnimationsEnabled(false);
    browser->GetWindow()->Show();                                 // <-- deletion-capable, no guard
    dragged_widget->SetVisibilityChangedAnimationsEnabled(true);  // <-- freed-Widget deref
    
    BrowserView::Show() absorbs any destruction via its weak-ptr guard and returns normally (browser_view.cc:1638-1643).
  4. Variant A — window destroyed during Show(): all pointers freed on the same stack: BrowserWidget::OnNativeWidgetDestroyed() destroys the Browser synchronously (chrome/browser/ui/views/frame/browser_widget.cc:394-407) → BrowserManagerService::SynchronouslyDestroyBrowser → browser.reset() (chrome/browser/ui/browser_manager_service.cc:32-40,111-145) → ~Browser → BrowserView::DeleteBrowserWindow() resets the unique_ptr<BrowserWidget> (browser_view.cc:4239-4251) → view-tree teardown → TabStrip::~TabStrip() → DestroyDragController() (tab_strip.cc:1176-1184). browser, dragged_browser_view, dragged_widget, and this are all freed before :1787 executes.
  5. Variant B — drag ended during Show(): this freed: capture loss dispatches EndDrag(kCaptureLost) (chrome/browser/ui/views/tabs/common/tab_drag_handler.cc:658-660). The capture-lost ignore-cases (tab_drag_controller.cc:804-816) cover only kDraggingWindow/kDraggingUsingSystemDnD; at :1786 the state is still kDraggingTabs, so EndDragImpl runs to owning_context->DestroyDragController() (:2028-2030) — synchronous delete this under the Show() dispatch.
  6. Post-Show dereferences on freed memory: each is an independent primitive:
    • :1787 — Widget::SetVisibilityChangedAnimationsEnabled reads native_widget_ from the freed Widget and virtual-dispatches through it (ui/views/widget/widget.cc:932-936).
    • :1795-1797 (when features::kInitialWebUI is enabled — off by default but an active rollout candidate in testing/variations/fieldtrial_testing_config.json) — freed-Widget read, write-after-free of current_state_ through freed this, and VisibilityWaiter registering an observer into the freed Widget’s observer list.
    • :1826 — weak_factory_.GetWeakPtr() on freed this AddRef-writes through the freed factory’s flag pointer.
    • :1827 — browser->GetWindow() is pure virtual (chrome/browser/ui/browser_window/public/browser_window_interface.h:80): an indirect call through a vtable pointer loaded from the freed Browser.

Reproduction

  1. On Windows, macOS, Linux/X11, or ChromeOS (Wayland exits earlier to system-DnD via ShouldDragWindowUsingSystemDnD, tab_drag_controller.cc:1762-1768), open a window with two tabs, one rendering attacker-controlled web content.
  2. Drag that tab out of the tab strip. ContinueDragging → DragBrowserToNewTabStrip(nullptr) (:1064-1075) → DetachIntoNewBrowserAndRunMoveLoop reaches the first Show() at :1786.
  3. Within the nested platform dispatch of that Show() (Win32 ::ShowWindow sent-message cascade, AppKit orderFront callbacks, X11/WM churn), the new native window is destroyed by the WM, or mouse capture is lost and ends the drag.
  4. Expected: the caller detects the destruction (as web_app_launch_utils.cc:388-394 does) and returns Liveness::kDeleted.

Observed (Linux/X11 ASan build, a real xdotool-synthesized tab drag, with the destruction event delivered deterministically from inside the first-Show dispatch — i.e. the timing condition supplied synthetically, the vulnerable function unmodified):

  • Window-destroy variant: heap-use-after-free READ of size 8 at tab_drag_controller.cc:1787 on the freed 960-byte BrowserWidget allocation (allocated by CreateBrowserForDrag, freed under SynchronouslyDestroyBrowser while Show() at :1786 is on the stack).
  • End-drag variant: heap-use-after-free WRITE of size 4 at tab_drag_controller.cc:1796 (current_state_ write through freed this; freed by EndDragImpl at :2030). With kInitialWebUI off, the same freed-this access lands at :1826 instead.
  • Both reports: MiraclePtr Status: NOT PROTECTED.

On an uninstrumented build the destruction event is a timing-sensitive WM/activation race rather than deterministic; an attacker popup generating window-manager churn during the drag can widen the window, and the dragged tab’s renderer gives the attacker allocation control for grooming the freed slots.

Severity

high — memory corruption (UAF with two virtual-dispatch-through-freed-memory sites at :1787/:1827 and refcount/state writes through freed this at :1796/:1826) in the unsandboxed browser process, matching the high tier for browser-process memory corruption requiring unusual user interaction (a routine tab tear-off drag). Not critical because it additionally needs a timing-sensitive destruction event inside the first-Show dispatch; not medium because that is a single timing condition, not multiple independent ones. No MiraclePtr/BackupRefPtr protection applies (confirmed by ASan), and no release-build CHECK interposes.

Suggested Fix Direction

Apply the in-tree pattern from web_app_launch_utils.cc:388-394 and from the Activate() guard at :1824-1831: capture weak_factory_.GetWeakPtr() and browser->GetWeakPtr() before :1785, return Liveness::kDeleted if the controller died during Show(), and re-derive the widget from the live browser afterwards — this guard was experimentally confirmed to eliminate both variants. The unguarded GetAttachedBrowserWidget()->Show() before CompleteDrag() in EndDragImpl (tab_drag_controller.cc:2008-2011) deserves the same treatment.


POC: Browser-process UAF in TabDragController::DetachIntoNewBrowserAndRunMoveLoop — unguarded first Show() of the dragged browser

Strategy: code-injection Bug Type: heap-use-after-free (browser process, TabDragKind tab tear-off path)

TabDragController::DetachIntoNewBrowserAndRunMoveLoop (chrome/browser/ui/views/tabs/dragging/tab_drag_controller.cc) creates the new browser for a tab tear-off, captures bare stack locals (browser, dragged_browser_view, dragged_widget, lines 1713–1718), then performs the first Show() of that window (line 1786) with no liveness guard for the locals or this, and immediately dereferences all of them (lines 1787 / 1796 / 1826 / 1827). BrowserView::Show() itself carries a crash-driven weak-ptr guard (“Guard against destruction by platform event handlers called during Show()”, browser_view.cc:1638-1643) that silently swallows any such destruction and returns void — so the tab-drag caller resumes on freed objects.

This POC performs a real tab tear-off drag (synthesized X11 mouse input against the stock UI path: Tab::OnMousePressed → TabDragContextImpl:: OnMouseDragged → Drag → ContinueDragging → DragBrowserToNewTabStrip → DetachIntoNewBrowserAndRunMoveLoop) and deterministically supplies the timing-sensitive platform event of the report: a small env-gated hook (chromium-changes.patch) inside the already-guarded region of BrowserView::Show() simulates, during the first Show() dispatch of the dragged window, either of the two real-world destruction variants:

  • end-drag — the native mouse grab is revoked (another client / WM activation churn). Delivered through the genuine production plumbing: Widget::OnMouseCaptureLost → RootView → TabDragContextImpl → TabDragController::EndDrag(kCaptureLost) → EndDragImpl → DestroyDragController → the TabDragController is deleted.
  • destroy-window — the window manager destroys the new native window: Widget::CloseNow → DesktopWindowTreeHostPlatform::OnClosed → DesktopNativeWidgetAura::OnHostClosed → BrowserManagerService::SynchronouslyDestroyBrowser → the Browser, BrowserView, BrowserWidget and TabDragController are all freed on the same stack.

The vulnerable function is not modified; no security check is bypassed or disabled. The injection only supplies the timing condition (a platform event landing inside the first-Show dispatch) that is otherwise a race against window-manager/activation events — the same premise already conceded in-tree by the BrowserView::Show() guard, by the guarded Activate() two lines below the vulnerable Show(), and by the equivalent WeakPtr-before-Show fix in web_app_launch_utils.cc:388-394.

Results (both variants reproduce at the exact claimed sites)

Mode ASAN report Site
destroy-window READ of size 8 on freed views::Widget (960-byte region) tab_drag_controller.cc:1787 — dragged_widget->SetVisibilityChangedAnimationsEnabled(true) reads the freed BrowserWidget on the way to a virtual dispatch through it
end-drag WRITE of size 4 on freed TabDragController (624-byte region) tab_drag_controller.cc:1796 — current_state_ = DragState::kWaitingForWindowToShow write-after-free through freed this

Both reports show MiraclePtr Status: NOT PROTECTED — the dereferencing pointers are bare stack locals / this, so BackupRefPtr quarantine does not apply.

Notes:

  • The end-drag run lands on the kInitialWebUI branch (line 1795–1796) because this build applies the field-trial testing config, where InitialWebUI is enabled (testing/variations/fieldtrial_testing_config.json — an active rollout candidate). With the feature off, the same freed-this deref lands a few lines later at line 1826 (weak_factory_.GetWeakPtr() constructing a WeakPtr from the freed factory, then AddRef-writing through its flag pointer).
  • In destroy-window, the freed region’s allocation stack is CreateBrowserForDrag (tab_drag_controller.cc:2790) called from DetachIntoNewBrowserAndRunMoveLoop (:1714) — i.e. exactly the dragged_widget local captured before the Show().
  • The freed derefs feed virtual dispatches (Widget::SetVisibilityChangedAnimationsEnabled → native_widget_ vtable call; browser->GetWindow() is pure virtual) and a refcount write, in the unsandboxed browser process, while the dragged tab renders attacker-controlled web content (renderer-side allocation control for grooming the freed slots).

Prerequisites

  • Linux ASAN build of chrome (out/asan, X11/ozone) with chromium-changes.patch applied.
  • An X display, xdotool, python3.

Reproduction

ART=$(pwd)                                # directory containing these files
cd /chromium_src/src
git apply "$ART/chromium-changes.patch"   # adds the env-gated event simulation
autoninja -C out/asan chrome

# Variant A: WM destroys the dragged window during its first Show()
"$ART/run_poc.sh" destroy-window          # -> asan-destroy-window.log

# Variant B: capture loss ends the drag during the first Show()
"$ART/run_poc.sh" end-drag                # -> asan-end-drag.log

run_poc.sh starts server.py (serves attacker.html on localhost:8080), launches chrome headful with a fresh profile and two tabs, then uses xdotool to press on tab 2 and drag it below the tab strip, triggering the detach. The injected hook only fires for the browser window created by the drag (TabDragKind::kAllTabs) and only when TABDRAG_POC_MODE is set.

Expected Result

An AddressSanitizer heap-use-after-free in TabDragController::DetachIntoNewBrowserAndRunMoveLoop, with the freed-by stack rooted in the unguarded browser->GetWindow()->Show() at tab_drag_controller.cc:1786.

Output

Variant A — destroy-window (full log: asan-destroy-window.log)

==2983==ERROR: AddressSanitizer: heap-use-after-free on address 0x6e5d22854860
READ of size 8 at 0x6e5d22854860 thread T0 (chrome)
    #0 base::internal::WeakReference::IsValid() const base/memory/scoped_refptr.h:319:43
    #1 views::Widget::SetVisibilityChangedAnimationsEnabled(bool) base/memory/weak_ptr.h:238:32
    #2 TabDragController::DetachIntoNewBrowserAndRunMoveLoop(gfx::Point) chrome/browser/ui/views/tabs/dragging/tab_drag_controller.cc:1787:19
    #3 TabDragController::DragBrowserToNewTabStrip(TabDragContext*, gfx::Point const&) tab_drag_controller.cc:1075:12
    #4 TabDragController::ContinueDragging(gfx::Point const&) tab_drag_controller.cc:1024:12
    #5 TabDragController::Drag(gfx::Point const&) tab_drag_controller.cc:772:10
    #6 TabStrip::TabDragContextImpl::OnMouseDragged(ui::MouseEvent const&) tab_strip.cc:371:27
    ... (real X11 mouse-drag dispatch: X11Window::DispatchUiEvent -> ... -> RootView::OnMouseDragged)

0x6e5d22854860 is located 224 bytes inside of 960-byte region
freed by thread T0 (chrome) here:
    #1 BrowserView::DeleteBrowserWindow()            (browser_widget_.reset())
    #2 Browser::~Browser()
    #4 BrowserManagerService::DeleteBrowser(BrowserWindowInterface*)
    #5 BrowserManagerService::SynchronouslyDestroyBrowser(BrowserWindowInterface*) browser_manager_service.cc:39:9
    #6 views::DesktopNativeWidgetAura::OnHostClosed() desktop_native_widget_aura.cc:404:30
    #7 views::DesktopWindowTreeHostPlatform::OnClosed() desktop_window_tree_host_platform.cc:1052:32
    #8 views::DesktopWindowTreeHostPlatform::CloseNow() desktop_window_tree_host_platform.cc:452:22
    #9 views::Widget::CloseNow() widget.cc:1105:21
    #10 BrowserView::Show() browser_view.cc:1699:24   [simulated WM destroy inside Show()]
    #11 TabDragController::DetachIntoNewBrowserAndRunMoveLoop(gfx::Point) tab_drag_controller.cc:1786:25
    ...

previously allocated by thread T0 (chrome) here:
    #1 BrowserWindow::CreateBrowserWindow(BrowserWindowInterface*, bool, bool)
    #2 Browser::Browser(BrowserWindowCreateParams) browser.cc:315:13
    #5 TabDragController::CreateBrowserForDrag(TabDragContext*, gfx::Size) tab_drag_controller.cc:2790:7
    #6 TabDragController::DetachIntoNewBrowserAndRunMoveLoop(gfx::Point) tab_drag_controller.cc:1714:7
    ...
MiraclePtr Status: NOT PROTECTED

Variant B — end-drag (full log: asan-end-drag.log)

==2660==ERROR: AddressSanitizer: heap-use-after-free on address 0x76c79c0501a8
WRITE of size 4 at 0x76c79c0501a8 thread T0 (chrome)
    #0 TabDragController::DetachIntoNewBrowserAndRunMoveLoop(gfx::Point) chrome/browser/ui/views/tabs/dragging/tab_drag_controller.cc:1796:22
    #1 TabDragController::DragBrowserToNewTabStrip(TabDragContext*, gfx::Point const&) tab_drag_controller.cc:1075:12
    #2 TabDragController::ContinueDragging(gfx::Point const&) tab_drag_controller.cc:1024:12
    #3 TabDragController::Drag(gfx::Point const&) tab_drag_controller.cc:772:10
    #4 TabStrip::TabDragContextImpl::OnMouseDragged(ui::MouseEvent const&) tab_strip.cc:371:27
    ...

0x76c79c0501a8 is located 40 bytes inside of 624-byte region
freed by thread T0 (chrome) here:
    #1 TabDragController::EndDragImpl(TabDragController::EndDragType) tab_drag_controller.cc:2030:19
    #2 TabDragController::EndDrag(EndDragReason)
    #3 BrowserView::Show() browser_view.cc:1686:30    [simulated capture loss inside Show()]
    #4 TabDragController::DetachIntoNewBrowserAndRunMoveLoop(gfx::Point) tab_drag_controller.cc:1786:25
    ...

previously allocated by thread T0 (chrome) here:
    #1 TabStrip::TabDragContextImpl::MaybeStartDrag(...)
    #3 Tab::OnMousePressed(ui::MouseEvent const&) tab.cc:749:20
    ...
MiraclePtr Status: NOT PROTECTED

SUMMARY: AddressSanitizer: heap-use-after-free tab_drag_controller.cc:1796:22 in TabDragController::DetachIntoNewBrowserAndRunMoveLoop

Bandaid Fix

bandaid-fix.patch applies the same pattern the tree already uses for the Activate() call two lines below (and for the identical fixed sibling in web_app_launch_utils.cc): capture base::WeakPtr<TabDragController> and browser->GetWeakPtr() before the Show(), and return Liveness::kDeleted if the controller died inside it:

  {
    base::WeakPtr<TabDragController> self(weak_factory_.GetWeakPtr());
    base::WeakPtr<BrowserWindowInterface> weak_browser(browser->GetWeakPtr());
    browser->GetWindow()->Show();
    if (!self) {
      return Liveness::kDeleted;
    }
    CHECK(weak_browser);
  }

Verified: with the fix applied (on top of the POC injection), both variants were re-run; the injected destruction still fires inside Show() ([TABDRAG-POC] ... markers in fix-verified-end-drag.log / fix-verified-destroy-window.log) and no ASAN report is produced — the function returns Liveness::kDeleted cleanly, matching the existing caller contract.

Files

  • attacker.html, server.py — attacker-controlled page in the dragged tab
  • run_poc.sh — end-to-end driver (chrome launch + xdotool drag)
  • chromium-changes.patch — env-gated platform-event simulation in BrowserView::Show() (the only source change used to trigger the bug)
  • asan-end-drag.log, asan-destroy-window.log — full ASAN UAF reports
  • bandaid-fix.patch — guard fix in tab_drag_controller.cc
  • fix-verified-end-drag.log, fix-verified-destroy-window.log — runs with the fix applied: injection fires, no ASAN report

Results so far have been promising, but there can be wrong deductions. Feel free to adjust as follows:

  • If you are familiar with the severity guidelines, you may adjust the severity.
  • If this is a false positive, and there’s no work to be done, please close as WAI.
  • If there is work to do here but not a vulnerability, please change the issue type to Task/Bug/FR.

Data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.

View on issue tracker