CVE-2026-79140

Medium chrome UAF 📄 Reporter bug report 🔧 Commit mapped

Overview

Medium

Severity

—

CVSS

Exploited ITW

Fixed

Fix Status

ImpactUse after free in Views

DescriptionUse after free in Views

ComponentViews

Bug ClassUAF

Tracker502101200

Fix commitc763f3b4837e (chromium/src) +174/-100

CISA KEVNot listed

CreditedGoogle

Disclosed2026-08-25

Changed Functions

Function	Change	Notes
`ASSERT_TRUE` chrome/browser/ui/signin/signin_view_controller_interactive_uitest.cc	modified

Files Changed

chrome/browser/ui/cocoa/browser_window_mac_browsertest.mm
chrome/browser/ui/extensions/settings_overridden_dialog_browsertest.cc
chrome/browser/ui/signin/signin_view_controller_interactive_uitest.cc
chrome/browser/ui/test/test_browser_dialog.cc

Patch

view commit

From c763f3b4837ea55058610ecf628cd140ef40b52e Mon Sep 17 00:00:00 2001
From: Bryan Oltman <[email protected]>
Date: Tue, 21 Jul 2026 09:30:20 -0700
Subject: [PATCH] Reland "Reland "[macOS] Make modal sheet presentation asynchronous to prevent UAF bugs""

This reverts commit d4c2f03a85a5eb43c9bb0f43088cfe3acfd0efec.

Reason for revert: msan tests were failing, this failure has been fixed

