Overview

Critical
Severity
9.6
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactUse after free in Views
DescriptionUse after free in Views
ComponentViews
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.
Tracker518006007
Fix commitc763f3b4837e (chromium/src) +174/-100
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-05-29 (per vendor advisory)
Fix landed2026-07-21 (public git)
Disclosed2026-08-25
Patch gap35 days — fix public in source before the advisory

Background

Modal sheet
a macOS window presented attached to a parent via AppKit’s -beginSheet:completionHandler:, whose slide-in animation runs a nested run loop that keeps pumping the main-thread message loop while it executes.
`NativeWidgetNSWindowBridge`
the app-shim C++ object (referenced in the patch as this, obtained through weak_this = factory_.GetWeakPtr()) that owns and drives an NSWindow and its delegate on the browser/app-shim side.
`MustPostTaskToRunModalSheetAnimation()`
a host-helper predicate that previously decided whether the sheet animation had to be deferred onto a fresh stack (returned true for remote-cocoa, false for in-process windows via NativeWidgetMacNSWindowHost).
Nested-run-loop reentrancy
because the sheet animation pumps the message loop synchronously, other posted tasks and events (including window/widget close) run inside it, so an object alive when the loop is entered may be destroyed before it returns.

Root Cause Analysis

Before the patch, the sheet-presentation path (the function that builds begin_sheet_closure and calls [parent_window beginSheet:window_ ...], inferred NativeWidgetNSWindowBridge::ShowAsModalSheet) branched on host_helper_->MustPostTaskToRunModalSheetAnimation(): for remote cocoa it posted the closure, but for in-process windows it ran it synchronously via std::move(begin_sheet_closure).Run(). That synchronous beginSheet: call entered AppKit’s nested run loop while this was still on the stack; if the parent or child widget was closed during the animation, the NativeWidgetNSWindowBridge bridge was destroyed inside the loop, and when the loop returned, execution resumed in member functions of the freed bridge — a use-after-free.

The fix removes the MustPostTaskToRunModalSheetAnimation() distinction entirely and always posts the presentation task with base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(...), so the nested loop is entered on a fresh stack that holds no live this pointer. Inside the posted task and the completion handler, every access to the bridge is now re-validated through the weak_this WeakPtr (bridge = weak_this.get(); if (!bridge ...) return; and the if (weak_this) guards around wants_to_be_visible_ and OnWindowWillClose()), so a bridge destroyed during the animation is detected rather than dereferenced. This works because the vulnerable object is no longer captured by a raw stack reference across a reentrant run loop.

Key insight
The single mistake was running AppKit’s reentrant modal-sheet animation synchronously for in-process windows, leaving the raw NativeWidgetNSWindowBridge* this on the stack across a nested run loop that could destroy it; the fix makes presentation unconditionally asynchronous and gates every post-loop access behind a WeakPtr (weak_this).

Attack Path

  1. Trigger a modal sheet on an in-process window Cause a browser dialog that is shown as a macOS modal sheet (e.g. bookmark editor, sign-in modal, extension/settings dialogs — the regression tests exercise exactly these), so the in-process path takes std::move(begin_sheet_closure).Run() and synchronously enters beginSheet:’s nested run loop with this on the stack.
  2. Close the parent/child widget during the animation While the nested run loop is pumping, drive an event or task that closes the widget/window (the added browser_window_mac_browsertest.mm test does this with editor_raw->GetWidget()->CloseNow()), which synchronously destroys the NativeWidgetNSWindowBridge.
  3. Return into the freed bridge When the nested run loop exits, control resumes in the presentation function and the sheet completion handler, which touch member state (wants_to_be_visible_) and call member functions (OnWindowWillClose(), [window orderOut:nil]) on the now-freed bridge, producing the use-after-free.
  4. Escalation to controlled read/write or code execution (inferred) The advisory classifies this as a UAF in Views but the diff does not demonstrate any specific reclaimed-allocation reuse, read/write primitive, or code-execution sink.

