diff --git a/ui/views/controls/menu/menu_controller_unittest.cc b/ui/views/controls/menu/menu_controller_unittest.cc index bbe9564040284..fab5b9b557bef 100644 --- a/ui/views/controls/menu/menu_controller_unittest.cc +++ b/ui/views/controls/menu/menu_controller_unittest.cc @@ -21,6 +21,7 @@ #include "base/strings/utf_string_conversions.h" #include "base/task/current_thread.h" #include "base/task/single_thread_task_runner.h" +#include "base/test/bind.h" #include "build/build_config.h" #include "ui/accessibility/ax_action_data.h" #include "ui/accessibility/ax_mode.h" @@ -3043,6 +3044,70 @@ TEST_F(MenuControllerTest, NoUseAfterFreeWhenMenuCanceledOnMousePress) { submenu->Close(); } +// Demonstrates that MenuController::OnMousePressed calls SetHotTrackedButton +// (a documented potential destroyer of `this` via accessibility re-entrancy) +// BEFORE capturing its self-WeakPtr guard. If an accessibility observer closes +// the menu in response to the kSelection event fired at the end of +// SetHotTrackedButton, OnMousePressed continues executing on a freed `this`. +// Under ASAN this test produces heap-use-after-free in +// MenuController::OnMousePressed (the AsWeakPtr() read of freed +// weak_ptr_factory_). +TEST_F(MenuControllerTest, OnMousePressedDestroyedDuringSetHotTrackedButton) { + AddButtonMenuItems(/*single_child=*/false); + SubmenuView* const submenu = menu_item()->GetSubmenu(); + + const View* const buttons_view = submenu->children()[4]; + ASSERT_NE(nullptr, buttons_view); + GET_CHILD_BUTTON(button1, buttons_view, 0); + GET_CHILD_BUTTON(button2, buttons_view, 1); + + // Ensure hot_button_ is set to button1 so that mouse-pressing button2 enters + // the `hot_button_ != button` branch and calls SetHotTrackedButton(button2). + SetHotTrackedButton(button1); + ASSERT_EQ(button1, hot_button()); + + // Simulate an assistive technology (the production scenario documented at + // menu_controller.cc SetSelection() and observed in field crashes) that + // synchronously closes the menu when it receives the kSelection event fired + // at the very end of SetHotTrackedButton. Because this is the LAST callout + // in SetHotTrackedButton, the callee returns cleanly and the first invalid + // access lands in the OnMousePressed caller frame. + bool destroyed = false; + button2->GetViewAccessibility().set_accessibility_events_callback( + base::BindLambdaForTesting( + [&](const ui::AXPlatformNodeDelegate*, ax::mojom::Event event) { + if (event == ax::mojom::Event::kSelection && !destroyed) { + destroyed = true; + // Mirrors MenuRunnerImpl::OnMenuClosed: `delete controller_`. + DestroyMenuController(); + } + })); + + // Mouse-press on button2. Control flow inside OnMousePressed: + // SetHotTrackedButton(button2) + // -> NotifyAccessibilityEventDeprecated(kSelection) + // -> callback above -> DestroyMenuController() -> `this` freed + // AsWeakPtr() <-- UAF read of freed weak_ptr_factory_ + // forward_to_root->ProcessMousePressed(...) + // current_mouse_event_target_ = ... <-- UAF write into freed *this + const gfx::Point location = View::ConvertPointToTarget( + button2, submenu, button2->GetLocalBounds().CenterPoint()); + ProcessMousePressed( + submenu, + ui::MouseEvent(ui::EventType::kMousePressed, location, location, + ui::EventTimeForNow(), ui::EF_LEFT_MOUSE_BUTTON, + ui::EF_LEFT_MOUSE_BUTTON)); + + // If we reach here, the WeakPtr guard was effective (bug fixed). + EXPECT_TRUE(destroyed); + + // Cleanup: clear the callback (it captures stack locals) and close the + // submenu host to remove observers before TearDown. + button2->GetViewAccessibility().set_accessibility_events_callback( + ViewAccessibility::AccessibilityEventsCallback()); + submenu->Close(); +} + TEST_F(MenuControllerTest, SetSelectionIndices_MenuItemsOnly) { SubmenuView* const submenu = menu_item()->GetSubmenu(); MenuItemView* const item1 = submenu->GetMenuItemAt(0);