Original change's description:
> Revert "Reland "[macOS] Make modal sheet presentation asynchronous to prevent UAF bugs""
>
> This reverts commit 3c030585436edb1989cd19af246f8e4db610ebeb.
>
> Reason for revert: Culprit of failure of
> TailoredSecurityDesktopDialogManagerTest.OpeningANewDisableDialogWillCloseAnyOpenEnableDialogs
>
> Failure Link: https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20MSan%20Tests/64261/overview
>
> Original change's description:
> > Reland "[macOS] Make modal sheet presentation asynchronous to prevent UAF bugs"
> >
> > This reverts commit 879c1fa69bbe9480b7aaaa1b8e4b16881642b266.
> >
> > Reason for revert: Rolling forward with fixes
> >
> > Original change's description:
> > > Revert "[macOS] Make modal sheet presentation asynchronous to prevent UAF bugs"
> > >
> > > This reverts commit 90309b0d2c7d8f975a2e3cc672d3e59b3e19947e.
> > >
> > > Reason for revert:
> > > LUCI Bisection has identified this change as the cause of a test failure. See the analysis: https://ci.chromium.org/ui/p/chromium/bisection/test-analysis/b/5250851556818944
> > >
> > > Sample build with failed test: https://ci.chromium.org/b/8676059972782943729
> > > Affected test(s):
> > > [://chrome/test\:browser_tests!gtest::BookmarkAccountStorageMoveDialogPixelTest#InvokeUi_ShowMoveBookmarkToAccount](https://ci.chromium.org/ui/test/chromium/:%2F%2Fchrome%2Ftest%5C:browser_tests%21gtest::BookmarkAccountStorageMoveDialogPixelTest%23InvokeUi_ShowMoveBookmarkToAccount?q=VHash%3A812306a38c719b85)
> > > [://chrome/test\:browser_tests!gtest::BookmarkAccountStorageMoveDialogPixelTest#InvokeUi_ShowMoveFolderToAccount](https://ci.chromium.org/ui/test/chromium/:%2F%2Fchrome%2Ftest%5C:browser_tests%21gtest::BookmarkAccountStorageMoveDialogPixelTest%23InvokeUi_ShowMoveFolderToAccount?q=VHash%3A812306a38c719b85)
> > > [://chrome/test\:browser_tests!gtest::ExtensionInstallBlockedByParentDialogTest#InvokeUi_app](https://ci.chromium.org/ui/test/chromium/:%2F%2Fchrome%2Ftest%5C:browser_tests%21gtest::ExtensionInstallBlockedByParentDialogTest%23InvokeUi_app?q=VHash%3A812306a38c719b85)
> > > [://chrome/test\:browser_tests!gtest::ExtensionInstallBlockedByParentDialogTest#InvokeUi_extension](https://ci.chromium.org/ui/test/chromium/:%2F%2Fchrome%2Ftest%5C:browser_tests%21gtest::ExtensionInstallBlockedByParentDialogTest%23InvokeUi_extension?q=VHash%3A812306a38c719b85)
> > > [://chrome/test\:browser_tests!gtest::SettingsOverriddenDialogBrowserTest#InvokeUi_SimpleDialog](https://ci.chromium.org/ui/test/chromium/:%2F%2Fchrome%2Ftest%5C:browser_tests%21gtest::SettingsOverriddenDialogBrowserTest%23InvokeUi_SimpleDialog?q=VHash%3A812306a38c719b85)
> > > and 14 more ...
> > >
> > > If this is a false positive, please report it at http://b.corp.google.com/createIssue?component=1199205&description=Analysis%3A+https%3A%2F%2Fci.chromium.org%2Fui%2Fp%2Fchromium%2Fbisection%2Ftest-analysis%2Fb%2F5250851556818944&format=PLAIN&priority=P3&title=Wrongly+blamed+https%3A%2F%2Fchromium-review.googlesource.com%2Fc%2Fchromium%2Fsrc%2F%2B%2F8086464&type=BUG
> > >
> > > Original change's description:
> > > > [macOS] Make modal sheet presentation asynchronous to prevent UAF bugs
> > > >
> > > > Modal sheet animations run a nested run loop synchronously in AppKit.
> > > > During this loop, the main thread remains active, allowing the parent or
> > > > child widget to be closed and its C++ bridge synchronously destroyed.
> > > > This leads to Use-After-Free (UAF) bugs when the nested loop exits and
> > > > execution resumes in the member functions of the deleted bridge.
> > > >
> > > > This CL runs modal sheet presentations asynchronously to prevent UAF:
> > > > - Always post the sheet animation task to the message loop (previously
> > > >   only done for remote cocoa).
> > > > - Add the SetVisibilityStateBridgeDestruction regression test.
> > > >
> > > > This CL also removes a weak pointer check in
> > > > FullscreenControllerTransitionComplete that is no longer necessary now
> > > > that the modal sheet presentation is asynchronous.
> > > >
> > > > Include-Ci-Only-Tests: true
> > > > Fixed: 518006007,502109333,502101200
> > > > Change-Id: I9a911b80d087f79920cbd944af854dbf82c6f6ae
> > > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8086464
> > > > Commit-Queue: Bryan Oltman <[email protected]>
> > > > Reviewed-by: Keren Zhu <[email protected]>
> > > > Cr-Commit-Position: refs/heads/main@{#1663648}
> > > >
> > >
> > > No-Presubmit: true
> > > No-Tree-Checks: true
> > > No-Try: true
> > > Change-Id: I4604e56d435714b79d85c20029981319dd61c0e5
> > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8114268
> > > Owners-Override: Kalvin Lee <[email protected]>
> > > Commit-Queue: Kalvin Lee <[email protected]>
> > > Bot-Commit: [email protected] <[email protected]>
> > > Cr-Commit-Position: refs/heads/main@{#1663689}
> >
> > Fixed: 518006007,502109333,502101200
> > Include-Ci-Only-Tests: chromium.mac:mac15-x64-rel-tests|browser_tests
> > Cq-Include-Trybots: luci.chromium.try:mac15-x64-rel-tests
> > Change-Id: I1ae6f9509f889b4b492c8ef4de0f27f90388913a
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8117687
> > Reviewed-by: Keren Zhu <[email protected]>
> > Reviewed-by: Avi Drissman <[email protected]>
> > Commit-Queue: Bryan Oltman <[email protected]>
> > Cr-Commit-Position: refs/heads/main@{#1664862}
>
> Bug: 518006007,502109333,502101200
> Cq-Include-Trybots: luci.chromium.try:mac15-x64-rel-tests
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Change-Id: Ic23979a97b595e8a1ce8bb238acbdde41fd0b174
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8128775
> Owners-Override: Eriko Kurimoto <[email protected]>
> Auto-Submit: Eriko Kurimoto <[email protected]>
> Bot-Commit: [email protected] <[email protected]>
> Commit-Queue: [email protected] <[email protected]>
> Cr-Commit-Position: refs/heads/main@{#1665037}

Bug: 518006007,502109333,502101200
Cq-Include-Trybots: luci.chromium.try:mac15-x64-rel-tests; luci.chromium.try:linux_chromium_msan_rel_ng
Change-Id: I13a6db96aa0200411b93e2712340cb297cbac952
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8131917
Reviewed-by: Keren Zhu <[email protected]>
Reviewed-by: Avi Drissman <[email protected]>
Commit-Queue: Bryan Oltman <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1665550}
---

diff --git a/chrome/browser/ui/cocoa/browser_window_mac_browsertest.mm b/chrome/browser/ui/cocoa/browser_window_mac_browsertest.mm
index 8c297816..1fb89328 100644
--- a/chrome/browser/ui/cocoa/browser_window_mac_browsertest.mm
+++ b/chrome/browser/ui/cocoa/browser_window_mac_browsertest.mm
@@ -8,6 +8,7 @@
 
 #include <memory>
 
+#include "base/test/run_until.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/app_controller_mac.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
@@ -189,7 +190,8 @@
       BookmarkEditor::SHOW_TREE, base::DoNothing());
   editor->Show(browser()->GetWindow()->GetNativeWindow());
   auto* editor_raw = editor.release();
-  ASSERT_TRUE([AppController.sharedController keyWindowIsModal]);
+  ASSERT_TRUE(base::test::RunUntil(
+      [&]() { return [AppController.sharedController keyWindowIsModal]; }));
 
   // These commands should be disabled when the sheet is attached.
   EXPECT_FALSE([window validateUserInterfaceItem:bookmark_all_tabs_item]);
@@ -198,7 +200,8 @@
 
   // Close the sheet dialog.
   editor_raw->GetWidget()->CloseNow();
-  ASSERT_FALSE([AppController.sharedController keyWindowIsModal]);
+  ASSERT_TRUE(base::test::RunUntil(
+      [&]() { return ![AppController.sharedController keyWindowIsModal]; }));
 
   // These commands should be enabled again when the sheet is removed.
   EXPECT_TRUE([window validateUserInterfaceItem:bookmark_all_tabs_item]);
diff --git a/chrome/browser/ui/extensions/settings_overridden_dialog_browsertest.cc b/chrome/browser/ui/extensions/settings_overridden_dialog_browsertest.cc
index 43d7471..62843b66 100644
--- a/chrome/browser/ui/extensions/settings_overridden_dialog_browsertest.cc
+++ b/chrome/browser/ui/extensions/settings_overridden_dialog_browsertest.cc
@@ -13,6 +13,7 @@
 #include "base/memory/raw_ptr.h"
 #include "base/path_service.h"
 #include "base/strings/string_util.h"
+#include "base/task/current_thread.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/time/time.h"
 #include "chrome/browser/extensions/chrome_test_extension_loader.h"
@@ -132,7 +133,9 @@
         std::make_unique<TestDialogController>(std::move(params),
                                                &dialog_result_),
         browser->GetWindow()->GetNativeWindow());
-    return waiter.WaitIfNeededAndGet();
+    views::Widget* widget = waiter.WaitIfNeededAndGet();
+    EXPECT_TRUE(base::test::RunUntil([&] { return widget->IsVisible(); }));
+    return widget;
   }
 
   void ShowNtpOverriddenDefaultDialog() {
diff --git a/chrome/browser/ui/signin/signin_view_controller_interactive_uitest.cc b/chrome/browser/ui/signin/signin_view_controller_interactive_uitest.cc
index 3dfb0e6..8d01dd8a 100644
--- a/chrome/browser/ui/signin/signin_view_controller_interactive_uitest.cc
+++ b/chrome/browser/ui/signin/signin_view_controller_interactive_uitest.cc
@@ -42,6 +42,7 @@
 #include "components/signin/public/identity_manager/identity_manager.h"
 #include "components/signin/public/identity_manager/identity_test_utils.h"
 #include "components/sync/base/features.h"
+#include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
@@ -231,6 +232,14 @@
   EXPECT_TRUE(signin_view_controller->ShowsModalDialog());
   content_observer.Wait();
 
+  // Wait for the modal dialog sheet window to be presented and receive focus.
+  ASSERT_TRUE(base::test::RunUntil([&]() {
+    auto* web_contents =
+        signin_view_controller->GetModalDialogWebContentsForTesting();
+    return web_contents && web_contents->GetRenderWidgetHostView() &&
+           web_contents->GetRenderWidgetHostView()->HasFocus();
+  }));
+
   content::WebContentsDestroyedWatcher dialog_destroyed_watcher(
       signin_view_controller->GetModalDialogWebContentsForTesting());
 
diff --git a/chrome/browser/ui/test/test_browser_dialog.cc b/chrome/browser/ui/test/test_browser_dialog.cc
index 3ca4d2f..ef42317 100644
--- a/chrome/browser/ui/test/test_browser_dialog.cc

Loading diff…

Regression Test / PoC

shipped with the fix

diff --git a/chrome/browser/ui/cocoa/browser_window_mac_browsertest.mm b/chrome/browser/ui/cocoa/browser_window_mac_browsertest.mm
index 8c297816..1fb89328 100644
--- a/chrome/browser/ui/cocoa/browser_window_mac_browsertest.mm
+++ b/chrome/browser/ui/cocoa/browser_window_mac_browsertest.mm
@@ -8,6 +8,7 @@
 
 #include <memory>
 
+#include "base/test/run_until.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/app_controller_mac.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
@@ -189,7 +190,8 @@
       BookmarkEditor::SHOW_TREE, base::DoNothing());
   editor->Show(browser()->GetWindow()->GetNativeWindow());
   auto* editor_raw = editor.release();
-  ASSERT_TRUE([AppController.sharedController keyWindowIsModal]);
+  ASSERT_TRUE(base::test::RunUntil(
+      [&]() { return [AppController.sharedController keyWindowIsModal]; }));
 
   // These commands should be disabled when the sheet is attached.
   EXPECT_FALSE([window validateUserInterfaceItem:bookmark_all_tabs_item]);
@@ -198,7 +200,8 @@
 
   // Close the sheet dialog.
   editor_raw->GetWidget()->CloseNow();
-  ASSERT_FALSE([AppController.sharedController keyWindowIsModal]);
+  ASSERT_TRUE(base::test::RunUntil(
+      [&]() { return ![AppController.sharedController keyWindowIsModal]; }));
 
   // These commands should be enabled again when the sheet is removed.
   EXPECT_TRUE([window validateUserInterfaceItem:bookmark_all_tabs_item]);
diff --git a/chrome/browser/ui/extensions/settings_overridden_dialog_browsertest.cc b/chrome/browser/ui/extensions/settings_overridden_dialog_browsertest.cc
index 43d7471..62843b66 100644
--- a/chrome/browser/ui/extensions/settings_overridden_dialog_browsertest.cc
+++ b/chrome/browser/ui/extensions/settings_overridden_dialog_browsertest.cc
@@ -13,6 +13,7 @@
 #include "base/memory/raw_ptr.h"
 #include "base/path_service.h"
 #include "base/strings/string_util.h"
+#include "base/task/current_thread.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/time/time.h"
 #include "chrome/browser/extensions/chrome_test_extension_loader.h"
@@ -132,7 +133,9 @@
         std::make_unique<TestDialogController>(std::move(params),
                                                &dialog_result_),
         browser->GetWindow()->GetNativeWindow());
-    return waiter.WaitIfNeededAndGet();
+    views::Widget* widget = waiter.WaitIfNeededAndGet();
+    EXPECT_TRUE(base::test::RunUntil([&] { return widget->IsVisible(); }));
+    return widget;
   }
 
   void ShowNtpOverriddenDefaultDialog() {
diff --git a/chrome/browser/ui/signin/signin_view_controller_interactive_uitest.cc b/chrome/browser/ui/signin/signin_view_controller_interactive_uitest.cc
index 3dfb0e6..8d01dd8a 100644
--- a/chrome/browser/ui/signin/signin_view_controller_interactive_uitest.cc
+++ b/chrome/browser/ui/signin/signin_view_controller_interactive_uitest.cc
@@ -42,6 +42,7 @@
 #include "components/signin/public/identity_manager/identity_manager.h"
 #include "components/signin/public/identity_manager/identity_test_utils.h"
 #include "components/sync/base/features.h"
+#include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
@@ -231,6 +232,14 @@
   EXPECT_TRUE(signin_view_controller->ShowsModalDialog());
   content_observer.Wait();
 
+  // Wait for the modal dialog sheet window to be presented and receive focus.
+  ASSERT_TRUE(base::test::RunUntil([&]() {
+    auto* web_contents =
+        signin_view_controller->GetModalDialogWebContentsForTesting();
+    return web_contents && web_contents->GetRenderWidgetHostView() &&
+           web_contents->GetRenderWidgetHostView()->HasFocus();
+  }));
+
   content::WebContentsDestroyedWatcher dialog_destroyed_watcher(
       signin_view_controller->GetModalDialogWebContentsForTesting());
diff --git a/chrome/browser/ui/test/test_browser_dialog.cc b/chrome/browser/ui/test/test_browser_dialog.cc
index 3ca4d2f..ef42317 100644
--- a/chrome/browser/ui/test/test_browser_dialog.cc
+++ b/chrome/browser/ui/test/test_browser_dialog.cc
@@ -28,6 +28,7 @@
 
 #if defined(TOOLKIT_VIEWS)
 #include "base/strings/strcat.h"
+#include "base/test/run_until.h"
 #include "third_party/abseil-cpp/absl/cleanup/cleanup.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
@@ -119,6 +120,13 @@
   }
 
   views::Widget* dialog_widget = *(added.begin());
+  if (ShouldWaitForDialogBeforeVerify()) {
+    if (!base::test::RunUntil([&]() { return dialog_widget->IsVisible(); })) {
+      LOG(ERROR) << "VerifyUi(): Widget failed to become visible.";
+      return false;
+    }
+  }
+
   dialog_widget->SetBlockCloseForTesting(true);
   // Deactivate before taking screenshot. Deactivated dialog pixel outputs
   // is more predictable than activated dialog.
@@ -200,6 +208,10 @@
   return false;
 }
 
+bool TestBrowserDialog::ShouldWaitForDialogBeforeVerify() {
+  return BUILDFLAG(IS_MAC);
+}
+
 std::string TestBrowserDialog::GetNonDialogName() {
   return std::string();
 }
diff --git a/chrome/browser/ui/test/test_browser_dialog.h b/chrome/browser/ui/test/test_browser_dialog.h
index 08da2610..80a3388c 100644
--- a/chrome/browser/ui/test/test_browser_dialog.h
+++ b/chrome/browser/ui/test/test_browser_dialog.h
@@ -65,6 +65,12 @@
   // Widget::CanClose() and DialogDelegate::Close().
   virtual bool AlwaysCloseAsynchronously();
 
+  // Whether to wait for the dialog widget to become visible in VerifyUi() by
+  // pumping the run loop. Subclasses that run blocking modal loops should
+  // return false to avoid deadlocks. Defaults to true on macOS to account for
+  // asynchronous modal sheet presentation, and false on other platforms.
+  virtual bool ShouldWaitForDialogBeforeVerify();
+
   // Get the name of a non-dialog window that should be included in testing.
   // VerifyUi() only considers dialog windows and windows with a matching name.
   virtual std::string GetNonDialogName();
diff --git a/chrome/browser/ui/views/policy/enterprise_startup_dialog_view_browsertest.cc b/chrome/browser/ui/views/policy/enterprise_startup_dialog_view_browsertest.cc
index dea0cc68..604f521 100644
--- a/chrome/browser/ui/views/policy/enterprise_startup_dialog_view_browsertest.cc
+++ b/chrome/browser/ui/views/policy/enterprise_startup_dialog_view_browsertest.cc
@@ -53,6 +53,10 @@
   }
 #endif
 
+  // EnterpriseStartupDialogView runs a blocking modal loop, so return false to
+  // avoid deadlocks when pumping the run loop in VerifyUi().
+  bool ShouldWaitForDialogBeforeVerify() override { return false; }
+
  private:
   raw_ptr<EnterpriseStartupDialogView, AcrossTasksDanglingUntriaged> dialog;
 };
