From b25a691b8f65ace523de7ea33ce8a1fbe88d48c7 Mon Sep 17 00:00:00 2001
From: Stephen Nusko <[email protected]>
Date: Thu, 14 May 2026 17:49:40 -0700
Subject: [PATCH] Keep MenuItemView as a raw_ptr to prevent unsafe dangling

Previously a delegate could delete this and that could leave a
MenuItemView being freed. Normally stored as a raw_ptr, this defensive
copy wasn't kept as a raw_ptr so lost the ref count protection. This
protects `result` similar to the `this_ref` WeakPtr.

DanglingUntriaged-notes: Preventing MiraclePtr protection drop.
Bug: 506482786
Change-Id: Ifc68b46f160f5de0588318af3ac7b48b97672b1f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7847592
Auto-Submit: Stephen Nusko <[email protected]>
Reviewed-by: Dana Fried <[email protected]>
Commit-Queue: Stephen Nusko <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1630955}
---

diff --git a/ui/views/controls/menu/menu_controller.cc b/ui/views/controls/menu/menu_controller.cc
index 30ac16b8..ce5504b1 100644
--- a/ui/views/controls/menu/menu_controller.cc
+++ b/ui/views/controls/menu/menu_controller.cc
@@ -3556,17 +3556,23 @@
   // ExitTopMostMenu unwinds nested delegates
   internal::MenuControllerDelegate* delegate = delegate_;
   int accept_event_flags = accept_event_flags_;
+  // Since |delegate| may delete this, get a weak pointer first, and ensure
+  // |result| is safe from deletion (it can be freed but will be quarantined).
   base::WeakPtr<MenuController> this_ref = AsWeakPtr();
-  MenuItemView* result = ExitTopMostMenu();
+  // Dangling since a lot of tests in `views_unittests` and
+  // `interactive_ui_tests` detect this (likely correctly) as a dangling
+  // pointer.
+  raw_ptr<MenuItemView, DanglingUntriaged> result =
+      ExitTopMostMenu().ExtractAsDangling();
   delegate->OnMenuClosed(internal::MenuControllerDelegate::NOTIFY_DELEGATE,
-                         result, accept_event_flags);
+                         result.get(), accept_event_flags);
   // |delegate| may have deleted this.
   if (this_ref && nested && exit_type_ == ExitType::kAll) {
     ExitMenu();
   }
 }
 
-MenuItemView* MenuController::ExitTopMostMenu() {
+raw_ptr<MenuItemView> MenuController::ExitTopMostMenu() {
   // Release the lock which prevents Chrome from shutting down while the menu is
   // showing.
   base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
@@ -3622,8 +3628,9 @@
     did_capture_ = false;
   }
 
-  MenuItemView* result = result_;
-  // In case we're nested, reset |result_|.
+  // In case we're nested, reset |result_|, but use a raw_ptr to ensure we keep
+  // UaF protection.
+  raw_ptr<MenuItemView> result = result_;
   result_ = nullptr;
 
   if (exit_type_ == ExitType::kOutermost) {
diff --git a/ui/views/controls/menu/menu_controller.h b/ui/views/controls/menu/menu_controller.h
index fd62a4c..1e7dd4c 100644
--- a/ui/views/controls/menu/menu_controller.h
+++ b/ui/views/controls/menu/menu_controller.h
@@ -629,7 +629,7 @@
 
   // Performs the teardown of the menu launched by Run(). The selected item is
   // returned.
-  MenuItemView* ExitTopMostMenu();
+  raw_ptr<MenuItemView> ExitTopMostMenu();
 
   // Handles the mouse location event on the submenu |source|.
   void HandleMouseLocation(SubmenuView* source,