CVE-2021-3472 / ZDI-CAN-1259
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
Delivery of emulated events usually happens only to the owning client.
If there are grabs, only the grabbing client may receive these events.
This logic does not work during the touch event replay in
DeactivatePointerGrab(), as the previous grab is no longer in the
listener queue of the touch, so the next owner gets whole emulated event
sequence. This may trigger implicit grabs. After replay,
DeactivatePointerGrab() will update the global grab without regard to
this new implicit grab, which leads to issues down the line.
This change is effectively the same as 35e5a76cc1 except that the change
is limited to only emulated pointer events. Otherwise, in the case of a
device grab we end up not sending any touch events to clients that
selected XI_TouchOwnership event and should get touch events before they
get ownership of touch sequence.
Fixes#7https://bugs.freedesktop.org/show_bug.cgi?id=96536
Pointer-emulated touch events should only be delivered to the client
that owns the sequence even if it's a core client that became the
effective owner of the sequency by selecting for pointer press and
movement.
Currently the emulated events are delivered like this already (see
TouchResourceIsOwner() check in DeliverEmulatedMotionEvent()), except in
the case of TouchEnd, in which case the generated motion event is still
delivered to some client that's not necessarily the owner of the touch
sequence.
We already know whether a touch sequence that is about to emulate a
pointer event has an owner, we just need to check that. This further
allows to simplify DeliverEmulatedMotionEvent() as it won't ever be
called for non-owned touch events.
https://bugs.freedesktop.org/show_bug.cgi?id=60394
Signed-off-by: Povilas Kanapickas <povilas@radix.lt>
CVE-2020-14346 / ZDI-CAN-11429
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
Most (but not all) of these were found by using
codespell --builtin clear,rare,usage,informal,code,names
but not everything reported by that was fixed.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
We can't have high keycodes because everything in XKB relies on 8 bits. XI2's
API allows for 32-bit keycodes so we have to take those but nothing in the
server is really ready for this. The effect of this right now is that any high
keycode grab is clipped to 255 and thus ends up grabbing a different key
instead.
https://bugzilla.redhat.com/show_bug.cgi?id=1697804
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The input thread checks the barriers for pointer positioning, swapping the
list out from underneath is considered impolite.
Reported-by: Michel Dänzer <michel.daenzer@amd.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
When Retrieving touch delivery data we need to check if we have an active
grab on such device, and in that case use it to delivery events.
If we don't do this, when rejecting the touch events in DeactivatePointerGrab,
we will end-up in creating an implicit grab that will change the device
deviceGrab's state, causing a recursion during TouchEndTouch.
Fixes#7https://bugs.freedesktop.org/show_bug.cgi?id=96536
They were defined as empty macros on all platforms except for the
long unsupported Cray systems which needed to use bitfields to define
types smaller than 64-bits.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
This has caused nonsensical values in xinput output.
Signed-off-by: Roman Kapl <code@rkapl.cz>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The byte-swapping code forgot that the xXIButtonInfo is followed by a
button mask, not directly by the button labels. This resulted in client
crashes in cross-endian setups, for example in `xinput list --long`,
since the client got an invalid atom.
A new function was introduced to get the right positions for the label
and mask data.
Signed-off-by: Roman Kapl <code@rkapl.cz>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Roundhouse kick replacing the various (sizeof(foo)/sizeof(foo[0])) with
the ARRAY_SIZE macro from dix.h when possible. A semantic patch for
coccinelle has been used first. Additionally, a few macros have been
inlined as they had only one or two users.
Signed-off-by: Daniel Martin <consume.noise@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
[jcristau: originally this patch fixed the same issue as commit
211e05ac85 "Xi: Test exact size of XIBarrierReleasePointer", with the
addition of these checks]
This addresses CVE-2017-12179
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Reviewed-by: Julien Cristau <jcristau@debian.org>
Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Signed-off-by: Nathan Kidd <nkidd@opentext.com>
Signed-off-by: Julien Cristau <jcristau@debian.org>
Otherwise a client can send any value of num_barriers and cause reading or swapping of values on heap behind the receive buffer.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The SProcXSendExtensionEvent must not attempt to swap GenericEvent because
it is assuming that the event has fixed size and gives the swapping function
xEvent-sized buffer.
A GenericEvent would be later rejected by ProcXSendExtensionEvent anyway.
Signed-off-by: Michal Srb <msrb@suse.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The requirement is that events have type in range
EXTENSION_EVENT_BASE..lastEvent, but it was tested
only for first event of all.
Signed-off-by: Michal Srb <msrb@suse.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Make sure that the xEvent eventT is initialized with zeros, the same way as
in SProcSendEvent.
Some event swapping functions do not overwrite all 32 bytes of xEvent
structure, for example XSecurityAuthorizationRevoked. Two cooperating
clients, one swapped and the other not, can send
XSecurityAuthorizationRevoked event to each other to retrieve old stack data
from X server. This can be potentialy misused to go around ASLR or
stack-protector.
Signed-off-by: Michal Srb <msrb@suse.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Just like we do with XWarpPointer's.
Signed-off-by: Carlos Garnacho <carlosg@gnome.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
This is a work in progress that builds Xvfb, Xephyr, Xwayland, Xnest,
and Xdmx so far. The outline of Xquartz/Xwin support is in tree, but
hasn't been built yet. The unit tests are also not done.
The intent is to build this as a complete replacement for the
autotools system, then eventually replace autotools. meson is faster
to generate the build, faster to run the bulid, shorter to write the
build files in, and less error-prone than autotools.
v2: Fix indentation nits, move version declaration to project(), use
existing meson_options for version-config.h's vendor name/web.
Signed-off-by: Eric Anholt <eric@anholt.net>
Acked-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
This touches everything that ends up in the Xorg binary; the big missing
part is GLX since that's all generated code. Cuts about 14k from the
binary on amd64.
Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Eric Anholt <eric@anholt.net>
The previous code only worked when the barrier was created by the same client
as the one calling XIChangeDeviceHierarchy.
http://bugzilla.redhat.com/show_bug.cgi?id=1384432
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Commits 816015648f and
fee0827a9a made it so that
wl_keyboard::enter doesn't result in X clients getting KeyPress events
while still updating our internal xkb state to be in sync with the
host compositor.
wl_keyboard::leave needs to be handled in the same way as its
semantics from an X client POV should be the same as an X grab getting
triggered, i.e. X clients shouldn't get KeyRelease events for keys
that are still down at that point.
This patch uses LeaveNotify for these events on wl_keyboard::leave and
changes the current use of KeymapNotify to EnterNotify instead just to
keep some symmetry between both cases.
On ProcessDeviceEvent() we still need to deactivate X grabs if needed
for KeyReleases.
Signed-off-by: Rui Matos <tiagomatos@gmail.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Special case for the systemd-logind case in xfree86: when we're vt-switched
away and a device is plugged in, we get a paused fd from logind. Since we
can't probe the device or do anything with it, we store that device in the
xfree86 and handle it later when we vt-switch back. The device is not added to
inputInfo.devices until that time.
When the device is removed while still vt-switched away, the the config system
never notifies the DDX. It only runs through inputInfo.devices and our device
was never added to that.
When a device is plugged in, removed, and plugged in again while vt-switched
away, we have two entries in the xfree86-specific list that refer to the same
device node, both pending for addition later. On VT switch back, the first one
(the already removed one) will be added successfully, the second one (the
still plugged-in one) fails. Since the fd is correct, the device works until
it is removed again. The removed devices' config_info (i.e. the syspath)
doesn't match the actual device we addded tough (the input number increases
with each plug), it doesn't get removed, the fd remains open and we lose track
of the fd count. Plugging the device in again leads to a dead device.
Fix this by adding a call to notify the DDX to purge any remainders of devices
with the given config_info, that's the only identifiable bit we have at this
point.
https://bugs.freedesktop.org/show_bug.cgi?id=97928
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
This keeps the input driver SetProperty function from being called
while input events are being processed.
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
This removes all of the SIGIO handling support used for input
throughout the X server, preparing the way for using threads for input
handling instead.
Places calling OsBlockSIGIO and OsReleaseSIGIO are marked with calls
to stub functions input_lock/input_unlock so that we don't lose this
information.
xfree86 SIGIO support is reworked to use internal versions of
OsBlockSIGIO and OsReleaseSIGIO.
v2: Don't change locking order (Peter Hutterer)
v3: Comment weird && FALSE in xf86Helper.c
Leave errno save/restore in xf86ReadInput
Squash with stub adding patch (Peter Hutterer)
v4: Leave UseSIGIO config parameter so that
existing config files don't break (Peter Hutterer)
v5: Split a couple of independent patch bits out
of kinput.c (Peter Hutterer)
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
This was added in:
commit 312910b4e3
Author: Chase Douglas <chase.douglas@canonical.com>
Date: Wed Apr 18 11:15:40 2012 -0700
Update currentTime in dispatch loop
Unfortunately this is equivalent to calling GetTimeInMillis() once per
request. In the absolute best case (as on Linux) you're only hitting the
vDSO; on other platforms that's a syscall. Either way it puts a pretty
hard ceiling on request throughput.
Instead, push the call down to the requests that need it; basically,
grab processing and event generation.
Cc: Chase Douglas <chase.douglas@canonical.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Adam Jackson <ajax@redhat.com>
The touchpoint knows whether it should be emulating or not and we have a check
for that later. Check for this before we generate the event and try to deliver
it, lest we trigger a bug warning.
https://bugzilla.redhat.com/show_bug.cgi?id=1282252
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Add a new event source type for keypress events synthesised from focus
notifications (e.g. KeymapNotify from the parent server, when running
nested). This is used to keep the keys-down array in sync with the host
server's, without sending actual keypress events to clients.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
xichangehierarchy.c:424:23: warning: comparison of constant 536870911 with expression of type 'uint16_t'
(aka 'unsigned short') is always false [-Wtautological-constant-out-of-range-compare,Semantic Issue]
if (stuff->length > (INT_MAX >> 2))
~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
xichangehierarchy.c:438:26: warning: comparison of constant 536870911 with expression of type 'uint16_t'
(aka 'unsigned short') is always false [-Wtautological-constant-out-of-range-compare,Semantic Issue]
if ((any->length > (INT_MAX >> 2)) || (len < (any->length << 2)))
~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The client window can be closed in the middle of a touch sequence,
e.g. Qt 4 closes popup windows on MousePress and Qt 5.5 will do it
on TouchBegin. In this case the state of mouse buttons will not be
updated on TouchEnd because ProcessTouchEvent() calls UpdateDeviceState()
only when the event has been sent to the client. It results in a
stuck left mouse button.
This patch leads to calling UpdateDeviceState() in case the client
can't be found.
Signed-off-by: Alexander Volkov <a.volkov@rusbitech.ru>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
While it's documented in the XACE spec, the XACE_KEY_AVAIL hook is
currently never actually invoked by the xserver.
This hook was added in 13c6713c82 (25 Aug 2006), but as the keyboard
processing was moved into XKB, the hook was forgotten and silently
dropped. The code calling this hook was removed by 7af53799c (4 Jan
2009), but it was probably already unused before that.
This patch re-adds support for this hook. The "count" hook parameter is
unused.
Signed-off-by: Andrew Eikum <aeikum@codeweavers.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
It's going to multiply anyway, so if we have non-constant values, might
as well let it do the multiplication instead of adding another multiply,
and good versions of calloc will check for & avoid overflow in the process.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Matt Turner <mattst88@gmail.com>
Multiple functions in the Xinput extension handling of requests from
clients failed to check that the length of the request sent by the
client was large enough to perform all the required operations and
thus could read or write to memory outside the bounds of the request
buffer.
This commit includes the creation of a new REQUEST_AT_LEAST_EXTRA_SIZE
macro in include/dix.h for the common case of needing to ensure a
request is large enough to include both the request itself and a
minimum amount of extra data following the request header.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
I've been seeing sporadic (anywhere from once every few days to 3-4
times a day) crashes and freezes in X. The problematic behaviour isn't
always the same, but I chose a particular incident to debug, and found
that X was segfaulting in updateMotionHistory, on line 575 of
dix/getevents.c.
After some further investigation, I found that the bug was being
triggered when a SIGIO was received in DeepCopyPointerClasses, between
the AllocValuatorClass call (line 540) and updating the to->valuator
pointer (line 545). AllocValuatorClass calls realloc() on to->valuator,
so between these lines, it's not guaranteed to point to allocated
memory.
It seems the SIGIO handler is calling updateMotionHistory, which is
reading the memory pointed to by to->valuator and getting a wrong value
for last_motion, which updates buff to point to wildly the wrong place
and thus generates a segfault when a memcpy() is done into buff.
I am attaching a patch which I've been running on that machine for the
past three days, and haven't yet observed any more crashing or freezing
behaviour. The patch simply calls OsBlockSIGIO while
DeepCopyDeviceClasses is in progress, as the state of the X server's
device data structures is not guaranteed to be in a consistent state
during that time.
Debian bug#744303 <https://bugs.debian.org/744303>
Signed-off-by: Julien Cristau <jcristau@debian.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
==26141== Invalid read of size 8
==26141== at 0x58FAEA: DeliverEmulatedMotionEvent (exevents.c:1484)
An InternalEvent is bigger than a DeviceEvent, thus copying one to the other
reads past the allocated boundary. Shouldn't have any real effect since we
shouldn't access anything past the DeviceEvent boundary if the event type is
correct.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
This will also make it useful for cases when we have a new keymap to
apply to a device but don't have a source device.
Reviewed-by: Kristian Høgsberg <krh@bitplanet.net>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The other values are checked correctly, but if a modifier was outside the
allowed range, it would go unnoticed and cause a out-of-bounds read error for
any mask equal or larger than 256. The DetailRec where we store the grab masks
is only sized to 8 * sizeof(Mask).
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>