Chrome · Extensions
CVE-2026-87637
UAF in Extensions
Overview
Medium
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Files Changed
chrome/browser/ui/extensions/extension_settings_overridden_dialog.ccchrome/browser/ui/extensions/extension_settings_overridden_dialog.hchrome/browser/ui/extensions/extension_settings_overridden_dialog_unittest.ccchrome/browser/ui/extensions/settings_overridden_dialog.ccchrome/browser/ui/extensions/settings_overridden_dialog_browsertest.ccchrome/browser/ui/extensions/settings_overridden_dialog_controller.hchrome/browser/ui/extensions/settings_overridden_dialog_unittest.cc
Patch
From 2e2a4073d4d202f064bc10f26b61ae5d563c56c2 Mon Sep 17 00:00:00 2001
From: Tim <tjudkins@chromium.org>
Date: Tue, 28 Jul 2026 13:36:32 -0700
Subject: [PATCH] [Extensions] Notify settings-overridden controller before showing dialog
ShowSettingsOverriddenDialog() dereferenced the dialog delegate after
ShowModalDialog() returned to call OnDialogShown() on the controller. On
some platforms showing a modal dialog can spin a nested run loop during
which the dialog widget, and the delegate and controller it owns, may be
destroyed, so the delegate must not be accessed once the dialog has been
shown.
Call OnDialogShown() before the controller's ownership is passed to the
dialog. This also guarantees OnDialogShown() runs before
HandleDialogResult(). Remove the now-unused controller() accessor and
add a unit test that closes the dialog widget from inside
Widget::Show().
Since this also means this is now technically called before the dialog
is shown and not after, we have renamed OnDialogShown() to
OnDialogWillBeShown().
Bug: 534863145
Change-Id: Ibc3ab29cb210271fb95f2ed5afc173c39efab1a0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8129643
Auto-Submit: Tim <tjudkins@chromium.org>
Commit-Queue: Tim <tjudkins@chromium.org>
Reviewed-by: Eva Su <evasu@chromium.org>
Commit-Queue: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1669722}
---
diff --git a/chrome/browser/ui/extensions/extension_settings_overridden_dialog.cc b/chrome/browser/ui/extensions/extension_settings_overridden_dialog.cc
index 55043ed4..05c9a14 100644
--- a/chrome/browser/ui/extensions/extension_settings_overridden_dialog.cc
+++ b/chrome/browser/ui/extensions/extension_settings_overridden_dialog.cc
@@ -208,7 +208,7 @@
return params_.content;
}
-void ExtensionSettingsOverriddenDialog::OnDialogShown() {
+void ExtensionSettingsOverriddenDialog::OnDialogWillBeShown() {
DCHECK(ShouldShow());
show_time_ = base::TimeTicks::Now();
diff --git a/chrome/browser/ui/extensions/extension_settings_overridden_dialog.h b/chrome/browser/ui/extensions/extension_settings_overridden_dialog.h
index 5fcf9740..64f4f5e 100644
--- a/chrome/browser/ui/extensions/extension_settings_overridden_dialog.h
+++ b/chrome/browser/ui/extensions/extension_settings_overridden_dialog.h
@@ -103,7 +103,7 @@
// SettingsOverriddenDialogController:
bool ShouldShow() override;
ShowParams GetShowParams() override;
- void OnDialogShown() override;
+ void OnDialogWillBeShown() override;
void HandleDialogResult(DialogResult result) override;
// Sets a callback to be invoked when the dialog result is handled.
diff --git a/chrome/browser/ui/extensions/extension_settings_overridden_dialog_unittest.cc b/chrome/browser/ui/extensions/extension_settings_overridden_dialog_unittest.cc
index c832a2ae..7f64073 100644
--- a/chrome/browser/ui/extensions/extension_settings_overridden_dialog_unittest.cc
+++ b/chrome/browser/ui/extensions/extension_settings_overridden_dialog_unittest.cc
@@ -130,7 +130,7 @@
ExtensionSettingsOverriddenDialog controller(
CreateTestDialogParams(extension->id()), *profile());
EXPECT_TRUE(controller.ShouldShow());
- controller.OnDialogShown();
+ controller.OnDialogWillBeShown();
controller.HandleDialogResult(DialogResult::kChangeSettingsBack);
histogram_tester.ExpectUniqueSample(kTestDialogResultHistogramName,
@@ -151,7 +151,7 @@
ExtensionSettingsOverriddenDialog controller(
CreateTestDialogParams(extension->id()), *profile());
EXPECT_TRUE(controller.ShouldShow());
- controller.OnDialogShown();
+ controller.OnDialogWillBeShown();
controller.HandleDialogResult(DialogResult::kKeepNewSettings);
histogram_tester.ExpectUniqueSample(kTestDialogResultHistogramName,
@@ -168,7 +168,7 @@
ExtensionSettingsOverriddenDialog controller(
CreateTestDialogParams(extension->id()), *profile());
- controller.OnDialogShown();
+ controller.OnDialogWillBeShown();
controller.HandleDialogResult(DialogResult::kDialogDismissed);
histogram_tester.ExpectUniqueSample(kTestDialogResultHistogramName,
@@ -186,7 +186,7 @@
ExtensionSettingsOverriddenDialog controller(
CreateTestDialogParams(extension->id()), *profile());
- controller.OnDialogShown();
+ controller.OnDialogWillBeShown();
controller.HandleDialogResult(DialogResult::kDialogClosedWithoutUserAction);
histogram_tester.ExpectUniqueSample(
@@ -205,7 +205,7 @@
ExtensionSettingsOverriddenDialog controller(
CreateTestDialogParams(extension->id()), *profile());
EXPECT_TRUE(controller.ShouldShow());
- controller.OnDialogShown();
+ controller.OnDialogWillBeShown();
controller.HandleDialogResult(DialogResult::kDialogDismissed);
}
@@ -226,7 +226,7 @@
ExtensionSettingsOverriddenDialog controller(
CreateTestDialogParams(extension_one->id()), *profile());
EXPECT_TRUE(controller.ShouldShow());
- controller.OnDialogShown();
+ controller.OnDialogWillBeShown();
controller.HandleDialogResult(DialogResult::kDialogDismissed);
}
@@ -245,7 +245,7 @@
ExtensionSettingsOverriddenDialog controller(
CreateTestDialogParams(extension->id()), *profile());
EXPECT_TRUE(controller.ShouldShow());
- controller.OnDialogShown();
+ controller.OnDialogWillBeShown();
registrar()->UninstallExtension(
extension->id(), extensions::UNINSTALL_REASON_FOR_TESTING, nullptr);
diff --git a/chrome/browser/ui/extensions/settings_overridden_dialog.cc b/chrome/browser/ui/extensions/settings_overridden_dialog.cc
index 4c054ae56..1dc25ee 100644
--- a/chrome/browser/ui/extensions/settings_overridden_dialog.cc
+++ b/chrome/browser/ui/extensions/settings_overridden_dialog.cc
@@ -82,8 +82,6 @@
HandleDialogResult(selected_setting_.value());
}
- SettingsOverriddenDialogController* controller() { return controller_.get(); }
-
private:
void HandleDialogResult(DialogResult result) {
DCHECK(!result_)
@@ -203,6 +201,10 @@
SettingsOverriddenDialogController::ShowParams show_params =
controller->GetShowParams();
+ // Notify the controller before its ownership is passed to the dialog, since
+ // showing the dialog may run a nested loop that destroys it.
+ controller->OnDialogWillBeShown();
+
auto dialog_delegate_unique =
std::make_unique<SettingsOverriddenDialogDelegate>(std::move(controller));
SettingsOverriddenDialogDelegate* dialog_delegate =
@@ -227,8 +229,6 @@
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)
ShowModalDialog(parent, dialog_builder.Build());
-
- dialog_delegate->controller()->OnDialogShown();
}
} // namespace extensions
diff --git a/chrome/browser/ui/extensions/settings_overridden_dialog_browsertest.cc b/chrome/browser/ui/extensions/settings_overridden_dialog_browsertest.cc
index 62843b66..b562fd6b 100644
--- a/chrome/browser/ui/extensions/settings_overridden_dialog_browsertest.cc
+++ b/chrome/browser/ui/extensions/settings_overridden_dialog_browsertest.cc
@@ -63,7 +63,7 @@
private:
bool ShouldShow() override { return true; }
ShowParams GetShowParams() override { return show_params_; }
- void OnDialogShown() override {}
+ void OnDialogWillBeShown() override {}
void HandleDialogResult(DialogResult result) override {
ASSERT_FALSE(dialog_result_out_->has_value());
*dialog_result_out_ = result;
diff --git a/chrome/browser/ui/extensions/settings_overridden_dialog_controller.h b/chrome/browser/ui/extensions/settings_overridden_dialog_controller.h
index 002d8deb..d865afb 100644
--- a/chrome/browser/ui/extensions/settings_overridden_dialog_controller.h
+++ b/chrome/browser/ui/extensions/settings_overridden_dialog_controller.h
@@ -93,8 +93,8 @@
// synchronously.
virtual ShowParams GetShowParams() = 0;
- // Notifies the controller that the dialog has been shown.
- virtual void OnDialogShown() = 0;
+ // Notifies the controller that the dialog will be shown.
+ virtual void OnDialogWillBeShown() = 0;
// Handles the result of the dialog being shown.
virtual void HandleDialogResult(DialogResult result) = 0;
diff --git a/chrome/browser/ui/extensions/settings_overridden_dialog_unittest.cc b/chrome/browser/ui/extensions/settings_overridden_dialog_unittest.cc
index d9fa6bf..d0f22669 100644
--- a/chrome/browser/ui/extensions/settings_overridden_dialog_unittest.cc
+++ b/chrome/browser/ui/extensions/settings_overridden_dialog_unittest.cc
@@ -7,6 +7,7 @@
#include <optional>
#include "base/memory/raw_ptr.h"
+#include "base/test/bind.h"
#include "chrome/browser/ui/extensions/extensions_dialogs.h"
#include "chrome/browser/ui/extensions/settings_overridden_dialog_controller.h"
#include "chrome/browser/ui/views/chrome_constrained_window_views_client.h"
@@ -40,8 +41,9 @@
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/chrome/browser/ui/extensions/extension_settings_overridden_dialog_unittest.cc b/chrome/browser/ui/extensions/extension_settings_overridden_dialog_unittest.cc
index c832a2ae..7f64073 100644
--- a/chrome/browser/ui/extensions/extension_settings_overridden_dialog_unittest.cc
+++ b/chrome/browser/ui/extensions/extension_settings_overridden_dialog_unittest.cc
@@ -130,7 +130,7 @@
ExtensionSettingsOverriddenDialog controller(
CreateTestDialogParams(extension->id()), *profile());
EXPECT_TRUE(controller.ShouldShow());
- controller.OnDialogShown();
+ controller.OnDialogWillBeShown();
controller.HandleDialogResult(DialogResult::kChangeSettingsBack);
histogram_tester.ExpectUniqueSample(kTestDialogResultHistogramName,
@@ -151,7 +151,7 @@
ExtensionSettingsOverriddenDialog controller(
CreateTestDialogParams(extension->id()), *profile());
EXPECT_TRUE(controller.ShouldShow());
- controller.OnDialogShown();
+ controller.OnDialogWillBeShown();
controller.HandleDialogResult(DialogResult::kKeepNewSettings);
histogram_tester.ExpectUniqueSample(kTestDialogResultHistogramName,
@@ -168,7 +168,7 @@
ExtensionSettingsOverriddenDialog controller(
CreateTestDialogParams(extension->id()), *profile());
- controller.OnDialogShown();
+ controller.OnDialogWillBeShown();
controller.HandleDialogResult(DialogResult::kDialogDismissed);
histogram_tester.ExpectUniqueSample(kTestDialogResultHistogramName,
@@ -186,7 +186,7 @@
ExtensionSettingsOverriddenDialog controller(
CreateTestDialogParams(extension->id()), *profile());
- controller.OnDialogShown();
+ controller.OnDialogWillBeShown();
controller.HandleDialogResult(DialogResult::kDialogClosedWithoutUserAction);
histogram_tester.ExpectUniqueSample(
@@ -205,7 +205,7 @@
ExtensionSettingsOverriddenDialog controller(
CreateTestDialogParams(extension->id()), *profile());
EXPECT_TRUE(controller.ShouldShow());
- controller.OnDialogShown();
+ controller.OnDialogWillBeShown();
controller.HandleDialogResult(DialogResult::kDialogDismissed);
}
@@ -226,7 +226,7 @@
ExtensionSettingsOverriddenDialog controller(
CreateTestDialogParams(extension_one->id()), *profile());
EXPECT_TRUE(controller.ShouldShow());
- controller.OnDialogShown();
+ controller.OnDialogWillBeShown();
controller.HandleDialogResult(DialogResult::kDialogDismissed);
}
@@ -245,7 +245,7 @@
ExtensionSettingsOverriddenDialog controller(
CreateTestDialogParams(extension->id()), *profile());
EXPECT_TRUE(controller.ShouldShow());
- controller.OnDialogShown();
+ controller.OnDialogWillBeShown();
registrar()->UninstallExtension(
extension->id(), extensions::UNINSTALL_REASON_FOR_TESTING, nullptr);
diff --git a/chrome/browser/ui/extensions/settings_overridden_dialog_browsertest.cc b/chrome/browser/ui/extensions/settings_overridden_dialog_browsertest.cc
index 62843b66..b562fd6b 100644
--- a/chrome/browser/ui/extensions/settings_overridden_dialog_browsertest.cc
+++ b/chrome/browser/ui/extensions/settings_overridden_dialog_browsertest.cc
@@ -63,7 +63,7 @@
private:
bool ShouldShow() override { return true; }
ShowParams GetShowParams() override { return show_params_; }
- void OnDialogShown() override {}
+ void OnDialogWillBeShown() override {}
void HandleDialogResult(DialogResult result) override {
ASSERT_FALSE(dialog_result_out_->has_value());
*dialog_result_out_ = result;
diff --git a/chrome/browser/ui/extensions/settings_overridden_dialog_unittest.cc b/chrome/browser/ui/extensions/settings_overridden_dialog_unittest.cc
index d9fa6bf..d0f22669 100644
--- a/chrome/browser/ui/extensions/settings_overridden_dialog_unittest.cc
+++ b/chrome/browser/ui/extensions/settings_overridden_dialog_unittest.cc
@@ -7,6 +7,7 @@
#include <optional>
#include "base/memory/raw_ptr.h"
+#include "base/test/bind.h"
#include "chrome/browser/ui/extensions/extensions_dialogs.h"
#include "chrome/browser/ui/extensions/settings_overridden_dialog_controller.h"
#include "chrome/browser/ui/views/chrome_constrained_window_views_client.h"
@@ -40,8 +41,9 @@
private:
bool ShouldShow() override { return true; }
ShowParams GetShowParams() override { return show_params_; }
- void OnDialogShown() override {
- EXPECT_FALSE(state_->shown) << "OnDialogShown() called more than once!";
+ void OnDialogWillBeShown() override {
+ EXPECT_FALSE(state_->shown)
+ << "OnDialogWillBeShown() called more than once!";
state_->shown = true;
}
void HandleDialogResult(DialogResult result) override {
@@ -96,6 +98,10 @@
return dialog;
}
+ gfx::NativeWindow parent_window() {
+ return parent_widget_->GetNativeWindow();
+ }
+
private:
std::unique_ptr<views::Widget> parent_widget_;
};
@@ -147,3 +153,29 @@
ASSERT_TRUE(state.result);
EXPECT_EQ(DialogResult::kDialogClosedWithoutUserAction, state.result);
}
+
+// Showing a modal dialog can, on some platforms, result in the dialog widget
+// being synchronously destroyed before Widget::Show() returns (for example if
+// a nested run loop closes the parent window). Verify that the controller is
+// notified that the dialog was shown, and that the result is delivered, even
+// when the dialog is destroyed as soon as it is shown.
+TEST_F(SettingsOverriddenDialogUnitTest, DialogDestroyedWhileBeingShown) {
+ DialogState state;
+
+ views::AnyWidgetObserver observer(views::test::AnyWidgetTestPasskey{});
+ observer.set_shown_callback(
+ base::BindLambdaForTesting([&](views::Widget* widget) {
+ if (widget->GetName() != kExtensionSettingsOverriddenDialogName) {
+ return;
+ }
+ EXPECT_TRUE(state.shown);
+ widget->CloseNow();
+ }));
+
+ extensions::ShowSettingsOverriddenDialog(
+ std::make_unique<TestDialogController>(&state), parent_window());
+
+ EXPECT_TRUE(state.shown);
+ ASSERT_TRUE(state.result);
+ EXPECT_EQ(DialogResult::kDialogClosedWithoutUserAction, state.result);
+}
diff --git a/chrome/browser/ui/search_engines/default_search_extension_controlled_controller_unittest.cc b/chrome/browser/ui/search_engines/default_search_extension_controlled_controller_unittest.cc
index d4fd69f..315ea9b 100644
--- a/chrome/browser/ui/search_engines/default_search_extension_controlled_controller_unittest.cc
+++ b/chrome/browser/ui/search_engines/default_search_extension_controlled_controller_unittest.cc
@@ -348,7 +348,7 @@
ExtensionSettingsOverriddenDialog::ShowParams(u"Title", u"Body",
nullptr));
ExtensionSettingsOverriddenDialog dialog(std::move(params), profile_);
- dialog.OnDialogShown();
+ dialog.OnDialogWillBeShown();
DefaultSearchExtensionControlledController owned_controller(browser_window_,
profile_);
Loading diff…
Original Bug Report
The reporter's bug is still restricted on the tracker. Chrome de-restricts security bugs ~30–90 days after the fix ships; a later run will backfill it here.
References
On This Page