Commit da8ef10a authored by Nicholas Hollingum's avatar Nicholas Hollingum Committed by Commit Bot

Fixed a use-after-free in exo::Pointer

Basically you can get the UAF by binding to either one of the delegates
twice.  Naturally (as a comment suggested) this doesnt make much sense,
but its still an attack surface so this fix will stop it.

The fix means that if a user binds the delegate's interface twice, then
we will only keep the latest one alive, and we simulate removal of the
pointer interface for the other (which prevents it from invoking methods
on that pointer during its destruction).

Bug: b:135720248
Change-Id: I39f4ca1602058efa650a51a41e3ce7b955bb43bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1670574
Commit-Queue: Nic Hollingum <hollingum@google.com>
Reviewed-by: default avatarMitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671568}
parent fbe4473c
...@@ -193,15 +193,20 @@ void Pointer::SetCursorType(ui::CursorType cursor_type) { ...@@ -193,15 +193,20 @@ void Pointer::SetCursorType(ui::CursorType cursor_type) {
} }
void Pointer::SetGesturePinchDelegate(PointerGesturePinchDelegate* delegate) { void Pointer::SetGesturePinchDelegate(PointerGesturePinchDelegate* delegate) {
// For the |pinch_delegate_| (and |relative_pointer_delegate_| below) it is
// possible to bind multiple extensions to the same pointer interface (not
// that this is a particularly reasonable thing to do). When that happens we
// choose to only keep a single binding alive, so we simulate pointer
// destruction for the previous binding.
if (pinch_delegate_)
pinch_delegate_->OnPointerDestroying(this);
pinch_delegate_ = delegate; pinch_delegate_ = delegate;
} }
void Pointer::RegisterRelativePointerDelegate( void Pointer::RegisterRelativePointerDelegate(
RelativePointerDelegate* delegate) { RelativePointerDelegate* delegate) {
// It does not seem that wayland forbids multiple relative pointer interfaces if (relative_pointer_delegate_)
// being registered against the same pointer, though that is not really relative_pointer_delegate_->OnPointerDestroying(this);
// reasonable behaviour.
DCHECK(!relative_pointer_delegate_);
relative_pointer_delegate_ = delegate; relative_pointer_delegate_ = delegate;
} }
...@@ -334,8 +339,7 @@ void Pointer::OnMouseEvent(ui::MouseEvent* event) { ...@@ -334,8 +339,7 @@ void Pointer::OnMouseEvent(ui::MouseEvent* event) {
TRACE_EXO_INPUT_EVENT(event); TRACE_EXO_INPUT_EVENT(event);
if (event->IsMouseEvent() && if (event->IsMouseEvent() && event->type() != ui::ET_MOUSE_EXITED &&
event->type() != ui::ET_MOUSE_EXITED &&
event->type() != ui::ET_MOUSE_CAPTURE_CHANGED) { event->type() != ui::ET_MOUSE_CAPTURE_CHANGED) {
// Generate motion event if location changed. We need to check location // Generate motion event if location changed. We need to check location
// here as mouse movement can generate both "moved" and "entered" events // here as mouse movement can generate both "moved" and "entered" events
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment