diff --git a/ui/views/controls/menu/menu_runner_unittest.cc b/ui/views/controls/menu/menu_runner_unittest.cc index 53c32625fab9a..df1979515a82b 100644 --- a/ui/views/controls/menu/menu_runner_unittest.cc +++ b/ui/views/controls/menu/menu_runner_unittest.cc @@ -37,6 +37,7 @@ #include "ui/views/test/menu_test_utils.h" #include "ui/views/test/test_views.h" #include "ui/views/test/views_test_base.h" +#include "ui/views/widget/any_widget_observer.h" #include "ui/views/widget/native_widget_private.h" #include "ui/views/widget/widget.h" #include "ui/views/widget/widget_delegate.h" @@ -678,6 +679,62 @@ class MenuRunnerImplTest : public MenuRunnerTest { ~MenuRunnerImplTest() override = default; }; +// POC: MenuController::OpenMenuImpl dereferences `item` and writes +// `this->showing_submenu_` after ShowAt() without a liveness guard. +// +// On macOS, Widget::ShowInactive() can synchronously dispatch focus-change +// events that destroy the running menu (see menu_host.cc comment at the +// `destroying_` early-return). This test simulates that reentrancy on all +// platforms by hooking AnyWidgetObserver::set_shown_callback, which fires +// synchronously from Widget::HandleShowRequested() inside ShowInactive(). +// +// From the callback we call MenuRunnerImpl::Release(), which sets +// delete_after_run_=true and calls MenuController::Cancel(kDestroyed). Cancel +// reaches ExitMenu() -> MenuRunnerImpl::OnMenuClosed(), which: +// - `delete controller_.get()` -> frees the MenuController whose +// OpenMenuImpl frame is still on the stack (`this` dangles) +// - `delete this` -> frees MenuRunnerImpl and its owned +// `std::unique_ptr menu_`, freeing `item` +// +// SubmenuView::ShowAt's WeakPtr early-return then unwinds back to OpenMenuImpl, +// which proceeds to dereference the freed `item` (line ~2536) and write +// `showing_submenu_ = false` on the freed MenuController (line ~2555). +// +// Under ASAN this produces heap-use-after-free. +TEST_F(MenuRunnerImplTest, ReleaseDuringShowAtCausesUAFInOpenMenuImpl) { + internal::MenuRunnerImpl* menu_runner = + new internal::MenuRunnerImpl(CreateMenuItemView()); + + bool fired = false; + AnyWidgetObserver observer{views::test::AnyWidgetTestPasskey{}}; + observer.set_shown_callback( + base::BindLambdaForTesting([&](views::Widget* widget) { + // Only fire for the MenuHost widget, and only once. + if (fired || widget->GetName() != "MenuHost") { + return; + } + fired = true; + // The MenuItemView tree is about to be freed; drop the test's raw + // pointer to avoid the dangling-pointer detector firing on it. + ResetMenuItemView(); + // Simulate the macOS focus-change path: the MenuRunner owner is + // destroyed while the menu is showing, calling Release(). + menu_runner->Release(); + // At this point MenuController, MenuRunnerImpl, and the entire + // MenuItemView tree have been freed, but OpenMenuImpl's stack frame + // is still live and will dereference them on return. + })); + + // RunMenuAt -> MenuController::Run -> SetSelection -> + // CommitPendingSelection -> OpenMenu -> OpenMenuImpl -> ShowAt -> + // ShowInactive -> [callback above fires synchronously]. + menu_runner->RunMenuAt(owner(), nullptr, gfx::Rect(), + MenuAnchorPosition::kTopLeft, + ui::mojom::MenuSourceType::kNone, 0); + + EXPECT_TRUE(fired); +} + // Tests that when nested menu runners are destroyed out of order, that // MenuController is not accessed after it has been destroyed. This should not // crash on ASAN bots.