Impact Assessment

An attacker who can drive UI that triggers a macOS modal sheet and concurrently close the owning widget gains a use-after-free of the NativeWidgetNSWindowBridge in the browser (or app-shim) process on macOS. Preconditions are platform IS_MAC and timing control over widget teardown during the sheet animation’s nested run loop; the corruption occurs when execution returns from that loop into freed bridge members. The vendor advisory rates this critical severity and does not report in-the-wild exploitation; NVD scores it CVSS 9.6.

Changed Functions

FunctionChangeNotes
if
components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm
modified
ASSERT_TRUE
chrome/browser/ui/signin/signin_view_controller_interactive_uitest.cc
modified
if
chrome/browser/ui/test/test_browser_dialog.cc
modified

Files Changed

  • ui/views/widget/native_widget_mac_unittest.mm
  • chrome/browser/ui/test/test_browser_dialog.cc
  • chrome/browser/ui/views/safe_browsing/tailored_security_desktop_dialog_manager_browsertest.cc
  • components/remote_cocoa/app_shim/application_bridge.mm
  • components/remote_cocoa/app_shim/native_widget_ns_window_host_helper.h
  • chrome/browser/ui/views/policy/enterprise_startup_dialog_view_browsertest.cc
  • chrome/browser/ui/signin/signin_view_controller_interactive_uitest.cc
  • components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm
  • chrome/browser/ui/extensions/settings_overridden_dialog_browsertest.cc
  • chrome/browser/ui/views/web_dialog_view_interactive_uitest.cc
  • chrome/browser/ui/cocoa/browser_window_mac_browsertest.mm
  • chrome/browser/ui/views/web_apps/web_app_update_review_dialog_browsertest.cc
  • chrome/browser/ui/views/relaunch_notification/relaunch_notification_controller_platform_impl_desktop_browsertest.cc
  • ui/views/cocoa/native_widget_mac_ns_window_host.mm
  • chrome/browser/ui/test/test_browser_dialog.h
  • ui/views/widget/widget_interactive_uitest.cc
  • ui/views/cocoa/native_widget_mac_ns_window_host.h
  • chrome/browser/ui/views/web_apps/protocol_handler_launch_dialog_browsertest.cc

Audit Directions

  • Synchronous nested run loops holding raw `this`
    Audit every AppKit call that enters a nested run loop (beginSheet:, runModalForWindow:, NSMenu tracking, drag loops) reached from a C++ bridge member; if this or member state is touched after the call returns without a re-validating WeakPtr, it is a reentrancy UAF candidate.
  • Platform-conditional "post vs run inline" branches
    Look for other MustPostTask...-style predicates or if (condition) PostTask else closure.Run() patterns where one branch runs reentrant work synchronously; the inline branch is where the object can die under the caller’s stack.
  • Post-nested-loop member access in completion handlers
    Review block/completion-handler callbacks that capture a bridge and dereference it or call its methods after a modal/animation loop; each such access needs a weak-pointer check like the added if (weak_this) guards, not a raw capture.
From c763f3b4837ea55058610ecf628cd140ef40b52e Mon Sep 17 00:00:00 2001
From: Bryan Oltman <bryanoltman@google.com>
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 <bryanoltman@google.com>
> > > > Reviewed-by: Keren Zhu <kerenzhu@chromium.org>
> > > > 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 <kdlee@chromium.org>
> > > Commit-Queue: Kalvin Lee <kdlee@chromium.org>
> > > Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
> > > 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 <kerenzhu@chromium.org>
> > Reviewed-by: Avi Drissman <avi@chromium.org>
> > Commit-Queue: Bryan Oltman <bryanoltman@google.com>
> > 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 <elkurin@chromium.org>
> Auto-Submit: Eriko Kurimoto <elkurin@chromium.org>
> Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
> 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 <kerenzhu@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Bryan Oltman <bryanoltman@google.com>
Cr-Commit-Position: refs/heads/main@{#1665550}
---
diff --git a/components/remote_cocoa/app_shim/application_bridge.mm b/components/remote_cocoa/app_shim/application_bridge.mm
index d5afd4b..af1a802 100644
--- a/components/remote_cocoa/app_shim/application_bridge.mm
+++ b/components/remote_cocoa/app_shim/application_bridge.mm
@@ -96,7 +96,6 @@
     // Text input doesn't work across mojo yet.
     return nullptr;
   }