diff --git a/chrome/browser/ui/views/relaunch_notification/relaunch_notification_controller_platform_impl_desktop_browsertest.cc b/chrome/browser/ui/views/relaunch_notification/relaunch_notification_controller_platform_impl_desktop_browsertest.cc
index f52abab..0cbee8be 100644
--- a/chrome/browser/ui/views/relaunch_notification/relaunch_notification_controller_platform_impl_desktop_browsertest.cc
+++ b/chrome/browser/ui/views/relaunch_notification/relaunch_notification_controller_platform_impl_desktop_browsertest.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/views/relaunch_notification/relaunch_notification_controller_platform_impl_desktop.h"
 
 #include "base/run_loop.h"
+#include "base/test/run_until.h"
 #include "base/time/time.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/test/base/in_process_browser_test.h"
@@ -49,13 +50,18 @@
   impl.NotifyRelaunchRequired(deadline,
                               /*is_notification_style_ap_required=*/false,
                               base::OnceCallback<base::Time()>());
+  ASSERT_TRUE(base::test::RunUntil(
+      [&]() { return impl.GetWidgetForTesting() != nullptr; }));
   views::test::WidgetDestroyedWaiter fourth_destroyed_waiter(
       impl.GetWidgetForTesting());
   impl.CloseRelaunchNotification();
   fourth_destroyed_waiter.Wait();
