PatchGap
Chrome · Extensions › Extensions

CVE-2026-87637

UAF in Extensions (Extensions)
Medium watch CVSS 9.6 chrome UAF 📄 Reporter bug report 🔧 Commit mapped 🧬 A nested callout frees the object, then it is used

Overview

Medium
Severity
9.6
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactUse after free in Extensions
DescriptionUse after free in Extensions
ComponentExtensions
Sub-componentExtensions
Bug ClassUAF
Action tierwatch — corruption primitive but gated (uncommon) — not default-reachable
Computed by PatchGap's deterministic exploitability ruler from the fixing commit. patch-now = RCE-class & attacker-reachable in a default config, or exploited in the wild; watch = real security signal below that bar; noise = no security relevance.
Tracker534863145
Fix commit2e2a4073d4d2 (chromium/src) +51/-19
CWECWE-416 (Use-after-free)
CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:C/C:H/I:H/A:H
CISA KEVNot listed
CreditedGoogle
Reported2026-07-14 (per vendor advisory)
Fix landed2026-07-28 (public git)
Disclosed2026-09-08
Patch gap42 days — fix public in source before the advisory

Background

`ShowSettingsOverriddenDialog()`
free function in chrome/browser/ui/extensions/settings_overridden_dialog.cc that builds and displays the modal warning shown when an extension overrides browser settings (search engine, homepage, or new-tab page).
`SettingsOverriddenDialogController`
an interface (settings_overridden_dialog_controller.h) whose implementation owns the dialog’s data and receives lifecycle callbacks (ShouldShow(), GetShowParams(), OnDialogShown()/OnDialogWillBeShown(), HandleDialogResult()).
`SettingsOverriddenDialogDelegate`
the views dialog delegate that takes ownership of the SettingsOverriddenDialogController via std::move; the delegate is in turn owned by the dialog views::Widget.
Nested run loop
a message loop spun re-entrantly while ShowModalDialog() runs, during which the widget (and everything it owns) can be synchronously destroyed before the call returns.

Root Cause Analysis

In the pre-patch ShowSettingsOverriddenDialog(), ownership of the controller was moved into a SettingsOverriddenDialogDelegate, which was moved into the dialog views::Widget, and only after ShowModalDialog(parent, dialog_builder.Build()) returned did the code dereference the raw dialog_delegate pointer to call dialog_delegate->controller()->OnDialogShown().

The invariant that was violated is that dialog_delegate (and the controller it owned) must still be alive when dereferenced: on some platforms ShowModalDialog() spins a nested run loop during which the widget — and thus the delegate and controller it transitively owns — can be destroyed, leaving dialog_delegate dangling and making the subsequent ->controller()->OnDialogShown() a use-after-free. The removed comment-free trailing line and the commit message confirm the old, wrong ordering: notify the controller after the dialog is shown.

The fix moves the notification to controller->OnDialogWillBeShown() before ownership is transferred to the delegate (renaming OnDialogShown() accordingly and deleting the now-unused controller() accessor), so the controller is only ever touched while the caller still holds it. This eliminates any post-ShowModalDialog() access to the freed objects while preserving the guarantee that the notification runs before HandleDialogResult().

Key insight
The single mistake was ordering a callback on an object after handing that object’s ownership to a modal dialog that can synchronously destroy it; the fix reorders the callback to fire while the caller still owns the controller, removing the dangling dereference entirely.

Attack Path

  1. Trigger the overridden-settings dialog Cause ShowSettingsOverriddenDialog() to run, e.g. via an installed extension that overrides the default search engine, homepage, or new-tab page, which schedules the modal warning.
  2. Transfer controller ownership into the widget The controller is std::moved into a SettingsOverriddenDialogDelegate, which is moved into the dialog views::Widget; the raw dialog_delegate pointer is retained for use after showing.
  3. Destroy the widget during show ShowModalDialog() spins a nested run loop in which the dialog widget is destroyed (the regression test closes the widget from inside Widget::Show()), freeing the delegate and the controller it owns.
  4. Dangling dereference Control returns to the line dialog_delegate->controller()->OnDialogShown(), which reads through the freed dialog_delegate and freed controller — the use-after-free.
  5. Memory-corruption escalation (inferred) — Reclaiming the freed allocation before the dereference could turn the read/call into controlled behavior; the patch does not demonstrate a concrete read/write primitive or code execution.

Impact Assessment

An attacker gains a use-after-free of the controller/delegate objects in the browser process (Extensions UI code), which is trusted and unsandboxed. The precondition is that the settings-overridden dialog is shown and its widget is destroyed via a nested run loop before ShowModalDialog() returns — a platform-dependent condition the commit describes for “some platforms.” Any escalation to controlled memory corruption or code execution is (inferred); the advisory classifies this as a use-after-free in Extensions and I have no report of in-the-wild exploitation.

Changed Functions

FunctionChangeNotes
TEST_F
chrome/browser/ui/extensions/settings_overridden_dialog_unittest.cc
 modified
BindLambdaForTesting
chrome/browser/ui/extensions/settings_overridden_dialog_unittest.cc
 modified

