CVE-2026-118310
Overview
Background
- `MenuRunnerImpl`
- the views-layer object (
ui/views/controls/menu/menu_runner_impl.cc) that owns a running menu’s state. It is reference-counted: callers hold references and it is torn down throughRelease(), not by directdelete. - `delete_after_run_`
- a flag meaning the runner should destroy itself once the menu finishes running, so its lifetime is tied to the menu session rather than an external owner.
- `MenuController::IsStackActive()` / `DeferMenuRunnerDestruction()`
- when the menu controller’s run loop is still on the stack, the runner cannot be freed immediately; ownership is transferred to the controller via
DeferMenuRunnerDestruction()so destruction is deferred until the stack unwinds. - `OnMenuClosed()`
- callback invoked when the menu closes; it must retire the runner, honoring both the refcount and the “defer while stack active” rule.
- `base::WeakFactory` / `InvalidateWeakPtrs()`
- the runner hands out weak pointers; they must be invalidated at the moment the object is logically considered deleted.
Root Cause Analysis
OnMenuClosed() contained a hand-rolled copy of the deferred-destruction logic: in the delete_after_run_ branch it computed is_stack_active and either called controller_->DeferMenuRunnerDestruction(base::WrapUnique(this)) or, otherwise, delete this. The bug is that this branch bypassed the object’s reference-counted lifetime. MenuRunnerImpl is retired through Release(), which is the single place that correctly reconciles the refcount with the IsStackActive()/DeferMenuRunnerDestruction() decision. Calling delete this (or wrapping this into a unique_ptr for the controller) directly from OnMenuClosed() destroyed the runner without consulting the reference count, so if any other reference was still outstanding the object was freed while live references remained — a premature free leading to a dangling pointer and subsequent use-after-free. (The body of Release() is not shown by this diff; that it performs the correct stack-active-aware teardown and refcount drop is inferred from the commit message “Delegate … to Release() instead of duplicating the IsStackActive()/DeferMenuRunnerDestruction() logic.”)
The patch removes the duplicated block and replaces it with running_ = false; Release();, routing the delete_after_run_ path through the same single owner-aware teardown used everywhere else. The first hunk is a cosmetic/comment cleanup (is_stack_active inlined, typo fixed) on the sibling stack-active path that still calls InvalidateWeakPtrs() and transfers ownership. Note the discrepancy: the advisory labels this Improper input validation in Views, but the diff shows no input parsing or validation — it is a reference-counting/lifetime defect (a duplicated destruction path that could double-free or free-while-referenced). The accurate class is a use-after-free arising from the lifetime-management error.
OnMenuClosed() destroyed a reference-counted MenuRunnerImpl directly instead of through Release(), so an outstanding reference could be left dangling — a premature free rather than any input-validation flaw.Attack Path
- Open a menu
User/content action triggers a views menu that runs with
delete_after_run_set, so the runner intends to self-destruct on close. (reachability of a specific attacker-controlled menu is inferred) - Hold a reference
A second owner/weak consumer still references the
MenuRunnerImplwhen the menu closes (e.g., a nested/stack-active controller interaction). (inferred) - Close the menu
OnMenuClosed()runs the olddelete_after_run_branch anddelete this(or defers aWrapUnique(this)), freeing the object without regard to the refcount. - Dangle The still-outstanding reference now points at freed memory.
- Primitive Subsequent dereference of the dangling runner yields a use-after-free in the browser process. (freed-memory/escalation inferred)
Impact Assessment
ui/views), where menus are driven by UI/content interactions. No elevated privilege is required beyond causing a menu to open and close in a state with an outstanding reference. The achieved primitive is a use-after-free of a MenuRunnerImpl; its exploitability strength and any escalation to code execution are (inferred) since the diff shows only the lifetime fix and not the dangling access site. Gating depends on hitting the stack-active / outstanding-reference window, which the dedup to Release() closes. This is a lifetime bug, not the advisory’s “input validation.”Changed Functions
| Function | Change | Notes |
|---|---|---|
ifui/views/controls/menu/menu_runner_impl.cc |
modified |
Files Changed
ui/views/controls/menu/menu_runner_impl.cc
Audit Directions
- Direct `delete this` on refcounted views objectsGrep
ui/viewsmenu/runner/controller code fordelete thisandWrapUnique(this)that bypassRelease(); each is a candidate for the same free-while-referenced bug. - Duplicated lifetime decisionsLook for other sites that re-implement
IsStackActive()/DeferMenuRunnerDestruction()instead of delegating; divergence between copies is how this bug arose. - WeakPtr invalidation timingVerify every teardown path that “considers the object deleted” calls
InvalidateWeakPtrs()at the right moment, consistent with the stack-active branch.
Patch
From 72bc9fdbbe985f6131d7bd0e3b6d84f6e592d6c4 Mon Sep 17 00:00:00 2001 From: Mitsuru Oshima <[email protected]> Date: Tue, 20 Oct 2026 00:04:04 -0700 Subject: [PATCH] views: Deduplicate deferred destruction in MenuRunnerImpl::OnMenuClosed Delegate the `delete_after_run_` destruction path in `MenuRunnerImpl::OnMenuClosed` to `Release()` instead of duplicating the `IsStackActive()` / `DeferMenuRunnerDestruction()` logic. Bug: 574516887 CONV=cced0104-bc4a-465a-9306-2ed8a06093b5 Change-Id: I059734bf9e8f34ebf4d2ed642f089a7edf246a0a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8613233 Reviewed-by: Thomas Lukaszewicz <[email protected]> Commit-Queue: Mitsuru Oshima <[email protected]> Cr-Commit-Position: refs/heads/main@{#1722638} --- diff --git a/ui/views/controls/menu/menu_runner_impl.cc b/ui/views/controls/menu/menu_runner_impl.cc index d0f01b2..631be02 100644 --- a/ui/views/controls/menu/menu_runner_impl.cc +++ b/ui/views/controls/menu/menu_runner_impl.cc @@ -110,9 +110,8 @@ } } - bool is_stack_active = controller_ && controller_->IsStackActive(); - if (is_stack_active) { - // Invlidate the menu runner's weakptrs now because the menu runner is + if (controller_ && controller_->IsStackActive()) { + // Invalidate the menu runner's weakptrs now because the menu runner is // considered deleted at this point. weak_factory_.InvalidateWeakPtrs(); // Transfer ownership to controller. @@ -263,14 +262,8 @@ if (delete_after_run_) { FireFocusAfterMenuClose(parent_widget); - - bool is_stack_active = controller_ && controller_->IsStackActive(); - if (is_stack_active) { - // Transfer ownership to controller. - controller_->DeferMenuRunnerDestruction(base::WrapUnique(this)); - } else { - delete this; - } + running_ = false; + Release(); return; } running_ = false;