-  bool MustPostTaskToRunModalSheetAnimation() const override { return true; }
 
   mojo::AssociatedRemote<mojom::NativeWidgetNSWindowHost> host_remote_;
   mojo::AssociatedRemote<mojom::TextInputHost> text_input_host_remote_;
diff --git a/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm b/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm
index 955e295..26132796 100644
--- a/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm
+++ b/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm
@@ -1648,14 +1648,7 @@
   UpdateWindowDisplay();
 
   // Add any children that were skipped during the fullscreen transition.
-  // A weak pointer is needed because OrderChildren() can synchronously run a
-  // modal sheet animation (ShowAsModalSheet), entering a nested run-loop during
-  // which this bridge may be destroyed.
-  base::WeakPtr<NativeWidgetNSWindowBridge> weak_ptr = factory_.GetWeakPtr();
   OrderChildren();
-  if (!weak_ptr) {
-    return;
-  }
 
   host_->OnWindowFullscreenTransitionComplete(is_fullscreen);
   if (is_fullscreen && immersive_mode_controller_) {
@@ -2228,42 +2221,47 @@
     return;
   }
 
-  auto begin_sheet_closure = base::BindOnce(^{
-    [parent_window beginSheet:window_
-            completionHandler:^(NSModalResponse return_code) {
-              // This class, NativeWidgetNSWindowBridge, clears the window's
-              // delegate as an indication of its death, in which case this
-              // completion handler will no-op. This is necessary to handle
-              // AppKit invoking this selector via a posted task. See
-              // https://crbug.com/851376.
-              NSWindow* window = weak_window;
-              if (!window.delegate) {
-                return;
-              }
-              // Make sure to mark ourselves as not wanting to be visible.
-              // Otherwise if during the orderOut call our parent becomes the
-              // key window, it would try to show us as a new modal sheet.
-              wants_to_be_visible_ = false;
-              [window orderOut:nil];
-              // Notify that the sheet window is closing.
-              OnWindowWillClose();
-            }];
-  });
-
-  if (host_helper_->MustPostTaskToRunModalSheetAnimation()) {
-    // This function is called via mojo when using remote cocoa. Inside the
-    // nested run loop, we will wait for a message providing the correctly-sized
-    // frame for the new sheet. This message will not be processed until we
-    // return from handling this message, because it will coming on the same
-    // pipe. Avoid the resulting hang by posting a task to show the modal
-    // sheet (which will be executed on a fresh stack, which will not block
-    // the message).
-    // https://crbug.com/1234509
-    base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
-        FROM_HERE, std::move(begin_sheet_closure));
-  } else {
-    std::move(begin_sheet_closure).Run();
-  }
+  base::WeakPtr<NativeWidgetNSWindowBridge> weak_this = factory_.GetWeakPtr();
+  // AppKit's sheet presentation animation runs a nested run loop. We post
+  // this task to run the animation asynchronously for two reasons:
+  // 1. For remote cocoa (out-of-process), it prevents a deadlock by allowing
+  //    the sizing message to be processed before entering the nested run loop.
+  // 2. For in-process windows, it prevents Use-After-Free (UAF) bugs and
+  //    undefined behavior when returning from the nested run loop if the
+  //    window was closed during presentation, causing its bridge (this) to be
+  //    destroyed.
+  // https://crbug.com/40781530, https://crbug.com/517040438,
+  // https://crbug.com/518006007
+  base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
+      FROM_HERE, base::BindOnce(^{
+        NativeWidgetNSWindowBridge* bridge = weak_this.get();
+        if (!bridge || !bridge->wants_to_be_visible_) {
+          return;
+        }
+        [parent_window beginSheet:bridge->ns_window()
+                completionHandler:^(NSModalResponse return_code) {
+                  // This class, NativeWidgetNSWindowBridge, clears the window's
+                  // delegate as an indication of its death, in which case this
+                  // completion handler will no-op. This is necessary to handle
+                  // AppKit invoking this selector via a posted task. See
+                  // https://crbug.com/41393772
+                  NSWindow* window = weak_window;
+                  if (!window.delegate) {
+                    return;
+                  }
+                  // Make sure to mark ourselves as not wanting to be visible.
+                  // Otherwise if during the orderOut call our parent becomes
+                  // the key window, it would try to show us as a new modal
+                  // sheet.
+                  if (weak_this) {
+                    weak_this->wants_to_be_visible_ = false;
+                  }
+                  [window orderOut:nil];
+                  if (weak_this) {
+                    weak_this->OnWindowWillClose();
+                  }
+                }];
+      }));
 }
 
 }  // namespace remote_cocoa