+
   impl.NotifyRelaunchRequired(deadline,
                               /*is_notification_style_ap_required=*/false,
                               base::OnceCallback<base::Time()>());
+  ASSERT_TRUE(base::test::RunUntil(
+      [&]() { return impl.GetWidgetForTesting() != nullptr; }));
   views::test::WidgetDestroyedWaiter fifth_destroyed_waiter(
       impl.GetWidgetForTesting());
   impl.CloseRelaunchNotification();
@@ -65,6 +71,8 @@
   impl.NotifyRelaunchRequired(deadline,
                               /*is_notification_style_ap_required=*/true,
                               base::OnceCallback<base::Time()>());
+  ASSERT_TRUE(base::test::RunUntil(
+      [&]() { return impl.GetWidgetForTesting() != nullptr; }));
   views::test::WidgetDestroyedWaiter sixth_destroyed_waiter(
       impl.GetWidgetForTesting());
   impl.CloseRelaunchNotification();
diff --git a/chrome/browser/ui/views/safe_browsing/tailored_security_desktop_dialog_manager_browsertest.cc b/chrome/browser/ui/views/safe_browsing/tailored_security_desktop_dialog_manager_browsertest.cc
index 0bdb675d..b1a4c64 100644
--- a/chrome/browser/ui/views/safe_browsing/tailored_security_desktop_dialog_manager_browsertest.cc
+++ b/chrome/browser/ui/views/safe_browsing/tailored_security_desktop_dialog_manager_browsertest.cc
@@ -7,6 +7,7 @@
 #include "base/command_line.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/metrics/user_action_tester.h"
