CVE-2026-95351
Overview
Background
- `FocusManager`
- The per-
Widgetobject inui/viewsthat tracks whichViewcurrently has focus and drives keyboard focus traversal. - `AdvanceFocus`
FocusManager::AdvanceFocusmoves focus to the next or previous focusableViewduring Tab/Shift-Tab traversal.- `ViewTracker`
- A helper that holds a
Viewpointer as aViewObserverand automatically nulls it (operator boolbecomesfalse) when thatViewis destroyed. - Re-entrant callbacks
- Hooks such as
View::AboutToRequestFocusFromTabTraversalandFocusChangeListener::OnWillChangeFocusrun arbitrary client code that can synchronously deleteViewobjects mid-operation.
Root Cause Analysis
FocusManager::AdvanceFocus computed the next focusable view as a raw pointer View* v = GetNextFocusableView(...) and then held that pointer across several re-entrant callbacks. Between obtaining v and finally dereferencing it, the code called v->AboutToRequestFocusFromTabTraversal(reverse) and SetFocusedViewWithReason (which fires FocusChangeListener::OnWillChangeFocus), any of which can synchronously destroy the very View that v points to. Because v was a plain pointer, the subsequent dereferences (v->GetWidget(), v->GetWidget()->GetFocusManager(), v->GetWidget()->Activate()) operated on freed memory, violating the invariant that a pointer used after a re-entrant callback must be revalidated. Similarly, SetFocusedViewWithReason assigned focused_view_ = view after notifying listeners, even though OnWillChangeFocus could already have freed view.
The fix wraps v (and the view argument in SetFocusedViewWithReason) in a ViewTracker and inserts if (!v) return; guards after each callback, so a destroyed view is detected and the code bails out instead of dereferencing a dangling pointer.
View* across re-entrant focus callbacks that are allowed to delete that view; the fix converts the cached pointer to a ViewTracker weak reference and re-checks its validity (!v) after every callback before dereferencing.Attack Path
- Register a malicious focus observer
Content or a page adds a
FocusChangeListener(or aViewoverridingAboutToRequestFocusFromTabTraversal) whose callback destroys the view that is about to receive focus. - Trigger tab traversal
The victim presses Tab (or script drives focus), invoking
FocusManager::AdvanceFocus, which selects the next view into the raw pointerv. - Free the tracked view during the callback
Inside
v->AboutToRequestFocusFromTabTraversalor theOnWillChangeFocusnotification, the attacker-controlled callback deletes that view (the regression test does this viaRemoveChildViewT(next)), freeing its memory. - Use-after-free on the dangling pointer
AdvanceFocuscontinues to dereferencev(e.g.v->GetWidget()->GetFocusManager()), reading and calling through freedView/Widgetmemory. - Exploitation
(inferred)Reclaiming the freedViewallocation with attacker-controlled data before the dereference could turn the stale vtable/Widgetaccess into a controlled call, but the patch itself only demonstrates the crash, not a concrete read/write primitive.
Impact Assessment
Views UI can free a View during focus traversal and cause FocusManager::AdvanceFocus to operate on freed memory, yielding a use-after-free in the browser process where ui/views runs. Preconditions are the ability to install or trigger a focus callback that deletes the pending focus view and to drive a Tab-traversal focus change. The vendor advisory rates this high severity and does not report in-the-wild exploitation; NVD scores it CVSS 8.3. The advisory classifies it as a UAF in Views and states no exploited-in-the-wild status or severity beyond that.Changed Functions
| Function | Change | Notes |
|---|---|---|
ifui/views/focus/focus_manager.cc |
modified | |
MockFocusChangeListenerui/views/focus/focus_manager_unittest.cc |
modified |
Files Changed
ui/views/focus/focus_manager.ccui/views/focus/focus_manager_unittest.cc
Audit Directions
- Raw pointers held across re-entrant callbacksAudit
ui/viewscode that caches aView*/Widget*and then calls listener notifications,AboutToRequestFocusFromTabTraversal, or other client hooks before dereferencing, and convert such pointers toViewTracker. - Focus and observer notification sinksReview
FocusChangeListener,ViewObserver, andWidgetObserverdispatch sites (.Notify(...)) for state read or written after the callback that the callback could have invalidated. - Post-callback field assignmentsLook for assignments like
member_ = arg;that occur after notifying observers, whereargmay have been freed during the notification and should be re-fetched through a tracked reference.
Patch
From 8b5549467725a5e444c8a45c2d6f131c9fc0ca73 Mon Sep 17 00:00:00 2001 From: Dana Fried <[email protected]> Date: Thu, 17 Sep 2026 10:24:44 -0700 Subject: [PATCH] [Views] Harden FocusManager against callbacks during AdvanceFocus Now if any callback happens during AdvanceFocus that would cause the next view to be deleted, it should be handled gracefully. Fixed: 562242429 Change-Id: Ia0c69027d491e9264369814110973ebe1389f84f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8426759 Commit-Queue: Dana Fried <[email protected]> Reviewed-by: Mickey Burks <[email protected]> Cr-Commit-Position: refs/heads/main@{#1699939} --- diff --git a/ui/views/focus/focus_manager.cc b/ui/views/focus/focus_manager.cc index a56fa4f..0d549a09 100644 --- a/ui/views/focus/focus_manager.cc +++ b/ui/views/focus/focus_manager.cc @@ -156,34 +156,39 @@ } void FocusManager::AdvanceFocus(bool reverse) { - View* v = GetNextFocusableView(focused_view_, nullptr, reverse, false); + ViewTracker v(GetNextFocusableView(focused_view_, nullptr, reverse, false)); // Note: Do not skip this next block when v == focused_view_. If the user // tabs past the last focusable element in a webpage, we'll get here, and if // the TabContentsContainerView is the only focusable view (possible in // fullscreen mode), we need to run this block in order to cycle around to the // first element on the page. - if (v) { - views::View* focused_view = focused_view_; - v->AboutToRequestFocusFromTabTraversal(reverse); - // AboutToRequestFocusFromTabTraversal() may have changed focus. If it did, - // don't change focus again. - if (focused_view != focused_view_) { - return; - } + if (!v) { + return; + } - // Note that GetNextFocusableView may have returned a View in a different - // FocusManager. - DCHECK(v->GetWidget()); - v->GetWidget()->GetFocusManager()->SetFocusedViewWithReason( - v, FocusChangeReason::kFocusTraversal); + views::View* const focused_view = focused_view_; + v.view()->AboutToRequestFocusFromTabTraversal(reverse); + // AboutToRequestFocusFromTabTraversal() may have changed focus. If it did, + // don't change focus again. + if (!v || focused_view != focused_view_) { + return; + } - // When moving focus from a child widget to a top-level widget, - // the top-level widget may report IsActive()==true because it's - // active even though it isn't focused. Explicitly activate the - // widget to ensure that case is handled. - if (v->GetWidget()->GetFocusManager() != this) { - v->GetWidget()->Activate(); - } + // Note that GetNextFocusableView may have returned a View in a different + // FocusManager. + DCHECK(v.view()->GetWidget()); + v.view()->GetWidget()->GetFocusManager()->SetFocusedViewWithReason( + v.view(), FocusChangeReason::kFocusTraversal); + if (!v) { + return; + } + + // When moving focus from a child widget to a top-level widget, + // the top-level widget may report IsActive()==true because it's + // active even though it isn't focused. Explicitly activate the + // widget to ensure that case is handled. + if (v.view()->GetWidget()->GetFocusManager() != this) { + v.view()->GetWidget()->Activate(); } } @@ -350,12 +355,13 @@ // Update the reason for the focus change (since this is checked by // some listeners), then notify all listeners. focus_change_reason_ = reason; + ViewTracker view_tracker(view); focus_change_listeners_.Notify(&FocusChangeListener::OnWillChangeFocus, focused_view_, view); // Actions below like `Blur()` can destroy `focused_view_`. ViewTracker old_focused_view_tracker(focused_view_); - focused_view_ = view; + focused_view_ = view_tracker.view(); base::AutoReset<int> entrance_count_resetter( &setting_focused_view_entrance_count_, setting_focused_view_entrance_count_ + 1); diff --git a/ui/views/focus/focus_manager_unittest.cc b/ui/views/focus/focus_manager_unittest.cc index 0b2a6c64..c24d50de 100644 --- a/ui/views/focus/focus_manager_unittest.cc +++ b/ui/views/focus/focus_manager_unittest.cc @@ -20,6 +20,7 @@ #include "base/test/icu_test_util.h" #include "base/test/run_until.h" #include "build/build_config.h" +#include "testing/gmock/include/gmock/gmock.h" #include "ui/base/accelerators/accelerator.h" #include "ui/base/accelerators/test_accelerator_target.h" #include "ui/base/metadata/metadata_header_macros.h" @@ -1582,4 +1583,72 @@ #endif +// Regression tests for crashes during callbacks. + +namespace { + +class MockFocusChangeListener : public FocusChangeListener { + public: + MockFocusChangeListener() = default; + ~MockFocusChangeListener() override = default; + + MOCK_METHOD(void, OnWillChangeFocus, (View*, View*), (override)); + MOCK_METHOD(void, OnDidChangeFocus, (View*, View*), (override)); +}; + +} // namespace + +TEST_F(FocusManagerTest, + ShouldNotCrashIfViewDestroyedDuringAdvance_WillChangeFocus) { + MockFocusChangeListener listener; + + auto event_list = base::MakeRefCounted<FocusTestEventList>(); + const int kView1ID = 1; + const int kView2ID = 2; + ViewTracker view1(GetContentsView()->AddChildView( + std::make_unique<SimpleTestView>(event_list, kView1ID))); + ViewTracker view2(GetContentsView()->AddChildView( + std::make_unique<SimpleTestView>(event_list, kView2ID))); + view1.view()->SetFocusBehavior(View::FocusBehavior::ALWAYS); + view2.view()->SetFocusBehavior(View::FocusBehavior::ALWAYS); + view1.view()->RequestFocus(); + GetFocusManager()->AddFocusChangeListener(&listener); + EXPECT_CALL(listener, OnWillChangeFocus) + .WillOnce([this](View* prev, View* next) { + GetContentsView()->RemoveChildViewT(next); + }); + GetFocusManager()->AdvanceFocus(false); + EXPECT_FALSE(view2); + + GetFocusManager()->ClearFocus(); + GetFocusManager()->RemoveFocusChangeListener(&listener); +} + +TEST_F(FocusManagerTest, + ShouldNotCrashIfViewDestroyedDuringAdvance_DidChangeFocus) { + MockFocusChangeListener listener; + + auto event_list = base::MakeRefCounted<FocusTestEventList>(); + const int kView1ID = 1; + const int kView2ID = 2; + ViewTracker view1(GetContentsView()->AddChildView( + std::make_unique<SimpleTestView>(event_list, kView1ID))); + ViewTracker view2(GetContentsView()->AddChildView( + std::make_unique<SimpleTestView>(event_list, kView2ID))); + view1.view()->SetFocusBehavior(View::FocusBehavior::ALWAYS); + view2.view()->SetFocusBehavior(View::FocusBehavior::ALWAYS); + view1.view()->RequestFocus(); + GetFocusManager()->AddFocusChangeListener(&listener); + EXPECT_CALL(listener, OnDidChangeFocus) + .WillOnce([this](View* prev, View* next) { + GetContentsView()->RemoveChildViewT(next); + }); + EXPECT_CALL(listener, OnDidChangeFocus(testing::_, nullptr)); + GetFocusManager()->AdvanceFocus(false); + EXPECT_FALSE(view2); + + GetFocusManager()->ClearFocus(); + GetFocusManager()->RemoveFocusChangeListener(&listener); +} + } // namespace views
Regression Test / PoC
diff --git a/ui/views/focus/focus_manager_unittest.cc b/ui/views/focus/focus_manager_unittest.cc
index 0b2a6c64..c24d50de 100644
--- a/ui/views/focus/focus_manager_unittest.cc
+++ b/ui/views/focus/focus_manager_unittest.cc
@@ -20,6 +20,7 @@
#include "base/test/icu_test_util.h"
#include "base/test/run_until.h"
#include "build/build_config.h"
+#include "testing/gmock/include/gmock/gmock.h"
#include "ui/base/accelerators/accelerator.h"
#include "ui/base/accelerators/test_accelerator_target.h"
#include "ui/base/metadata/metadata_header_macros.h"
@@ -1582,4 +1583,72 @@
#endif
+// Regression tests for crashes during callbacks.
+
+namespace {
+
+class MockFocusChangeListener : public FocusChangeListener {
+ public:
+ MockFocusChangeListener() = default;
+ ~MockFocusChangeListener() override = default;
+
+ MOCK_METHOD(void, OnWillChangeFocus, (View*, View*), (override));
+ MOCK_METHOD(void, OnDidChangeFocus, (View*, View*), (override));
+};
+
+} // namespace
+
+TEST_F(FocusManagerTest,
+ ShouldNotCrashIfViewDestroyedDuringAdvance_WillChangeFocus) {
+ MockFocusChangeListener listener;
+
+ auto event_list = base::MakeRefCounted<FocusTestEventList>();
+ const int kView1ID = 1;
+ const int kView2ID = 2;
+ ViewTracker view1(GetContentsView()->AddChildView(
+ std::make_unique<SimpleTestView>(event_list, kView1ID)));
+ ViewTracker view2(GetContentsView()->AddChildView(
+ std::make_unique<SimpleTestView>(event_list, kView2ID)));
+ view1.view()->SetFocusBehavior(View::FocusBehavior::ALWAYS);
+ view2.view()->SetFocusBehavior(View::FocusBehavior::ALWAYS);
+ view1.view()->RequestFocus();
+ GetFocusManager()->AddFocusChangeListener(&listener);
+ EXPECT_CALL(listener, OnWillChangeFocus)
+ .WillOnce([this](View* prev, View* next) {
+ GetContentsView()->RemoveChildViewT(next);
+ });
+ GetFocusManager()->AdvanceFocus(false);
+ EXPECT_FALSE(view2);
+
+ GetFocusManager()->ClearFocus();
+ GetFocusManager()->RemoveFocusChangeListener(&listener);
+}
+
+TEST_F(FocusManagerTest,
+ ShouldNotCrashIfViewDestroyedDuringAdvance_DidChangeFocus) {
+ MockFocusChangeListener listener;
+
+ auto event_list = base::MakeRefCounted<FocusTestEventList>();
+ const int kView1ID = 1;
+ const int kView2ID = 2;
+ ViewTracker view1(GetContentsView()->AddChildView(
+ std::make_unique<SimpleTestView>(event_list, kView1ID)));
+ ViewTracker view2(GetContentsView()->AddChildView(
+ std::make_unique<SimpleTestView>(event_list, kView2ID)));
+ view1.view()->SetFocusBehavior(View::FocusBehavior::ALWAYS);
+ view2.view()->SetFocusBehavior(View::FocusBehavior::ALWAYS);
+ view1.view()->RequestFocus();
+ GetFocusManager()->AddFocusChangeListener(&listener);
+ EXPECT_CALL(listener, OnDidChangeFocus)
+ .WillOnce([this](View* prev, View* next) {
+ GetContentsView()->RemoveChildViewT(next);
+ });
+ EXPECT_CALL(listener, OnDidChangeFocus(testing::_, nullptr));
+ GetFocusManager()->AdvanceFocus(false);
+ EXPECT_FALSE(view2);
+
+ GetFocusManager()->ClearFocus();
+ GetFocusManager()->RemoveFocusChangeListener(&listener);
+}
+
} // namespace views