diff --git a/components/remote_cocoa/app_shim/native_widget_ns_window_host_helper.h b/components/remote_cocoa/app_shim/native_widget_ns_window_host_helper.h
index 298afa0..0b0dd04f 100644
--- a/components/remote_cocoa/app_shim/native_widget_ns_window_host_helper.h
+++ b/components/remote_cocoa/app_shim/native_widget_ns_window_host_helper.h
@@ -60,10 +60,6 @@
   // TODO(ccameron): Remove the needs for this call.
   virtual ui::TextInputClient* GetTextInputClient() = 0;
 
-  // Return whether or not the nested run loop to animate in a sheet must be
-  // made on a fresh stack, produced by posting a task to make the call.
-  // https://crbug.com/1234509
-  virtual bool MustPostTaskToRunModalSheetAnimation() const = 0;
 };
 
 }  // namespace remote_cocoa
diff --git a/ui/views/cocoa/native_widget_mac_ns_window_host.h b/ui/views/cocoa/native_widget_mac_ns_window_host.h
index d3dcbb57..9e748f5 100644
--- a/ui/views/cocoa/native_widget_mac_ns_window_host.h
+++ b/ui/views/cocoa/native_widget_mac_ns_window_host.h
@@ -330,7 +330,6 @@
                  gfx::Point* baseline_point) override;
   remote_cocoa::DragDropClient* GetDragDropClient() override;
   ui::TextInputClient* GetTextInputClient() override;
-  bool MustPostTaskToRunModalSheetAnimation() const override;
 
   // remote_cocoa::ApplicationHost::Observer:
   void OnApplicationHostDestroying(
diff --git a/ui/views/cocoa/native_widget_mac_ns_window_host.mm b/ui/views/cocoa/native_widget_mac_ns_window_host.mm
index 63428cd..6c60734 100644
--- a/ui/views/cocoa/native_widget_mac_ns_window_host.mm
+++ b/ui/views/cocoa/native_widget_mac_ns_window_host.mm
@@ -1068,10 +1068,6 @@
   return text_input_host_->GetTextInputClient();
 }
 
-bool NativeWidgetMacNSWindowHost::MustPostTaskToRunModalSheetAnimation() const {
-  return false;
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // NativeWidgetMacNSWindowHost, remote_cocoa::ApplicationHost::Observer:
 void NativeWidgetMacNSWindowHost::OnApplicationHostDestroying(
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
... (file diff truncated)
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 vm...@google.com

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:

  1. SetVisibilityState() calls ShowAsModalSheet() (line 997).
  2. 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) { ... }];
    
  3. This native AppKit call blocks the UI thread and synchronously runs a nested event run-loop to drive the presentation animation.
  4. 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.
  1. 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