Medium watch CVSS 9.6 chrome UAF 📄 Reporter bug report 🔧 Commit mapped

Overview

Medium
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.
Tracker502109333
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-04-13 (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 (macOS)
an AppKit dialog attached to a parent window via -[NSWindow beginSheet:completionHandler:], whose presentation animation runs a nested run loop.
Nested run loop
a re-entrant event loop AppKit spins during the sheet animation, during which the main thread keeps processing events (including window-close events) while the presenting call is still on the stack.
`NativeWidgetNSWindowBridge`
the remote_cocoa C++ bridge object owning a native window; it can be destroyed if its widget is closed, invalidating this for any code still executing in its member functions.
`factory_.GetWeakPtr()` / `weak_this`
a base::WeakPtr to the bridge used to detect whether the bridge has been destroyed before dereferencing it after re-entrant work.

Root Cause Analysis

Sheet presentation entered AppKit’s animation nested run loop synchronously: pre-patch, NativeWidgetNSWindowBridge only posted the beginSheet: closure to the task runner when host_helper_->MustPostTaskToRunModalSheetAnimation() was true (remote cocoa), and otherwise called std::move(begin_sheet_closure).Run() inline. For in-process windows the inline path meant the nested run loop ran on the current stack; if the parent or child widget was closed during that loop, the C++ bridge (this) was synchronously destroyed, and execution resuming in the bridge’s member functions after the loop dereferenced freed memory — a use-after-free.

The fix always posts the sheet-animation task to the message loop (removing the MustPostTaskToRunModalSheetAnimation distinction entirely) so the animation runs on a fresh stack, and captures weak_this = factory_.GetWeakPtr(): the posted task bails if !bridge or !wants_to_be_visible_, and the completion handler re-checks weak_this before touching wants_to_be_visible_ or calling OnWindowWillClose(). Because presentation is now asynchronous, the previously necessary weak-pointer guard in the fullscreen-transition-complete path (which called OrderChildren() that could synchronously run ShowAsModalSheet) was removed as no longer needed.

Key insight
The single mistake was running the modal sheet’s nested run loop synchronously on the caller’s stack, so a widget close during the loop freed the bridge and resumed in its methods (UAF); the fix always posts the animation to run asynchronously on a fresh stack and guards every post-loop dereference of the bridge with a base::WeakPtr.

Attack Path

  1. Present an in-process modal sheet A dialog (e.g. bookmark editor, extension/settings dialog, signin sheet) is shown as a modal sheet on macOS, invoking the sheet-show path in NativeWidgetNSWindowBridge.
  2. Nested run loop spins synchronously (pre-patch) For an in-process window MustPostTaskToRunModalSheetAnimation() returned false, so begin_sheet_closure ran inline and AppKit entered its animation nested run loop with the bridge still on the stack.
  3. Widget closed during the animation While the nested loop processes events, the parent or child widget is closed (programmatically or via user/timing), synchronously destroying the NativeWidgetNSWindowBridge.
  4. Use-after-free on loop exit When the nested run loop returns, execution resumes in the (now-freed) bridge’s member functions / completion handler, dereferencing freed memory; controlling reallocated memory to escalate this to code execution is (inferred) and not shown by the patch.

Impact Assessment

An attacker or adverse timing that closes a widget while a macOS modal sheet is animating can trigger a use-after-free of NativeWidgetNSWindowBridge in the browser/app-shim process, potentially leading to a crash or memory corruption. The precondition is a modal sheet presentation whose owning widget is destroyed during the presentation’s nested run loop, on macOS. The vendor advisory rates this medium severity and does not report in-the-wild exploitation; NVD scores it CVSS 9.6. Escalation beyond the memory-safety violation itself (e.g. code execution) is (inferred).

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/signin/signin_view_controller_interactive_uitest.cc
  • chrome/browser/ui/views/policy/enterprise_startup_dialog_view_browsertest.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

  • Nested run loops on `this`'s stack
    Audit code that enters AppKit nested run loops (beginSheet:, runModal*, blocking animations) while an owning C++ object is on the stack, and dereferences that object after the loop returns; prefer posting to a fresh stack plus WeakPtr guards.
  • Re-entrancy after synchronous UI calls
    Review UI paths that synchronously invoke platform code capable of pumping events (OrderChildren, focus changes, modal presentation) and then continue using member state, ensuring destruction during re-entry is handled.
  • Platform-conditional synchronous shortcuts
    Look for boolean “must post task” style flags that only defer work in some configurations (e.g. remote vs in-process), leaving the in-process path synchronously re-entrant and prone to UAF.
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 app_shim via implicit `this` capture in ShowAsModalSheet

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 unsandboxed app_shim process. An Objective-C block in NativeWidgetNSWindowBridge::ShowAsModalSheet implicitly captures a raw this pointer and is posted to the task runner. If the Mojo pipe is disconnected before the block executes, the bridge is destroyed, leading to a UAF that could potentially be exploited for Remote Code Execution.

Affected files:

  • components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm
  • components/remote_cocoa/app_shim/application_bridge.mm
  • components/remote_cocoa/app_shim/native_widget_ns_window_bridge.h

Estimated timestamp from git blame: 2024-03-28

Summary

A potential Use-After-Free (UAF) vulnerability exists in the macOS app_shim process within the NativeWidgetNSWindowBridge::ShowAsModalSheet method. The vulnerability arises because an Objective-C block implicitly captures the raw C++ this pointer by value. When the block is posted to a task runner and delayed, a race condition can allow the NativeWidgetNSWindowBridge instance to be deleted before the block executes, leading to a UAF.

Because the app_shim process on macOS operates without a sandbox (switches::kAppShimProcessType maps to Sandbox::kNoSandbox), successful exploitation of this memory corruption could yield full user privileges (Remote Code Execution).

Technical Details

In components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm, the ShowAsModalSheet() method prepares an Objective-C block (begin_sheet_closure) to display a modal sheet:

auto begin_sheet_closure = base::BindOnce(^{
  [parent_window beginSheet:window_
          completionHandler:^(NSModalResponse return_code) {
            // ...
            wants_to_be_visible_ = false;
            OnWindowWillClose();
          }];
});

Inside this block, the code accesses the class member variable window_ and within the nested completionHandler, it accesses wants_to_be_visible_ and calls the member function OnWindowWillClose(). In Objective-C++, referencing member variables or functions inside a block implicitly captures the C++ this pointer by value as a raw pointer.

When running in an out-of-process App Shim (e.g., a PWA), host_helper_->MustPostTaskToRunModalSheetAnimation() returns true (see NativeWidgetBridgeOwner in application_bridge.mm). Consequently, ShowAsModalSheet posts begin_sheet_closure to the UI thread’s task runner rather than executing it synchronously.

Potential Exploitation Steps

Note: These are suggested steps; our tooling agent does not have the ability to run code to verify an end-to-end exploit.

  1. An attacker compromises a PWA renderer process or creates a malicious PWA.
  2. The attacker’s JavaScript triggers an action that causes the browser to show a window-modal dialog (e.g., navigator.subApps.add()).
  3. The browser process sends a SetVisibilityState(kShow) Mojo message to the app_shim process.
  4. Immediately after triggering the modal, the attacker cancels the dialog or closes the parent window, causing the browser to close the mojom::NativeWidgetNSWindow Mojo pipe.
  5. The app_shim’s IO thread receives both the visibility message and the pipe disconnection notification sequentially, posting them to the UI thread’s task queue.
  6. The UI thread executes the visibility task, routing to ShowAsModalSheet(). This creates the block capturing the raw this pointer and appends it as a new task to the end of the UI thread’s queue.
  7. The UI thread then executes the Mojo disconnect task. This calls NativeWidgetBridgeOwner::OnMojoDisconnect(), which executes delete this;, destroying the NativeWidgetNSWindowBridge and freeing its memory.
  8. The attacker sprays the heap via other IPC messages, aiming to reclaim the freed NativeWidgetNSWindowBridge memory with attacker-controlled data.
  9. The UI thread finally executes the delayed begin_sheet_closure task. The block dereferences the implicitly captured, now-freed this pointer.
  10. If the memory was successfully reclaimed, the attacker-controlled data is passed to [NSWindow beginSheet:]. When AppKit attempts to send an Objective-C message to this corrupted object, it jumps to an attacker-controlled address, leading to RCE in the unsandboxed app_shim process.

Since the captured this pointer is a raw C++ pointer and not explicitly a base::raw_ptr<T>, MiraclePtr protection does not mitigate this vulnerability.

Suggested Fix

To prevent the UAF, use base::WeakPtr to safely capture a reference to the NativeWidgetNSWindowBridge instance inside the Objective-C block. If the object is destroyed before the block executes, the weak pointer will be safely nullified.

base::WeakPtr<NativeWidgetNSWindowBridge> weak_this = factory_.GetWeakPtr();
auto begin_sheet_closure = base::BindOnce(^{
  if (!weak_this) {
    return;
  }
  [parent_window beginSheet:weak_this->window_
          completionHandler:^(NSModalResponse return_code) {
            if (!weak_this) {
              return;
            }
            weak_this->wants_to_be_visible_ = false;
            weak_this->OnWindowWillClose();
          }];
});

(Note: factory_ must be added to NativeWidgetNSWindowBridge if it does not already exist).

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