Commits on Source (9)
-
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
-
If a device has both a button class and a key class and numButtons is zero, we can get an OOB write due to event under-allocation. This function seems to assume a device has either keys or buttons, not both. It has two virtually identical code paths, both of which assume they're applying to the first event in the sequence. A device with both a key and button class triggered a logic bug - only one xEvent was allocated but the deviceStateNotify pointer was pushed on once per type. So effectively this logic code: int count = 1; if (button && nbuttons > 32) count++; if (key && nbuttons > 0) count++; if (key && nkeys > 32) count++; // this is basically always true // count is at 2 for our keys + zero button device ev = alloc(count * sizeof(xEvent)); FixDeviceStateNotify(ev); if (button) FixDeviceStateNotify(ev++); if (key) FixDeviceStateNotify(ev++); // santa drops into the wrong chimney here If the device has more than 3 valuators, the OOB is pushed back - we're off by one so it will happen when the last deviceValuator event is written instead. Fix this by allocating the maximum number of events we may allocate. Note that the current behavior is not protocol-correct anyway, this patch fixes only the allocation issue. Note that this issue does not trigger if the device has at least one button. While the server does not prevent a button class with zero buttons, it is very unlikely. CVE-2024-0229, ZDI-CAN-22678 This vulnerability was discovered by: Jan-Niklas Sohn working with Trend Micro Zero Day Initiative -
The previous code only made sense if one considers buttons and keys to be mutually exclusive on a device. That is not necessarily true, causing a number of issues. This function allocates and fills in the number of xEvents we need to send the device state down the wire. This is split across multiple 32-byte devices including one deviceStateNotify event and optional deviceKeyStateNotify, deviceButtonStateNotify and (possibly multiple) deviceValuator events. The previous behavior would instead compose a sequence of [state, buttonstate, state, keystate, valuator...]. This is not protocol correct, and on top of that made the code extremely convoluted. Fix this by streamlining: add both button and key into the deviceStateNotify and then append the key state and button state, followed by the valuators. Finally, the deviceValuator events contain up to 6 valuators per event but we only ever sent through 3 at a time. Let's double that troughput. CVE-2024-0229, ZDI-CAN-22678 This vulnerability was discovered by: Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
-
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
-
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
-
The `DisableDevice()` function is called whenever an enabled device is disabled and it moves the device from the `inputInfo.devices` linked list to the `inputInfo.off_devices` linked list. However, its link/unlink operation has an issue during the recursive call to `DisableDevice()` due to the `prev` pointer pointing to a removed device. This issue leads to a length mismatch between the total number of devices and the number of device in the list, leading to a heap overflow and, possibly, to local privilege escalation. Simplify the code that checked whether the device passed to `DisableDevice()` was in `inputInfo.devices` or not and find the previous device after the recursion. CVE-2024-21886, ZDI-CAN-22840 This vulnerability was discovered by: Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
-
Disabling a master device floats all slave devices but we didn't do this to already-disabled slave devices. As a result those devices kept their reference to the master device resulting in access to already freed memory if the master device was removed before the corresponding slave device. And to match this behavior, also forcibly reset that pointer during CloseDownDevices(). Related to CVE-2024-21886, ZDI-CAN-22840
-
The XSELINUX code will label resources at creation by checking the access mode. When the access mode is DixCreateAccess, it will call the function to label the new resource SELinuxLabelResource(). However, GLX buffers do not go through the XACE hooks when created, hence leaving the resource actually unlabeled. When, later, the client tries to create another resource using that drawable (like a GC for example), the XSELINUX code would try to use the security ID of that object which has never been labeled, get a NULL pointer and crash when checking whether the requested permissions are granted for subject security ID. To avoid the issue, make sure to call the XACE hooks when creating the GLX buffers. Credit goes to Donn Seeley <donn@xmission.com> for providing the patch. CVE-2024-0408 Signed-off-by:
Olivier Fourdan <ofourdan@redhat.com> Acked-by:
Peter Hutterer <peter.hutterer@who-t.net>
-
The cursor in DIX is actually split in two parts, the cursor itself and the cursor bits, each with their own devPrivates. The cursor itself includes the cursor bits, meaning that the cursor bits devPrivates in within structure of the cursor. Both Xephyr and Xwayland were using the private key for the cursor bits to store the data for the cursor, and when using XSELINUX which comes with its own special devPrivates, the data stored in that cursor bits' devPrivates would interfere with the XSELINUX devPrivates data and the SELINUX security ID would point to some other unrelated data, causing a crash in the XSELINUX code when trying to (re)use the security ID. CVE-2024-0409 Signed-off-by:
Showing
- Xi/exevents.c 1 addition, 0 deletionsXi/exevents.c
- Xi/xichangehierarchy.c 22 additions, 5 deletionsXi/xichangehierarchy.c
- Xi/xiquerypointer.c 1 addition, 2 deletionsXi/xiquerypointer.c
- dix/devices.c 24 additions, 3 deletionsdix/devices.c
- dix/enterleave.c 55 additions, 71 deletionsdix/enterleave.c
- glx/glxcmds.c 8 additions, 0 deletionsglx/glxcmds.c
- hw/kdrive/ephyr/ephyrcursor.c 1 addition, 1 deletionhw/kdrive/ephyr/ephyrcursor.c
- hw/xwayland/xwayland-cursor.c 1 addition, 1 deletionhw/xwayland/xwayland-cursor.c