CVE-2026-19557
Overview
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.ccchrome/browser/ui/tabs/tab_group_deletion_dialog_controller.hchrome/browser/ui/tabs/tab_group_deletion_dialog_controller_unittest.cc
Patch
From d2299d0dd4583ae879045d7d362dfb5eb64e0fd4 Mon Sep 17 00:00:00 2001
From: David Pennington <dpenning@google.com>
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 <dpenning@chromium.org>
Reviewed-by: Darryl James <dljames@chromium.org>
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.