CVE-2026-19557
Overview
Background
- `DeletionDialogController`
- A per-browser controller, owned through
BrowserWindowFeaturesaccording to the commit message, that shows the confirmation dialog for deleting a tab group and observes theTabStripModel. - `chrome::ShowBrowserModal`
- Shows a
ui::DialogModelas a dialog that is modal to the browser window. The commit says that on some platforms (macOS) it can spin a nested run loop before it returns. - `ui::DialogModel` callbacks
- The OK, cancel, close-action and dialog-destroying callbacks are stored in the dialog model, so they can run after the object that registered them is gone.
- `base::Unretained` vs `base::WeakPtr`
base::Unretained(this)binds a raw pointer that has no lifetime tracking, while aWeakPtrbinding turns into a no-op once its factory is destroyed.
Root Cause Analysis
DeletionDialogController bound its dialog-show function (show_dialog_model_fn_) and every dialog-model callback (OnDialogOk, OnDialogCancel, and the close-action and dialog-destroying lambdas) with base::Unretained(this). This assumed the controller would outlive both the synchronous show and the dialog model. Both assumptions could fail. In CreateDialogFromBrowser, chrome::ShowBrowserModal can run a nested run loop, during which the owning browser, and with it this controller, can be destroyed, and the code then wrote the returned pointer into widget_ on the freed object. Separately, if the controller was destroyed while the dialog model was still alive, tearing down the dialog later ran the destroying/close callbacks, which wrote widget_ and reset state_ on freed memory. The regression test DestroyControllerBeforeDialogModel reproduces this second case.
The fix adds a base::WeakPtrFactory as the last member and rebinds every callback through weak_ptr_factory_.GetWeakPtr(). It also captures a weak pointer before ShowBrowserModal and writes widget_ only if that weak pointer is still valid afterwards.
this pointers to a dialog and a modal-show call that could outlive it, or destroy it re-entrantly. Switching every binding to WeakPtr and re-checking it after the synchronous show turns those late calls into no-ops instead of writes to freed memory.Attack Path
- Trigger the tab-group deletion dialog
The user starts an action that makes
DeletionDialogController::MaybeShowDialogshow a confirmation dialog throughshow_dialog_model_fn_/CreateDialogFromBrowser. - Controller destroyed while the dialog is pending
During the nested run loop inside
chrome::ShowBrowserModal(macOS, per the commit), or while the dialog is still alive afterwards, the owning browser window is torn down, which destroys the controller. - Late write to the freed controller
Before the fix, either the post-show
widget_ = ...assignment inCreateDialogFromBrowseror a later dialog-model callback (close action, dialog destroying, OK, or cancel) wrotewidget_/state_through a danglingUnretainedpointer. - Heap corruption The result is a use-after-free write in the browser process. Turning it into a controlled primitive by reclaiming the freed allocation is (inferred) and not shown by the patch.
Impact Assessment
ShowBrowserModal spins a nested run loop (macOS), which makes it a timing/lifetime race rather than something ordinary web content can directly drive. The advisory describes the issue only as “Use after free in TabStrip”. The vendor advisory rates this high severity and does not report in-the-wild exploitation; NVD scores it CVSS 8.3. The provided metadata gives no severity or in-the-wild status.Changed Functions
| Function | Change | Notes |
|---|---|---|
ifchrome/browser/ui/tabs/tab_group_deletion_dialog_controller.cc |
modified | |
TEST_Fchrome/browser/ui/tabs/tab_group_deletion_dialog_controller_unittest.cc |
modified |
Files Changed
chrome/browser/ui/tabs/tab_group_deletion_dialog_controller_unittest.ccchrome/browser/ui/tabs/tab_group_deletion_dialog_controller.hchrome/browser/ui/tabs/tab_group_deletion_dialog_controller.cc
Audit Directions
- `base::Unretained(this)` passed to `ui::DialogModel`Search UI controllers for
SetCloseActionCallback,SetDialogDestroyingCallback,AddOkButtonandAddCancelButtonbound withUnretained, where the dialog can outlive its controller (browser-scoped features, tab-scoped helpers). - Member writes after a call that can re-enterLook for patterns like
member_ = ShowBrowserModal(...),ShowWebModal,RunMenuAtor other calls that may spin nested run loops, wherethisis written after the call returns without a weak-pointer or destruction check. - `BrowserWindowFeatures`-owned controllers holding callbacksCheck other per-browser features that store
RepeatingCallbacks bound tothisand hand them to longer-lived UI objects, because browser teardown order is often not what the binding assumes.
Patch
From d2299d0dd4583ae879045d7d362dfb5eb64e0fd4 Mon Sep 17 00:00:00 2001 From: David Pennington <[email protected]> Date: Thu, 30 Jul 2026 11:36:57 -0700 Subject: [PATCH] Protect against synchronous browser deletion for TGDDialog Showing a browser-modal dialog can spin a nested run loop on some platforms, during which the owning browser (and thus the DeletionDialogController held by BrowserWindowFeatures) may be torn down. Resolve this by using weak ptrs and checking after the synchronous show, Fixed: 534867485 Change-Id: Ib4930e288d478a88aa9addc8101755d4e67609c4 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8170480 Commit-Queue: David Pennington <[email protected]> Reviewed-by: Darryl James <[email protected]> Cr-Commit-Position: refs/heads/main@{#1671274} --- diff --git a/chrome/browser/ui/tabs/tab_group_deletion_dialog_controller.cc b/chrome/browser/ui/tabs/tab_group_deletion_dialog_controller.cc index 96abe862..e429456 100644 --- a/chrome/browser/ui/tabs/tab_group_deletion_dialog_controller.cc +++ b/chrome/browser/ui/tabs/tab_group_deletion_dialog_controller.cc @@ -352,11 +352,10 @@ Profile* profile, TabStripModel* tab_strip_model) : profile_(CHECK_DEREF(profile)), - show_dialog_model_fn_(base::BindRepeating( - &DeletionDialogController::CreateDialogFromBrowser, - base::Unretained(this), - browser)), tab_strip_model_(CHECK_DEREF(tab_strip_model)) { + show_dialog_model_fn_ = + base::BindRepeating(&DeletionDialogController::CreateDialogFromBrowser, + weak_ptr_factory_.GetWeakPtr(), browser); tab_strip_model_->AddObserver(this); } @@ -382,7 +381,16 @@ void DeletionDialogController::CreateDialogFromBrowser( BrowserWindowInterface* browser, std::unique_ptr<ui::DialogModel> dialog_model) { - widget_ = chrome::ShowBrowserModal(browser, std::move(dialog_model)); + // Showing a modal dialog can spin a nested run loop on macOS, during which + // the owning browser and controller may be torn down. Guard the post-show + // write with a weak pointer to avoid a use-after-free write if `this` is + // destroyed during ShowBrowserModal. + auto weak_ptr = weak_ptr_factory_.GetWeakPtr(); + views::Widget* widget = + chrome::ShowBrowserModal(browser, std::move(dialog_model)); + if (weak_ptr) { + weak_ptr->widget_ = widget; + } } bool DeletionDialogController::MaybeShowDialog( @@ -461,6 +469,15 @@ state_.reset(); } +void DeletionDialogController::OnCloseAction() { + state_.reset(); +} + +void DeletionDialogController::OnDialogDestroying() { + widget_ = nullptr; + state_.reset(); +} + std::unique_ptr<ui::DialogModel> DeletionDialogController::BuildDialogModel( const DialogMetadata& metadata) { DialogText strings = GetDialogText(GetProfile(), metadata); @@ -475,25 +492,20 @@ dialog_builder.SetTitle(strings.title) .AddParagraph(ui::DialogModelLabel(strings.body)) .AddCancelButton(base::BindOnce(&DeletionDialogController::OnDialogCancel, - base::Unretained(this)), + weak_ptr_factory_.GetWeakPtr()), cancel_button_params) .AddOkButton(base::BindOnce(&DeletionDialogController::OnDialogOk, - base::Unretained(this)), + weak_ptr_factory_.GetWeakPtr()), ui::DialogModel::Button::Params() .SetLabel(strings.ok_text) .SetEnabled(true) .SetId(kDeletionDialogOkButtonId)) - .SetCloseActionCallback(base::BindOnce( - [](DeletionDialogController* dialog_controller) { - dialog_controller->state_.reset(); - }, - base::Unretained(this))) - .SetDialogDestroyingCallback(base::BindOnce( - [](DeletionDialogController* dialog_controller) { - dialog_controller->widget_ = nullptr; - dialog_controller->state_.reset(); - }, - base::Unretained(this))); + .SetCloseActionCallback( + base::BindOnce(&DeletionDialogController::OnCloseAction, + weak_ptr_factory_.GetWeakPtr())) + .SetDialogDestroyingCallback( + base::BindOnce(&DeletionDialogController::OnDialogDestroying, + weak_ptr_factory_.GetWeakPtr())); if (IsDialogSkippable(metadata.type)) { dialog_builder.AddCheckbox( kDeletionDialogDontAskCheckboxId, diff --git a/chrome/browser/ui/tabs/tab_group_deletion_dialog_controller.h b/chrome/browser/ui/tabs/tab_group_deletion_dialog_controller.h index 911999278..6c33c76a 100644 --- a/chrome/browser/ui/tabs/tab_group_deletion_dialog_controller.h +++ b/chrome/browser/ui/tabs/tab_group_deletion_dialog_controller.h @@ -11,6 +11,7 @@ #include "base/functional/callback.h" #include "base/memory/raw_ptr.h" #include "base/memory/raw_ref.h" +#include "base/memory/weak_ptr.h" #include "chrome/browser/ui/tabs/tab_strip_model_observer.h" #include "ui/views/widget/widget_observer.h" @@ -145,6 +146,8 @@ // Methods that are bound by the DialogModel to call the callbacks. void OnDialogOk(); void OnDialogCancel(); + void OnCloseAction(); + void OnDialogDestroying(); Profile* GetProfile(); @@ -162,6 +165,8 @@ raw_ptr<views::Widget> widget_; const raw_ref<TabStripModel> tab_strip_model_; + + base::WeakPtrFactory<DeletionDialogController> weak_ptr_factory_{this}; }; } // namespace tab_groups diff --git a/chrome/browser/ui/tabs/tab_group_deletion_dialog_controller_unittest.cc b/chrome/browser/ui/tabs/tab_group_deletion_dialog_controller_unittest.cc index 6dfc11a..52cf82a 100644 --- a/chrome/browser/ui/tabs/tab_group_deletion_dialog_controller_unittest.cc +++ b/chrome/browser/ui/tabs/tab_group_deletion_dialog_controller_unittest.cc @@ -69,3 +69,19 @@ // Make sure the DeletionDialogController has destroyed it's state. EXPECT_FALSE(controller_->IsShowingDialog()); } + +TEST_F(DeletionDialogControllerUnitTest, DestroyControllerBeforeDialogModel) { + controller_->MaybeShowDialog( + DeletionDialogController::DialogMetadata( + DeletionDialogController::DialogType::DeleteSingle), + base::DoNothing()); + EXPECT_TRUE(controller_->IsShowingDialog()); + EXPECT_TRUE(dialog_host_); + + // Destroy the controller while the dialog model is still alive. + controller_.reset(); + + // Destroying the dialog model host should not crash when triggering callbacks + // on the destroyed controller. + ui::TestDialogModelHost::DestroyWithoutAction(std::move(dialog_host_)); +}
Regression Test / PoC
diff --git a/chrome/browser/ui/tabs/tab_group_deletion_dialog_controller_unittest.cc b/chrome/browser/ui/tabs/tab_group_deletion_dialog_controller_unittest.cc
index 6dfc11a..52cf82a 100644
--- a/chrome/browser/ui/tabs/tab_group_deletion_dialog_controller_unittest.cc
+++ b/chrome/browser/ui/tabs/tab_group_deletion_dialog_controller_unittest.cc
@@ -69,3 +69,19 @@
// Make sure the DeletionDialogController has destroyed it's state.
EXPECT_FALSE(controller_->IsShowingDialog());
}
+
+TEST_F(DeletionDialogControllerUnitTest, DestroyControllerBeforeDialogModel) {
+ controller_->MaybeShowDialog(
+ DeletionDialogController::DialogMetadata(
+ DeletionDialogController::DialogType::DeleteSingle),
+ base::DoNothing());
+ EXPECT_TRUE(controller_->IsShowingDialog());
+ EXPECT_TRUE(dialog_host_);
+
+ // Destroy the controller while the dialog model is still alive.
+ controller_.reset();
+
+ // Destroying the dialog model host should not crash when triggering callbacks
+ // on the destroyed controller.
+ ui::TestDialogModelHost::DestroyWithoutAction(std::move(dialog_host_));
+}
Original Bug Report
Potential Use-After-Free in DeletionDialogController on macOS via synchronous modal sheet
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: On macOS, showing a tab group deletion dialog synchronously spins a nested run loop during the sheet animation. If the browser window is closed during this loop, the owning DeletionDialogController is freed, leading to a potential Use-After-Free write when the function returns.
Affected files:
chrome/browser/ui/tabs/tab_group_deletion_dialog_controller.ccchrome/browser/ui/tabs/tab_group_deletion_dialog_controller.h
Estimated timestamp from git blame: 2025-06-17
Root Cause Analysis
In chrome/browser/ui/tabs/tab_group_deletion_dialog_controller.cc, DeletionDialogController::CreateDialogFromBrowser() displays a modal dialog and assigns the resulting views::Widget* directly to a member variable:
void DeletionDialogController::CreateDialogFromBrowser(
BrowserWindowInterface* browser,
std::unique_ptr<ui::DialogModel> dialog_model) {
widget_ = chrome::ShowBrowserModal(browser, std::move(dialog_model));
}
On macOS, showing a window-modal dialog synchronous-pathway (for in-process views) routes to NativeWidgetNSWindowBridge::ShowAsModalSheet(), which executes native Cocoa -[NSWindow beginSheet:completionHandler:] on the current call stack. This native Cocoa animation loop blocks the main thread and spins a nested CFRunLoop.
Because the dialog presentation is often initiated by a native UI gesture (such as right-clicking a tab group header on the native event stack), the Thread Controller’s task_execution_allowed flag remains true. Consequently, Chromium’s message loop is allowed to dispatch pending application tasks during the sheet’s slide-in animation.
If a window-close task (e.g. triggered by an extension calling chrome.windows.remove() or a compromised renderer initiating window closure) is executed inside the nested run loop, the main Browser window is destroyed. This synchronously destroys BrowserWindowFeatures and frees the DeletionDialogController instance. When beginSheet: finishes and chrome::ShowBrowserModal returns, the execution resumes inside CreateDialogFromBrowser(). The assignment to this->widget_ is then a Use-After-Free (UAF) write on a deleted this pointer.
Since this is not protected by MiraclePtr/BackupRefPtr (BRP) inside its own member functions, this results in unsandboxed memory corruption in the browser process.
Potential Trigger Steps
Note: These are potential steps and have not been validated in a running environment, as our tooling agent does not have execution capabilities.
- A saved tab group is present in the browser window.
- The user right-clicks the tab group header and selects “Delete group” (initiating a native NSEvent stack trace).
- During the modal sheet slide-in animation, an attacker-controlled extension or a compromised renderer triggers a window-close operation (e.g. via
chrome.windows.remove()). - The browser window is closed, synchronously tearing down
BrowserWindowFeaturesand deleting theDeletionDialogControllerinstance. - The sheet animation loop returns, and
CreateDialogFromBrowser()attempts to write the returnedWidget*to the deallocatedwidget_member, resulting in a heap write UAF.
Suggested Fix
To prevent accessing a deallocated this pointer when returning from the synchronous modal sheet presentation, add a base::WeakPtrFactory to DeletionDialogController and check a local stack-allocated base::WeakPtr before assigning the member variable:
// In tab_group_deletion_dialog_controller.h:
class DeletionDialogController : public TabStripModelObserver {
...
private:
...
base::WeakPtrFactory<DeletionDialogController> weak_ptr_factory_{this};
};
// In tab_group_deletion_dialog_controller.cc:
void DeletionDialogController::CreateDialogFromBrowser(
BrowserWindowInterface* browser,
std::unique_ptr<ui::DialogModel> dialog_model) {
base::WeakPtr<DeletionDialogController> weak_this = weak_ptr_factory_.GetWeakPtr();
views::Widget* widget = chrome::ShowBrowserModal(browser, std::move(dialog_model));
if (weak_this) {
widget_ = widget;
}
}
Evaluated with Chrome root at commit: b5b015ea5f690560237d1f0cff1405844cd12b8d
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.