CVE-2026-19147
Overview
Background
- `x11::GeometryCache`
- a per-window cache in
ui/gfx/xthat tracks an X11 window’s geometry and parent (via asyncGetGeometry/QueryTreerequests) and invokes a bounds-changed callback when the computed screen bounds change;X11Windowuses it to answer bounds queries. - `x11::Future` / `FutureImpl`
- a handle to a pending X11 request;
DispatchNow()forces the reply to be processed immediately, which runs the registered response handler (e.g.GeometryCache::OnQueryTreeResponse) synchronously. - `Sync()` vs new `Peek()`
Sync()blocks and takes the reply out of the connection’s request queue; the newPeek()blocks until the reply arrives and returns a copy without removing it or clearing its response callback, so the handler still runs later during normal dispatch.- Synchronous re-entrancy
- when a getter like
GetBoundsPx()runs callbacks, those callbacks can reachPlatformWindowDelegate::OnBoundsChangedand destroy the window (and its owners) while callers higher up the stack still hold rawthispointers.
Root Cause Analysis
Pre-patch, GeometryCache::GetBoundsPx() called parent_future_.DispatchNow() and geometry_future_.DispatchNow() when the parent or geometry was not yet cached, which synchronously ran OnQueryTreeResponse/geometry handlers and, through them, the bounds-changed callback into X11Window’s delegate. That delegate could free the window (the updated tests name the re-entry points: OnWindowMapped/Maximize, SetOverrideRedirect, and XI event dispatch to a grabbing window), so callers of GetBoundsPx() and GetBoundsInPixels() resumed on freed objects — a use-after-free in the browser’s Aura/X11 windowing layer. The removed weak_this checks inside GetBoundsPx() and in the hunk at line ~121 show an earlier workaround that only protected the GeometryCache itself, not the callers above it.
The fix removes the synchronous dispatch entirely: GetBoundsPx() now uses Future::Peek() to read the QueryTree and GetGeometry replies without consuming them or running their handlers, computing bounds from the peeked data (creating or resetting parent_ as needed), so the bounds-changed callbacks run only later during ordinary event dispatch, as the tests now check via connection->DispatchAll() and EXPECT_FALSE(last_bounds.has_value()) immediately after GetBoundsPx().
DispatchNow() ran reply handlers and change callbacks re-entrantly, letting arbitrary observers destroy windows mid-call; the fix makes GetBoundsPx() side-effect-free by peeking at replies instead of dispatching them.Attack Path
- Window state change triggers a bounds query
an
X11Windowoperation such as map/maximize,SetOverrideRedirect(true), or dispatching an XI input event to a grabbing window callsGetBoundsPx()while the parent or geometry is not yet cached (the scenarios inx11_window_ozone_unittest.cc). - Synchronous reply dispatch
pre-patch
GetBoundsPx()calledDispatchNow()onparent_future_/geometry_future_, runningOnQueryTreeResponseand the geometry handler inline. - Callback destroys the window
the handler fired the bounds-changed callback into
delegate->OnBoundsChanged(), whose observer frees theX11Window(the tests setfreed = trueinside that delegate callback). - Caller resumes on freed memory
control unwinds into
X11Windowor other callers that continue using the destroyed object, a browser-process use-after-free; how remote web content would reliably drive this window teardown is not shown in the patch(inferred: e.g. via popup/fullscreen/window-move flows).
Impact Assessment
GetBoundsPx(); exploitation for code execution in the unsandboxed browser process is (inferred) and not demonstrated by the patch.Changed Functions
| Function | Change | Notes |
|---|---|---|
ifui/gfx/x/future.h |
modified | |
ifui/gfx/x/geometry_cache.cc |
modified | |
TESTui/gfx/x/geometry_cache_unittest.cc |
modified | |
EXPECT_TRUEui/ozone/platform/x11/x11_window_ozone_unittest.cc |
modified |
Files Changed
ui/gfx/x/future.ccui/gfx/x/future.hui/gfx/x/geometry_cache.ccui/gfx/x/geometry_cache_unittest.ccui/ozone/platform/x11/x11_window_ozone_unittest.cc
Audit Directions
- Getters that pump eventssearch
ui/gfx/x,ui/ozone/platform/x11, andui/base/xforDispatchNow(),Sync(), or nested run loops inside query-style methods, and treat any that can reachPlatformWindowDelegatecallbacks as re-entrancy hazards. - Callee-side guards that miss the caller
WeakPtrchecks inside a helper (like the removed ones inGeometryCache) do not protect callers up the stack; audit sibling workarounds (the revertedX11DesktopWindowMoveClientandDesktopWindowTreeHostPlatform::CenterWindowguards) to confirm no remaining path re-enters. - Removed guards relying on a new invariantthis patch deletes
weak_thischecks on the assumption thatGetBoundsPx()never dispatches; verifyFutureImpl::Wait()and any future edits toGetBoundsPx()keep that invariant, and thatPeek()discarding errors does not yield stale bounds used for security-relevant decisions.
Patch
diff --git a/ui/gfx/x/future.cc b/ui/gfx/x/future.cc
index a0d2ec39..0524b22 100644
--- a/ui/gfx/x/future.cc
+++ b/ui/gfx/x/future.cc
@@ -36,6 +36,17 @@
TakeResponse(raw_reply, error);
}
+void FutureImpl::Peek(RawReply* raw_reply, std::unique_ptr<Error>* error) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(connection_->sequence_checker_);
+ Wait();
+
+ auto* request = connection_->GetRequestForFuture(this);
+ CHECK(request->have_response);
+
+ *raw_reply = request->reply;
+ *error = nullptr;
+}
+
void FutureImpl::OnResponse(ResponseCallback callback) {
UpdateRequestHandler(std::move(callback));
}
diff --git a/ui/gfx/x/future.h b/ui/gfx/x/future.h
index 2141a29..6be59a2 100644
--- a/ui/gfx/x/future.h
+++ b/ui/gfx/x/future.h
@@ -28,6 +28,10 @@
void Sync(RawReply* raw_reply, std::unique_ptr<Error>* error);
+ // Similar to Sync(), but does not remove the response from the connection's
+ // queue or clear the response callback.
+ void Peek(RawReply* raw_reply, std::unique_ptr<Error>* error);
+
void OnResponse(ResponseCallback callback);
// Update an existing Request with a new handler. |sequence| must
@@ -102,7 +106,8 @@
"to FutureBase");
}
- // Blocks until we receive the response from the server. Returns the response.
+ // Blocks until the response is received from the server. Returns the
+ // response, removing it from the request queue.
Response<Reply> Sync() {
if (!impl()) {
return {nullptr, nullptr};
@@ -114,7 +119,29 @@
std::unique_ptr<Reply> reply;
if (raw_reply) {
- auto buf = ReadBuffer(raw_reply);
+ ReadBuffer buf(raw_reply);
+ reply = detail::ReadReply<Reply>(&buf);
+ }
+
+ return {std::move(reply), std::move(error)};
+ }
+
+ // Blocks until the response is received from the server. Returns the response
+ // without taking it from the request queue or clearing the response callback.
+ // Unlike Sync(), this method does not remove the response from the request
+ // queue.
+ Response<Reply> Peek() {
+ if (!impl()) {
+ return {nullptr, nullptr};
+ }
+
+ RawReply raw_reply;
+ std::unique_ptr<Error> error;
+ impl()->Peek(&raw_reply, &error);
+
+ std::unique_ptr<Reply> reply;
+ if (raw_reply) {
+ ReadBuffer buf(raw_reply);
reply = detail::ReadReply<Reply>(&buf);
}
@@ -185,4 +212,4 @@
} // namespace x11
-#endif // UI_GFX_X_FUTURE_H_
+#endif // UI_GFX_X_FUTURE_H_
diff --git a/ui/gfx/x/geometry_cache.cc b/ui/gfx/x/geometry_cache.cc
index d7b2c05..a36c897e 100644
--- a/ui/gfx/x/geometry_cache.cc
+++ b/ui/gfx/x/geometry_cache.cc
@@ -34,31 +34,43 @@
GeometryCache::~GeometryCache() = default;
gfx::Rect GeometryCache::GetBoundsPx() {
- auto weak_this = weak_ptr_factory_.GetWeakPtr();
- if (!have_parent_) {
- parent_future_.DispatchNow();
- if (!weak_this) {
- return {};
+ auto get_local_geometry = [this]() {
+ if (have_geometry_) {
+ return geometry_;
}
- }
- CHECK(have_parent_);
- if (!have_geometry_) {
- geometry_future_.DispatchNow();
- if (!weak_this) {
- return {};
+ if (auto response = geometry_future_.Peek()) {
+ return response ? gfx::Rect(response->x, response->y, response->width,
+ response->height)
+ : gfx::Rect();
}
- }
- CHECK(have_geometry_);
-
- if (!parent_) {
return geometry_;
+ };
+
+ if (!have_parent_) {
+ auto response = parent_future_.Peek();
+ if (!response) {
+ return get_local_geometry();
+ }
+ Window parent_window = response ? response->parent : Window::None;
+ if (parent_window == Window::None) {
+ parent_.reset();
+ return get_local_geometry();
+ }
+ if (!parent_ || parent_->window_ != parent_window) {
+ parent_ = std::make_unique<GeometryCache>(
+ connection_, parent_window,
+ base::BindRepeating(&GeometryCache::OnParentGeometryChanged,
+ weak_ptr_factory_.GetWeakPtr()));
+ }
+ }
+
+ gfx::Rect geometry = get_local_geometry();
+ if (!parent_) {
+ return geometry;
}
auto parent_bounds = parent_->GetBoundsPx();
- if (!weak_this) {
- return {};
- }
gfx::Vector2d offset(parent_bounds.x(), parent_bounds.y());
- return geometry_ + offset;
+ return geometry + offset;
}
void GeometryCache::OnQueryTreeResponse(QueryTreeResponse response) {
@@ -121,12 +133,7 @@
return;
}
- auto weak_this = weak_ptr_factory_.GetWeakPtr();
auto geometry = GetBoundsPx();
- if (!weak_this) {
- return;
- }
-
if (last_notified_geometry_ == geometry) {
return;
}
diff --git a/ui/gfx/x/geometry_cache_unittest.cc b/ui/gfx/x/geometry_cache_unittest.cc
index 4e876ed2..7312e1b3 100644
--- a/ui/gfx/x/geometry_cache_unittest.cc
+++ b/ui/gfx/x/geometry_cache_unittest.cc
@@ -101,8 +101,11 @@
EXPECT_FALSE(last_bounds.has_value());
EXPECT_EQ(geometry_cache.GetBoundsPx(), bounds);
- // `last_bounds` should have gotten set after the call to GetBoundsPx().
- // Reset it for the next part of the test.
+ // Calling GetBoundsPx() should not trigger the callback synchronously.
+ EXPECT_FALSE(last_bounds.has_value());
+
+ // Dispatching responses/events will invoke the callback.
+ connection->DispatchAll();
EXPECT_TRUE(last_bounds.has_value());
last_bounds = std::nullopt;
@@ -176,9 +179,8 @@
connection, window.id(),
base::BindRepeating(bounds_changed_callback, &geometry_cache));
- // This will trigger the callback via DispatchNow() and then return.
- // If there's a UAF, this should crash or trigger ASAN.
geometry_cache->GetBoundsPx();
+ connection->DispatchAll();
}
TEST(GeometryCacheTest, DestroyInParentGeometryChangedCallback) {
diff --git a/ui/ozone/platform/x11/x11_window_ozone_unittest.cc b/ui/ozone/platform/x11/x11_window_ozone_unittest.cc
index b62a827..2a5b43ef 100644
--- a/ui/ozone/platform/x11/x11_window_ozone_unittest.cc
+++ b/ui/ozone/platform/x11/x11_window_ozone_unittest.cc
@@ -2,27 +2,27 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "ui/ozone/platform/x11/x11_window.h"
-#include "ui/platform_window/extensions/x11_extension_delegate.h"
-#include "base/nix/xdg_util.h"
-#include "ui/base/x/x11_util.h"
-#include "ui/gfx/x/atom_cache.h"
-
#include <memory>
#include <utility>
+#include "base/nix/xdg_util.h"
#include "base/run_loop.h"
+#include "base/test/run_until.h"
#include "base/test/task_environment.h"
#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/x/x11_util.h"
#include "ui/display/display.h"
#include "ui/display/screen_base.h"
#include "ui/events/devices/x11/touch_factory_x11.h"
#include "ui/events/event.h"
#include "ui/events/platform/x11/x11_event_source.h"
#include "ui/events/test/events_test_utils_x11.h"
+#include "ui/gfx/x/atom_cache.h"
#include "ui/gfx/x/event.h"
+#include "ui/ozone/platform/x11/x11_window.h"
#include "ui/ozone/platform/x11/x11_window_manager.h"
#include "ui/ozone/test/mock_platform_window_delegate.h"
+#include "ui/platform_window/extensions/x11_extension_delegate.h"
#include "ui/platform_window/platform_window_delegate.h"
#include "ui/platform_window/platform_window_init_properties.h"
@@ -481,10 +481,11 @@
x11::Event map_event(/*send_event=*/false, std::move(map_notify));
connection->DispatchEvent(map_event);
- // The synchronous-free path must have been exercised; otherwise the test
- // didn't reach the vulnerable frame.
- EXPECT_TRUE(freed) << "delegate->OnBoundsChanged() was not invoked "
- "synchronously from OnWindowMapped/Maximize";
+ EXPECT_TRUE(base::test::RunUntil([&]() {
+ connection->DispatchAll();
+ return freed;
+ })) << "delegate->OnBoundsChanged() was not invoked from "
+ "OnWindowMapped/Maximize";
connection->DestroyWindow({new_parent});
}
@@ -554,10 +555,10 @@
auto* x11_window_raw = static_cast<X11Window*>(window.get());
x11_window_raw->SetOverrideRedirect(true);
- // The synchronous-free path must have been exercised; otherwise the test
- // didn't reach the vulnerable frame.
- EXPECT_TRUE(freed) << "delegate->OnBoundsChanged() was not invoked "
- "synchronously from SetOverrideRedirect";
+ EXPECT_TRUE(base::test::RunUntil([&]() {
+ connection->DispatchAll();
+ return freed;
+ })) << "delegate->OnBoundsChanged() was not invoked from SetOverrideRedirect";
connection->DestroyWindow({new_parent});
}
@@ -635,9 +636,10 @@
// Dispatch the event to window2.
DispatchXEvent(xi_event, widget2);
- // Verify that window1 was indeed synchronously freed during the geometry
- // fetch.
- EXPECT_TRUE(freed) << "grabber delegate was never invoked synchronously";
+ EXPECT_TRUE(base::test::RunUntil([&]() {
+ connection->DispatchAll();
+ return freed;
+ })) << "grabber delegate was never invoked";
connection->DestroyWindow({new_parent});
}
@@ -712,9 +714,10 @@
// Step 7: Dispatch the event to widget. This will trigger DispatchEvent.
DispatchXEvent(xi_event, widget);
- // If we got here without ASAN/crash tripping, verify the window was indeed
- // freed.
- EXPECT_TRUE(freed) << "delegate was never invoked synchronously";
+ EXPECT_TRUE(base::test::RunUntil([&]() {
+ connection->DispatchAll();
+ return freed;
+ })) << "delegate was never invoked";
connection->DestroyWindow({new_parent});
}
Regression Test / PoC
diff --git a/ui/gfx/x/geometry_cache_unittest.cc b/ui/gfx/x/geometry_cache_unittest.cc
index 4e876ed2..7312e1b3 100644
--- a/ui/gfx/x/geometry_cache_unittest.cc
+++ b/ui/gfx/x/geometry_cache_unittest.cc
@@ -101,8 +101,11 @@
EXPECT_FALSE(last_bounds.has_value());
EXPECT_EQ(geometry_cache.GetBoundsPx(), bounds);
- // `last_bounds` should have gotten set after the call to GetBoundsPx().
- // Reset it for the next part of the test.
+ // Calling GetBoundsPx() should not trigger the callback synchronously.
+ EXPECT_FALSE(last_bounds.has_value());
+
+ // Dispatching responses/events will invoke the callback.
+ connection->DispatchAll();
EXPECT_TRUE(last_bounds.has_value());
last_bounds = std::nullopt;
@@ -176,9 +179,8 @@
connection, window.id(),
base::BindRepeating(bounds_changed_callback, &geometry_cache));
- // This will trigger the callback via DispatchNow() and then return.
- // If there's a UAF, this should crash or trigger ASAN.
geometry_cache->GetBoundsPx();
+ connection->DispatchAll();
}
TEST(GeometryCacheTest, DestroyInParentGeometryChangedCallback) {
diff --git a/ui/ozone/platform/x11/x11_window_ozone_unittest.cc b/ui/ozone/platform/x11/x11_window_ozone_unittest.cc
index b62a827..2a5b43ef 100644
--- a/ui/ozone/platform/x11/x11_window_ozone_unittest.cc
+++ b/ui/ozone/platform/x11/x11_window_ozone_unittest.cc
@@ -2,27 +2,27 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "ui/ozone/platform/x11/x11_window.h"
-#include "ui/platform_window/extensions/x11_extension_delegate.h"
-#include "base/nix/xdg_util.h"
-#include "ui/base/x/x11_util.h"
-#include "ui/gfx/x/atom_cache.h"
-
#include <memory>
#include <utility>
+#include "base/nix/xdg_util.h"
#include "base/run_loop.h"
+#include "base/test/run_until.h"
#include "base/test/task_environment.h"
#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/x/x11_util.h"
#include "ui/display/display.h"
#include "ui/display/screen_base.h"
#include "ui/events/devices/x11/touch_factory_x11.h"
#include "ui/events/event.h"
#include "ui/events/platform/x11/x11_event_source.h"
#include "ui/events/test/events_test_utils_x11.h"
+#include "ui/gfx/x/atom_cache.h"
#include "ui/gfx/x/event.h"
+#include "ui/ozone/platform/x11/x11_window.h"
#include "ui/ozone/platform/x11/x11_window_manager.h"
#include "ui/ozone/test/mock_platform_window_delegate.h"
+#include "ui/platform_window/extensions/x11_extension_delegate.h"
#include "ui/platform_window/platform_window_delegate.h"
#include "ui/platform_window/platform_window_init_properties.h"
@@ -481,10 +481,11 @@
x11::Event map_event(/*send_event=*/false, std::move(map_notify));
connection->DispatchEvent(map_event);
- // The synchronous-free path must have been exercised; otherwise the test
- // didn't reach the vulnerable frame.
- EXPECT_TRUE(freed) << "delegate->OnBoundsChanged() was not invoked "
- "synchronously from OnWindowMapped/Maximize";
+ EXPECT_TRUE(base::test::RunUntil([&]() {
+ connection->DispatchAll();
+ return freed;
+ })) << "delegate->OnBoundsChanged() was not invoked from "
+ "OnWindowMapped/Maximize";
connection->DestroyWindow({new_parent});
}
@@ -554,10 +555,10 @@
auto* x11_window_raw = static_cast<X11Window*>(window.get());
x11_window_raw->SetOverrideRedirect(true);
- // The synchronous-free path must have been exercised; otherwise the test
- // didn't reach the vulnerable frame.
- EXPECT_TRUE(freed) << "delegate->OnBoundsChanged() was not invoked "
- "synchronously from SetOverrideRedirect";
+ EXPECT_TRUE(base::test::RunUntil([&]() {
+ connection->DispatchAll();
+ return freed;
+ })) << "delegate->OnBoundsChanged() was not invoked from SetOverrideRedirect";
connection->DestroyWindow({new_parent});
}
@@ -635,9 +636,10 @@
// Dispatch the event to window2.
DispatchXEvent(xi_event, widget2);
- // Verify that window1 was indeed synchronously freed during the geometry
- // fetch.
- EXPECT_TRUE(freed) << "grabber delegate was never invoked synchronously";
+ EXPECT_TRUE(base::test::RunUntil([&]() {
+ connection->DispatchAll();
+ return freed;
+ })) << "grabber delegate was never invoked";
connection->DestroyWindow({new_parent});
}
@@ -712,9 +714,10 @@
// Step 7: Dispatch the event to widget. This will trigger DispatchEvent.
DispatchXEvent(xi_event, widget);
- // If we got here without ASAN/crash tripping, verify the window was indeed
- // freed.
- EXPECT_TRUE(freed) << "delegate was never invoked synchronously";
+ EXPECT_TRUE(base::test::RunUntil([&]() {
+ connection->DispatchAll();
+ return freed;
+ })) << "delegate was never invoked";
connection->DestroyWindow({new_parent});
}
Original Bug Report
Potential Browser-Process UAF in WindowTreeHost::MoveCursorToInternal on Linux/X11
Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.
Overview: A potential Use-After-Free (UAF) vulnerability exists in WindowTreeHost::MoveCursorToInternal in the Chromium browser process on Linux/X11. During a cursor movement operation, synchronous X11 message dispatching can trigger window bounds changes and subsequent observer destruction of the WindowTreeHost. Because WindowTreeHost::MoveCursorToInternal lacks a liveness check after returning from the cursor move call, it can access freed member variables, potentially leading to memory corruption.
Affected files:
ui/aura/window_tree_host.cc
Estimated timestamp from git blame: 2025-03-06
Root Cause Analysis
In ui/aura/window_tree_host.cc, the function WindowTreeHost::MoveCursorToInternal does not employ a liveness guard after calling MoveCursorToScreenLocationInPixels. On Linux/X11, this call can lead to synchronous window destruction while still on the call stack.
// ui/aura/window_tree_host.cc:798-809
void WindowTreeHost::MoveCursorToInternal(const gfx::Point& root_location,
const gfx::Point& host_location) {
last_cursor_request_position_in_host_ = host_location;
MoveCursorToScreenLocationInPixels(host_location);
// <--- Potential destruction of |this| can occur here synchronously
client::CursorClient* cursor_client = client::GetCursorClient(window()); // UAF
if (cursor_client) {
const display::Display& display =
display::Screen::Get()->GetDisplayNearestWindow(window());
cursor_client->SetDisplay(display);
}
dispatcher()->OnCursorMovedToRootLocation(root_location); // UAF
}
When MoveCursorToScreenLocationInPixels executes on X11, the execution flow is as follows:
WindowTreeHostPlatform::MoveCursorToScreenLocationInPixels(inui/aura/window_tree_host_platform.cc) is invoked.- This delegates to
platform_window_->MoveCursorTo(location_in_pixels), which is implemented byX11Window::MoveCursorTo(inui/ozone/platform/x11/x11_window.cc). - Inside
X11Window::MoveCursorTo, the code requests the current bounds viaGetBoundsInPixels(). - If the window’s geometry cache is not yet ready (such as in a newly opened window or right after a reparenting event),
geometry_cache_->GetBoundsPx()is called, which invokesDispatchNow()on a pending X11 future. DispatchNow()synchronously processes pending X server replies, triggering the registeredbounds_changed_callback_(X11Window::OnBoundsChanged).- This propagates upwards to
WindowTreeHostPlatform::OnBoundsChangedand callsOnHostMovedInPixels(), which notifies registeredWindowTreeHostObservers. - An observer (such as a transient bubble or anchor tracker) might synchronously destroy the window/widget via
Widget::CloseNow(). This results in the synchronous deletion of theWindowTreeHostinstance (host_.reset()). - When the synchronous execution stack unwinds,
X11Window::MoveCursorTosafely returns because of its localweak_thischeck. However, control returns toWindowTreeHost::MoveCursorToInternal, which has no liveness guard. It proceeds to dereferencethis->window_andthis->dispatcher_, leading to a Use-After-Free.
Potential Impact
This is a potential Use-After-Free (UAF) in the unsandboxed browser process. Since the dangling pointer is this, MiraclePtr (raw_ptr) does not mitigate this access because the member accesses (window_ and dispatcher_) are loads from the freed WindowTreeHost allocation itself.
If an attacker can reoccupy the freed allocation using heap-spraying techniques during the observer execution phase, they could potentially control the returned Window* and subsequently hijack the indirect virtual call to cursor_client->SetDisplay() or the virtual dispatch inside dispatcher(). This could lead to arbitrary code execution in the context of the browser process.
Suggested Potential Trigger Steps
Note: These are suggested and analytical execution steps. Our tooling does not currently have the capability to execute live browser runs or generate functional PoCs.
- From a compromised renderer, trigger a Pointer Lock request via Mojo IPC (
blink.mojom.WidgetInputHandlerHost::RequestMouseLock) on an X11 window whose geometry cache is not yet initialized (e.g., a freshly opened popup window). - The browser process grants the lock and triggers
MoveCursorToCenterwhich propagates toWindowTreeHost::MoveCursorToInternal. - The synchronous X11 message dispatch causes the geometry cache to execute its response handler synchronously.
- If a custom or transient window observer reacts to the bounds change by synchronously invoking
CloseNow(), theWindowTreeHostis deleted. - The memory is sprayed to reoccupy the slot, and control returns to
MoveCursorToInternalwhere the stalethispointer is dereferenced.
Suggested Fix
To prevent this vulnerability, a base::WeakPtr liveness check should be introduced in WindowTreeHost::MoveCursorToInternal immediately following the call to MoveCursorToScreenLocationInPixels.
void WindowTreeHost::MoveCursorToInternal(const gfx::Point& root_location,
const gfx::Point& host_location) {
last_cursor_request_position_in_host_ = host_location;
base::WeakPtr<WindowTreeHost> weak_this = GetWeakPtr();
MoveCursorToScreenLocationInPixels(host_location);
if (!weak_this) {
return;
}
client::CursorClient* cursor_client = client::GetCursorClient(window());
if (cursor_client) {
const display::Display& display =
display::Screen::Get()->GetDisplayNearestWindow(window());
cursor_client->SetDisplay(display);
}
dispatcher()->OnCursorMovedToRootLocation(root_location);
}
Evaluated with Chrome root at commit: 70c6813870b6701fa16670076bf633ee6c3a439f
Results so far have been promising, but there can be wrong deductions. Feel free to adjust as follows:
- If you are familiar with the severity guidelines, you may adjust the severity.
- If this is a false positive, and there’s no work to be done, please close as WAI.
- If there is work to do here but not a vulnerability, please change the issue type to Task/Bug/FR.
Data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.