CVE-2026-79150

Critical chrome UAF 📄 Reporter bug report 🔧 Commit mapped

Overview

Critical

Severity

—

CVSS

Exploited ITW

Fixed

Fix Status

ImpactUse after free in Views

DescriptionUse after free in Views

ComponentViews

Bug ClassUAF

Tracker518006007

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 in NativeWidgetNSWindowBridge::SetVisibilityState via synchronous sheet presentation

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 NativeWidgetNSWindowBridge::SetVisibilityState() on macOS. When showing a window-modal sheet, the bridge synchronously spins a nested event run-loop during which the window (and its associated bridge/host objects) can be destroyed. Upon return from the nested loop, the method dereferences and executes virtual functions on the freed objects without a WeakPtr guard.

Affected files:

components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm

Estimated timestamp from git blame: Unknown (Google3 checkout)

Summary

A potential Use-After-Free (UAF) vulnerability has been identified in NativeWidgetNSWindowBridge::SetVisibilityState() within the macOS-specific remote cocoa/views implementation (components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm). Under certain conditions, showing a window modal dialog (such as a JavaScript-initiated app-modal dialog like alert(), confirm(), or prompt()) triggers a synchronous presentation animation that runs a nested event run-loop. If the window or its parent is destroyed during this nested run-loop, the underlying NativeWidgetNSWindowBridge and NativeWidgetMacNSWindowHost instances are synchronously deleted. When the nested loop exits and the stack unwinds, the execution flow proceeds to dereference members and make virtual calls on these freed objects, leading to a potential UAF in the browser process.

Potential Root Cause and Control Flow

In components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm:

void NativeWidgetNSWindowBridge::SetVisibilityState(
    WindowVisibilityState new_state) {
  ...
  if (IsWindowModalSheet()) {
    ShowAsModalSheet();
    return;
  }
  ...
}

When a window-modal sheet is shown:

SetVisibilityState() calls ShowAsModalSheet() (line 997).
Because the window runs in-process in the browser, host_helper_->MustPostTaskToRunModalSheetAnimation() returns false, causing the block to run synchronously (line 2257):
```
[parent_window beginSheet:window_
        completionHandler:^(NSModalResponse return_code) { ... }];
```
This native AppKit call blocks the UI thread and synchronously runs a nested event run-loop to drive the presentation animation.
If a task or event to close the window is processed during this nested loop, the destruction sequence is executed: -[ViewsNSWindowDelegate windowWillClose:] -> _parent->OnWindowWillClose() -> host_->OnWindowHasClosed() -> NativeWidgetMac::WindowDestroyed() -> ns_window_host_.reset() -> ~NativeWidgetMacNSWindowHost() -> in_process_ns_window_bridge_.reset() -> delete this.

As documented in components/remote_cocoa/app_shim/views_nswindow_delegate.mm line 311:

  _parent->OnWindowWillClose();
  // |self| may be deleted here (it's NSObject, so who really knows).
  // |parent_| _will_ be deleted for sure.

And in native_widget_ns_window_bridge.mm line 1401:

  host_->OnWindowHasClosed();
  // Note: |this| and its host will be deleted here.

Once the nested loop exits, control returns to ShowAsModalSheet(), which immediately returns to SetVisibilityState(). At this point, the stack resumes execution. However, the this pointer (the bridge) and the host_ pointer have already been deleted. This results in a critical UAF dereference when the code subsequently calls ShouldRunCustomAnimationFor() or accesses host_->GetWidgetIsModal(). A similar UAF occurs in the host class caller NativeWidgetMacNSWindowHost::SetVisibilityState() at ui/views/cocoa/native_widget_mac_ns_window_host.mm:906 when calling GetWidget() on a deleted this pointer.

Note that our tooling agent does not have the ability to run functional exploit code, and these are potential/theoretical execution steps.

Suggested Fix

To prevent the UAF, a base::WeakPtr guard should be introduced to verify whether the bridge or host object has been destroyed during the synchronous nested loop execution. An identical lifetime guard pattern was previously introduced in FullscreenControllerTransitionComplete() (line 1643) and should be applied here:

In NativeWidgetNSWindowBridge::SetVisibilityState():

  base::WeakPtr<NativeWidgetNSWindowBridge> weak_ptr = factory_.GetWeakPtr();
  if (IsWindowModalSheet()) {
    ShowAsModalSheet();
    return;
  }
  if (!weak_ptr) {
    return;
  }

Additionally, a similar base::WeakPtr check should be performed inside NativeWidgetMacNSWindowHost::SetVisibilityState() after calling GetNSWindowMojo()->SetVisibilityState(new_state) to avoid dereferencing this when calling GetWidget() if the host has been destroyed.

Evaluated with Chrome root at commit: fb72408a8493c46bc75fae1c70d03daec96b3040

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

References

Vendor advisory Fix commit Bug tracker NVD

On This Page