+#include "base/test/run_until.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/test/test_browser_dialog.h"
@@ -21,6 +22,7 @@
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/scoped_animation_duration_scale_mode.h"
 #include "ui/views/bubble/bubble_dialog_delegate_view.h"
+#include "ui/views/test/widget_test.h"
 #include "ui/views/widget/any_widget_observer.h"
 #include "ui/views/widget/widget.h"
 #include "url/gurl.h"
@@ -106,7 +108,9 @@
         safe_browsing::kTailoredSecurityNoticeDialog);
     dialog_manager_->ShowEnabledDialogForBrowser(browser, base::DoNothing());
 
-    return waiter.WaitIfNeededAndGet();
+    views::Widget* widget = waiter.WaitIfNeededAndGet();
+    EXPECT_TRUE(base::test::RunUntil([&]() { return widget->IsVisible(); }));
+    return widget;
   }
 
   views::Widget* ShowTailoredSecurityDisabledDialog(Browser* browser) {
@@ -115,7 +119,9 @@
         safe_browsing::kTailoredSecurityNoticeDialog);
     dialog_manager_->ShowDisabledDialogForBrowser(browser, base::DoNothing());
 
-    return waiter.WaitIfNeededAndGet();
+    views::Widget* widget = waiter.WaitIfNeededAndGet();
+    EXPECT_TRUE(base::test::RunUntil([&]() { return widget->IsVisible(); }));
+    return widget;
   }
 
  private:
