Reported in #1817:
xwayland-24.1.6/redhat-linux-build/../Xi/xibarriers.c:729:5:
warning[-Wanalyzer-null-dereference]:
dereference of NULL ‘AllocBarrierDevice()’
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
(cherry picked from commit 0ee6039053)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2077>
Reported in #1817:
xwayland-24.1.6/redhat-linux-build/../Xi/listdev.c:171:5:
warning[-Wanalyzer-possible-null-dereference]:
dereference of possibly-NULL ‘dev’
xwayland-24.1.6/redhat-linux-build/../Xi/listdev.c:379:23:
acquire_memory: this call could return NULL
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
(cherry picked from commit 19c6195e71)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2077>
Reported in #1817:
xwayland-24.1.6/redhat-linux-build/../Xi/getprop.c:163:25:
warning[-Wanalyzer-possible-null-dereference]:
dereference of possibly-NULL ‘buf’
xwayland-24.1.6/redhat-linux-build/../Xi/getprop.c:121:19:
acquire_memory: this call could return NULL
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
(cherry picked from commit 7b18313e2a)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2077>
(The existing setting of led_mask is probably wrong, but has been set
like this since X11R5 and going back as far as the first version in
the X Consortium source control archives.)
Reported in #1817:
xwayland-24.1.6/redhat-linux-build/../Xi/getfctl.c:108:9:
warning[-Wanalyzer-use-of-uninitialized-value]:
use of uninitialized value ‘*k2.led_values’
108|-> swapl(&k2->led_values);
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
(cherry picked from commit 90c8429d35)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2077>
The wOtherInputMasks(win) macro will return NULL if
win->optional is NULL.
Reported in #1817:
xwayland-24.1.6/redhat-linux-build/../Xi/exevents.c:1390:13:
warning[-Wanalyzer-null-dereference]: dereference of NULL ‘0’
xwayland-24.1.6/redhat-linux-build/../Xi/exevents.c:1404:13:
warning[-Wanalyzer-null-dereference]: dereference of NULL ‘0’
xwayland-24.1.6/redhat-linux-build/../Xi/exevents.c:2293:9:
warning[-Wanalyzer-null-dereference]: dereference of NULL ‘0’
xwayland-24.1.6/redhat-linux-build/../Xi/exevents.c:3244:22:
warning[-Wanalyzer-null-dereference]: dereference of NULL ‘inputMasks’
xwayland-24.1.6/redhat-linux-build/../Xi/exevents.c:3338:9:
warning[-Wanalyzer-null-dereference]: dereference of NULL ‘0’
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
(cherry picked from commit 7b7bcf9231)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2077>
Grabbing a disabled (pointer) device will lead to a segfault later
in the myriad of places where we look at the device's spriteInfo - which
will be NULL.
As a workaround, disallow grabbing a disabled device by pretending it's
already grabbed. Since the point of a grab is to receive all events by
that device and disabled devices cannot send events, this should be Good
Enough.
Tested-by: Olivier Fourdan <ofourdan@redhat.com>
(cherry picked from commit 797f63b8be)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1894>
The function GetBarrierDevice() would search for the pointer device
based on its device id and return the matching value, or supposedly NULL
if no match was found.
Unfortunately, as written, it would return the last element of the list
if no matching device id was found which can lead to out of bounds
memory access.
Fix the search function to return NULL if not matching device is found,
and adjust the callers to handle the case where the device cannot be
found.
CVE-2025-26598, ZDI-CAN-25740
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit bba9df1a9d)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1830>
The comments in that function say "This only happens if master is a
slave device. don't do that" but static analysis doesn't respect that.
Found by Oracle Parfait 13.3:
Null pointer dereference [null-pointer-deref]:
Read from null pointer XTestptr
at line 274 of Xi/xichangehierarchy.c in function 'remove_master'.
Null pointer introduced at line 691 of Xext/xtest.c in function
'GetXTestDevice'.
Function GetXTestDevice may return constant 'NULL' at line 691,
called at line 273 of Xi/xichangehierarchy.c in function
'remove_master'.
Null pointer dereference [null-pointer-deref]:
Read from null pointer XTestkeybd
at line 279 of Xi/xichangehierarchy.c in function 'remove_master'.
Null pointer introduced at line 691 of Xext/xtest.c in function
'GetXTestDevice'.
Function GetXTestDevice may return constant 'NULL' at line 691,
called at line 278 of Xi/xichangehierarchy.c in function
'remove_master'.
Fixes: 0814f511d ("input: store the master device's ID in the devPrivate for XTest devices.")
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
(cherry picked from commit d10589cc09)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1767>
If either the master pointer or keyboard was disabled, the respective
GetMaster() call returns NULL, causing a segfault later accessing the
deviceid.
Fix this by looking in the off_devices list for any master
device of the type we're looking for. Master devices lose the pairing
when disabled (on enabling a keyboard we simply pair with the first
available unpaired pointer).
And for readability, split the device we get from the protocol request
into a new "dev" variable instead of re-using ptr.
Fixes#1611
(cherry picked from commit e7c876ab0b)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1767>
The `XISendDeviceHierarchyEvent()` function allocates space to store up
to `MAXDEVICES` (256) `xXIHierarchyInfo` structures in `info`.
If a device with a given ID was removed and a new device with the same
ID added both in the same operation, the single device ID will lead to
two info structures being written to `info`.
Since this case can occur for every device ID at once, a total of two
times `MAXDEVICES` info structures might be written to the allocation.
To avoid it, once one add/remove master is processed, send out the
device hierarchy event for the current state and continue. That event
thus only ever has exactly one of either added/removed in it (and
optionally slave attached/detached).
CVE-2024-21885, ZDI-CAN-22744
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
(cherry picked from commit 4a5e9b1895)
There's a racy sequence where a master device may copy the button class
from the slave, without ever initializing numButtons. This leads to a
device with zero buttons but a button class which is invalid.
Let's copy the numButtons value from the source - by definition if we
don't have a button class yet we do not have any other slave devices
with more than this number of buttons anyway.
CVE-2024-0229, ZDI-CAN-22678
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
(cherry picked from commit df3c65706e)
Both DeviceFocusEvent and the XIQueryPointer reply contain a bit for
each logical button currently down. Since buttons can be arbitrarily mapped
to anything up to 255 make sure we have enough bits for the maximum mapping.
CVE-2023-6816, ZDI-CAN-22664, ZDI-CAN-22665
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
(cherry picked from commit 9e2ecb2af8)
button->xkb_acts is supposed to be an array sufficiently large for all
our buttons, not just a single XkbActions struct. Allocating
insufficient memory here means when we memcpy() later in
XkbSetDeviceInfo we write into memory that wasn't ours to begin with,
leading to the usual security ooopsiedaisies.
CVE-2023-6377, ZDI-CAN-22412, ZDI-CAN-22413
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
(cherry picked from commit 0c1a93d319)
The handling of appending/prepending properties was incorrect, with at
least two bugs: the property length was set to the length of the new
part only, i.e. appending or prepending N elements to a property with P
existing elements always resulted in the property having N elements
instead of N + P.
Second, when pre-pending a value to a property, the offset for the old
values was incorrect, leaving the new property with potentially
uninitalized values and/or resulting in OOB memory writes.
For example, prepending a 3 element value to a 5 element property would
result in this 8 value array:
[N, N, N, ?, ?, P, P, P ] P, P
^OOB write
The XI2 code is a copy/paste of the RandR code, so the bug exists in
both.
CVE-2023-5367, ZDI-CAN-22153
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 541ab2ecd4)
CVE-2023-0494, ZDI-CAN-19596
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 0ba6d8c370)
This fixes an OOB read and the resulting information disclosure.
Length calculation for the request was clipped to a 32-bit integer. With
the correct stuff->num_items value the expected request size was
truncated, passing the REQUEST_FIXED_SIZE check.
The server then proceeded with reading at least stuff->num_items bytes
(depending on stuff->format) from the request and stuffing whatever it
finds into the property. In the process it would also allocate at least
stuff->num_items bytes, i.e. 4GB.
The same bug exists in ProcChangeProperty and ProcXChangeDeviceProperty,
so let's fix that too.
CVE-2022-46344, ZDI-CAN 19405
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Acked-by: Olivier Fourdan <ofourdan@redhat.com>
(cherry picked from commit 8f454b793e)
Both ProcXChangeDeviceProperty and ProcXIChangeProperty checked the
property for validity but didn't actually return the potential error.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Acked-by: Olivier Fourdan <ofourdan@redhat.com>
(cherry picked from commit b8a84cb0f2)
The XKB protocol effectively prevents us from ever using keycodes above
255. For buttons it's theoretically possible but realistically too niche
to worry about. For all other passive grabs, the detail must be zero
anyway.
This fixes an OOB write:
ProcXIPassiveUngrabDevice() calls DeletePassiveGrabFromList with a
temporary grab struct which contains tempGrab->detail.exact = stuff->detail.
For matching existing grabs, DeleteDetailFromMask is called with the
stuff->detail value. This function creates a new mask with the one bit
representing stuff->detail cleared.
However, the array size for the new mask is 8 * sizeof(CARD32) bits,
thus any detail above 255 results in an OOB array write.
CVE-2022-46341, ZDI-CAN 19381
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Acked-by: Olivier Fourdan <ofourdan@redhat.com>
(cherry picked from commit 51eb63b0ee)
When processing events we operate on InternalEvent pointers. They may
actually refer to a an instance of DeviceEvent, GestureEvent or any
other event that comprises the InternalEvent union. This works well in
practice because we always look into event type before doing anything,
except in the case of copying the event.
*dst_event = *src_event would copy whole InternalEvent event and would
cause out of bounds read in case the pointed to event was not
InternalEvent but e.g. DeviceEvent.
This regression has been introduced in
23a8b62d34.
Fixes https://gitlab.freedesktop.org/xorg/xserver/-/issues/1261
Signed-off-by: Povilas Kanapickas <povilas@radix.lt>
(cherry picked from commit 6ef5c05728)
This fixes a crash when a DeviceEvent struct converted to
InteralEvent was beeing copied as InternalEvent (and thus
causing out of bounds reads) in ActivateGrabNoDelivery()
in events.c: 3876 *grabinfo->sync.event = *real_event;
Possible fix for https://gitlab.freedesktop.org/xorg/xserver/-/issues/1253
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
(cherry picked from commit 5b8817a019)
[45/388] Compiling C object Xi/liblibxserver_xi.a.p/xichangehierarchy.c.o
../Xi/xichangehierarchy.c:61:32: warning: argument 1 of type ‘int[256]’ with mismatched bound [-Warray-parameter=]
61 | XISendDeviceHierarchyEvent(int flags[MAXDEVICES])
| ~~~~^~~~~~~~~~~~~~~~~
In file included from ../Xi/xichangehierarchy.c:54:
../Xi/xichangehierarchy.h:42:37: note: previously declared as ‘int[]’
42 | void XISendDeviceHierarchyEvent(int flags[]);
| ~~~~^~~~~~~
libxcb 14.1 and older are not forwards-compatible with new device
classes as it does not properly ignore unknown device classes. Since
breaking libxcb would break quite a lot of applications, we instead
report Gesture device class only if the client advertised support for XI
2.4.
Clients may still not work in cases when a client advertises XI 2.4
support and then a completely separate module within the client uses
broken libxcb to call XIQueryDevice.
Signed-off-by: Povilas Kanapickas <povilas@radix.lt>
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>