Files Changed

  • chrome/browser/ui/extensions/settings_overridden_dialog_unittest.cc
  • chrome/browser/ui/extensions/settings_overridden_dialog_controller.h
  • chrome/browser/ui/search_engines/default_search_extension_controlled_controller_unittest.cc
  • chrome/browser/ui/extensions/extension_settings_overridden_dialog.h
  • chrome/browser/ui/extensions/extension_settings_overridden_dialog.cc
  • chrome/browser/ui/extensions/extension_settings_overridden_dialog_unittest.cc
  • chrome/browser/ui/extensions/settings_overridden_dialog.cc
  • chrome/browser/ui/extensions/settings_overridden_dialog_browsertest.cc

Audit Directions

  • Post-show dereference of moved-ownership objects
    Audit any code that keeps a raw pointer to an object whose ownership was moved into a widget/dialog and then dereferences it after a Show/RunModal/ShowModalDialog call that can spin a nested run loop.
  • Callbacks ordered after modal display
    Look for lifecycle notifications (OnDialogShown, On*Shown, completion callbacks) invoked after showing a modal, and verify the receiver cannot be destroyed by re-entrant loops before the call.
  • Nested run loop re-entrancy in UI controllers
    Review views/dialog controllers on Windows and macOS (BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) paths) where widget destruction during show is possible, treating any access after the show call as suspect.

Patch

view commit
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/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_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/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…

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

reported by vm...@google.com

Potential Use-After-Free in ShowSettingsOverriddenDialog due to macOS nested run loop

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: A potential Use-After-Free (UAF) vulnerability exists in the browser process when displaying the settings overridden dialog on macOS. Initiating the modal sheet can synchronously trigger a nested event-tracking run loop during which the parent window and dialog widget can be destroyed. This results in stack-local raw pointers being dereferenced upon return, leading to potential memory corruption in the unsandboxed browser process.

Affected files:

  • chrome/browser/ui/extensions/settings_overridden_dialog.cc

Estimated timestamp from git blame: 2022-08-09

Root Cause Analysis

In chrome/browser/ui/extensions/settings_overridden_dialog.cc, ShowSettingsOverriddenDialog retains a bare stack pointer to a SettingsOverriddenDialogDelegate whose ownership has been transferred to the ui::DialogModel:

void ShowSettingsOverriddenDialog(
    std::unique_ptr<SettingsOverriddenDialogController> controller,
    gfx::NativeWindow parent) {
  ...
  auto dialog_delegate_unique =
      std::make_unique<SettingsOverriddenDialogDelegate>(std::move(controller));
  SettingsOverriddenDialogDelegate* dialog_delegate =
      dialog_delegate_unique.get();

  ui::DialogModel::Builder dialog_builder =
      ui::DialogModel::Builder(std::move(dialog_delegate_unique));
  ...
  ShowModalDialog(parent, dialog_builder.Build());

  dialog_delegate->controller()->OnDialogShown();
}

On macOS, ShowModalDialog eventually calls NativeWidgetNSWindowBridge::ShowAsModalSheet in components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm. This synchronously executes -[NSWindow beginSheet:completionHandler:], which enters a synchronous, nested run loop to animate the sheet.

Because the nested run loop continues processing the browser’s UI thread tasks and IPC messages, a queued close event (e.g., from a webpage closing its parent window or via extension APIs) can run and destroy the widget. Since the BubbleDialogModelHost and its underlying DialogModel are owned by the widget, this destruction chain deletes both the SettingsOverriddenDialogDelegate and its owned SettingsOverriddenDialogController.

Upon returning from the nested run loop, the stack-local dialog_delegate pointer is now dangling. Dereferencing it to call dialog_delegate->controller()->OnDialogShown() results in a Use-After-Free and an immediate virtual function dispatch through a potentially attacker-controlled vtable.

Potential Trigger Steps

Note: These are potential steps based on static code analysis; our automated tooling does not currently have the capability to execute proof-of-concept code.

  1. Install an extension that overrides the New Tab Page (NTP) on macOS.
  2. Programmatically open the NTP using chrome.tabs.create or another page-opening mechanism to trigger ShowSettingsOverriddenDialog.
  3. While the sheet slide-down animation is executing (spinning the AppKit nested run loop), immediately close the window or tab (e.g., using chrome.tabs.remove from the extension’s background script).
  4. The window close IPC is processed during the nested run loop, destroying the widget and the dialog model host.
  5. Once the animation finishes and the nested loop terminates, control returns to ShowSettingsOverriddenDialog, which attempts to invoke OnDialogShown() on the freed delegate/controller.

Suggested Fix

To prevent holding a dangling pointer across the potentially blocking ShowModalDialog call, retrieve a weak pointer to the controller prior to showing the dialog, and check its validity before invoking the notification:

// Inside ShowSettingsOverriddenDialog
base::WeakPtr<SettingsOverriddenDialogController> weak_controller = controller->AsWeakPtr();

ShowModalDialog(parent, dialog_builder.Build());

if (weak_controller) {
  weak_controller->OnDialogShown();
}

Alternatively, if the design allows, notify the controller that the dialog was shown prior to calling ShowModalDialog.

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.

View on issue tracker
On This Page