@@ -308,8 +314,9 @@
 IN_PROC_BROWSER_TEST_P(TailoredSecurityDesktopDialogManagerTest,
                        OpeningANewEnableDialogWillCloseAnyOpenDisableDialogs) {
   auto* disabled_dialog = ShowTailoredSecurityDisabledDialog(browser());
+  views::test::WidgetDestroyedWaiter destroyed_waiter(disabled_dialog);
   auto* enabled_dialog = ShowTailoredSecurityEnabledDialog(browser());
-  EXPECT_TRUE(disabled_dialog->IsClosed());
+  destroyed_waiter.Wait();
   EXPECT_FALSE(enabled_dialog->IsClosed());
 }
 
@@ -336,8 +343,9 @@
 IN_PROC_BROWSER_TEST_P(TailoredSecurityDesktopDialogManagerTest,
                        OpeningANewDisableDialogWillCloseAnyOpenEnableDialogs) {
   auto* enabled_dialog = ShowTailoredSecurityEnabledDialog(browser());
+  views::test::WidgetDestroyedWaiter destroyed_waiter(enabled_dialog);
   auto* disabled_dialog = ShowTailoredSecurityDisabledDialog(browser());
-  EXPECT_TRUE(enabled_dialog->IsClosed());
+  destroyed_waiter.Wait();
   EXPECT_FALSE(disabled_dialog->IsClosed());
 }
diff --git a/chrome/browser/ui/views/web_apps/protocol_handler_launch_dialog_browsertest.cc b/chrome/browser/ui/views/web_apps/protocol_handler_launch_dialog_browsertest.cc
index 4e874453..868700c 100644
--- a/chrome/browser/ui/views/web_apps/protocol_handler_launch_dialog_browsertest.cc
+++ b/chrome/browser/ui/views/web_apps/protocol_handler_launch_dialog_browsertest.cc
@@ -37,6 +37,7 @@
 #include "services/network/public/mojom/permissions_policy/permissions_policy_feature.mojom.h"
 #include "third_party/blink/public/common/features.h"
 #include "ui/views/test/dialog_test.h"
+#include "ui/views/test/widget_test.h"
 #include "ui/views/view.h"
 #include "ui/views/widget/any_widget_observer.h"
 #include "ui/views/widget/widget.h"
@@ -152,9 +153,19 @@
     webapps::AppId test_app_id = InstallTestWebApp(browser()->GetProfile());
     ShowWebAppProtocolLaunchDialog(protocol_url, browser()->GetProfile(),
                                    test_app_id, base::DoNothing());
-    waiter.WaitIfNeededAndGet()->CloseWithReason(
-        views::Widget::ClosedReason::kEscKeyPressed);
+    widget_ = waiter.WaitIfNeededAndGet()->GetWeakPtr();
   }
+
+  void DismissUi() override {
+    if (widget_) {
+      views::test::WidgetDestroyedWaiter waiter(widget_.get());
+      widget_->CloseWithReason(views::Widget::ClosedReason::kEscKeyPressed);
+      waiter.Wait();
+    }
+  }
+
+ private:
+  base::WeakPtr<views::Widget> widget_;
 };
 
 IN_PROC_BROWSER_TEST_F(
diff --git a/chrome/browser/ui/views/web_apps/web_app_update_review_dialog_browsertest.cc b/chrome/browser/ui/views/web_apps/web_app_update_review_dialog_browsertest.cc
index d65d724..ba35a3f 100644
--- a/chrome/browser/ui/views/web_apps/web_app_update_review_dialog_browsertest.cc
+++ b/chrome/browser/ui/views/web_apps/web_app_update_review_dialog_browsertest.cc
@@ -16,6 +16,7 @@
 #include "base/test/bind.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/metrics/user_action_tester.h"
+#include "base/test/run_until.h"
 #include "base/test/test_future.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time/time.h"
@@ -448,6 +449,8 @@
... (truncated)

Loading diff…

Original Bug Report

reported by [email protected]

Potential UAF via iterator invalidation in NativeWidgetNSWindowBridge::OrderChildren

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 without the Chrome Security team.

Overview: A potential Use-After-Free (UAF) exists in the macOS browser process due to iterator invalidation in NativeWidgetNSWindowBridge::OrderChildren. A synchronous nested run loop allows task execution that can mutate the child_windows_ vector during a range-based for loop iteration. Resuming the loop dereferences a dangling pointer, potentially leading to arbitrary code execution.

Affected files:

components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm
ui/views/cocoa/native_widget_mac_ns_window_host.mm
ui/views/widget/native_widget_mac.mm
components/remote_cocoa/app_shim/views_nswindow_delegate.mm

Estimated timestamp from git blame: 2023-07-07

Description

A potential Use-After-Free (UAF) vulnerability exists in the macOS implementation of NativeWidgetNSWindowBridge::OrderChildren.

When a window’s visibility changes (e.g., from an AppKit windowDidDeminiaturize: callback), OrderChildren iterates over the child_windows_ vector using a C++ range-based for loop (for (auto* child : child_windows_)). A range-based for loop caches the end iterator of the vector before the loop begins.

During iteration, if a child window is a modal sheet, the code calls child->ShowAsModalSheet(). For in-process Mac windows, this synchronously invokes the native macOS API -[NSWindow beginSheet:completionHandler:]. This native method spins a nested run loop to handle the sheet animation.

Because the call stack originated directly from a native OS AppKit callback and not from within a standard Chrome task loop, Chrome’s re-entrancy protection (task_execution_allowed_ being set to false in DoWorkImpl) is bypassed. The nested message pump will process pending Chrome application tasks.

If one of these pending tasks closes another child window attached to the same parent, that child window is deleted and erased from the child_windows_ vector. This vector mutation shrinks the logical size of the vector, invalidating the cached end iterator of the paused range-based for loop.

When the nested run loop exits and the iteration resumes, the loop reads past the new logical end of the vector, fetching a stale raw pointer from the vector’s capacity buffer. This dangling pointer points to the newly freed child window object. The loop then reads member variables from this freed object (child->wants_to_be_visible(), child->window_) and passes the attacker-controlled NSWindow* to [window addChildWindow:child_window ordered:NSWindowAbove], providing a strong primitive for Remote Code Execution (RCE) in the unsandboxed browser process.

Suggested Reproduction Steps

Note: These are suggested/potential steps to trigger the vulnerability. Our tooling agent does not yet have the ability to run code to verify them dynamically.

Open a standard browser window on macOS (ensuring an in-process NativeWidgetNSWindowBridge).
Trigger the creation of two child windows on this parent: a modal dialog (e.g., a certificate selector) and a standard bubble (e.g., a permission prompt).
Minimize the parent window. The modal sheet is queued to be shown but remains hidden because the parent is minimized.
Queue a malicious task or Mojo IPC message that will close the standard bubble when executed.
Restore (deminiaturize) the parent window.
The OS invokes windowDidDeminiaturize:, leading to OrderChildren.
OrderChildren processes the modal dialog and calls -[NSWindow beginSheet:], spinning the nested run loop.
The queued task executes during the nested loop, destroying the standard bubble and erasing it from child_windows_.
The nested loop returns, the range-based for loop resumes with an invalid iterator, and a UAF occurs.

Suggested Fix

Avoid iterating directly over the mutable child_windows_ vector if the loop body can spin a nested run loop.

Since NativeWidgetNSWindowBridge supports base::WeakPtr (via its factory_), a safe approach is to create a local vector of weak pointers to the children before iterating:

std::vector<base::WeakPtr<NativeWidgetNSWindowBridge>> children_copy;
children_copy.reserve(child_windows_.size());
for (auto* child : child_windows_) {
  children_copy.push_back(child->GetWeakPtr());
}

for (auto& weak_child : children_copy) {
  if (!weak_child)
    continue;
  // ... existing loop logic ...
}

This ensures that if a child is destroyed during the nested run loop, the weak pointer will safely evaluate to null when the iteration resumes.

Evaluated with Chrome root at commit: 096fc8fdbfacf2546485756d03f160a3d04fcc9b

Results so far have been promising, but there can be wrong deductions. If this proves to be a false positive, please close as WAI; 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

References

Vendor advisory Fix commit Bug tracker NVD

On This Page