- Apr 03, 2024
-
-
Olivier Fourdan authored
Signed-off-by:
Olivier Fourdan <ofourdan@redhat.com> Part-of: <xorg/xserver!1465>
-
Previously, AllocateGlyph would return a new glyph with refcount=0 and a re-used glyph would end up not changing the refcount at all. The resulting glyph_new array would thus have multiple entries pointing to the same non-refcounted glyphs. AddGlyph may free a glyph, resulting in a UAF when the same glyph pointer is then later used. Fix this by returning a refcount of 1 for a new glyph and always incrementing the refcount for a re-used glyph, followed by dropping that refcount back down again when we're done with it. CVE-2024-31083, ZDI-CAN-22880 This vulnerability was discovered by: Jan-Niklas Sohn working with Trend Micro Zero Day Initiative (backported from commit bdca6c3d) Part-of: <xorg/xserver!1464>
-
Alan Coopersmith authored
CVE-2024-31081 Fixes: d220d690 ("Xi: add GrabButton and GrabKeysym code.") Signed-off-by:
Alan Coopersmith <alan.coopersmith@oracle.com> (cherry picked from commit 3e77295f) Part-of: <xorg/xserver!1464>
-
Alan Coopersmith authored
CVE-2024-31080 Reported-by: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=69762 Fixes: 53e821ab ("Xi: add request processing for XIGetSelectedEvents.") Signed-off-by:
Alan Coopersmith <alan.coopersmith@oracle.com> (cherry picked from commit 96798fc1) Part-of: <xorg/xserver!1464>
-
- Apr 02, 2024
-
-
Fixes 219c54b8 (cherry picked from commit 133e0d65) Part-of: <xorg/xserver!1452>
-
- Feb 08, 2024
-
-
X11 clients tend to assume that pointers have buttons. This assumption means they often fail to handle the X error that is generated when querying the button mapping of a pointer device that lacks buttons. This failure to handle the X error leads to those client applications to abruptly exit. This commit assigns vestigial buttons to the gesture pointer device for the sole purpose of backward compatibility with legacy X11 clients. That technique is already employed for a different pointer, the relative pointer device, for similar reasons, so this just makes the legacy client compatibility more complete. See https://gitlab.gnome.org/GNOME/mutter/-/issues/2353 (cherry picked from commit 456b0e86)
-
- Jan 16, 2024
-
-
José Expósito authored
Signed-off-by:
José Expósito <jose.exposito89@gmail.com>
-
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:
Olivier Fourdan <ofourdan@redhat.com> Reviewed-by:
Peter Hutterer <peter.hutterer@who-t.net> (cherry picked from commit 2ef0f111)
-
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> (cherry picked from commit e5e8586a)
-
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 (cherry picked from commit 26769aa7)
-
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 (cherry picked from commit bc1fdbe4)
-
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 4a5e9b18)
-
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 df3c6570)
-
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 (cherry picked from commit 219c54b8)
-
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 (cherry picked from commit ece23be8)
-
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 9e2ecb2a)
-
- Jan 11, 2024
-
-
For ALUs which may leave the alpha channel at values other than 1.0. Closes: xorg/xserver#1615 v2: * List safe ALUs instead of unsafe ones (cherry picked from commit e5a3f3e8)
-
Preparation for the following commit, no functional change intended. (cherry picked from commit 8f66c156)
-
It caused an incorrect result of the blend operation. Use glColorMask to prevent non-1.0 alpha channel values in a depth 32 pixmap backing an effective depth 24 window. For blending operations, the expectation is that the destination drawable contains valid pixel values, so the alpha channel should already be 1.0. Fixes: d1f14289 ("glamor: Ignore destination alpha as necessary for composite operation") Issue: https://gitlab.gnome.org/GNOME/mutter/-/issues/3104 (cherry picked from commit d1bbf82d)
-
XTest requests lets the client specify a device ID, only if none is specified do we fall back to the XTEST special device. As of commit aa407425 input: Add new hook DeviceSendEventsProc for XTEST regular devices are no longer able to send XTest events because they have no sendEventsProc set. This caused issue #1574 and the crash was fixed with commit e820030d xtest: Check whether there is a sendEventsProc to call but we still cannot send XTest events through a specific device. Fix this by defaulting every device to the XTest send function and punting it to the DDX (i.e. Xwayland) to override the devices as necessary. Fixes e820030d Fixes aa407425 (cherry picked from commit de0031ee)
-
Otherwise only XTest events on the XTest device get handled, XTest requests on real devices are still processed as normal events. (cherry picked from commit 7f7adfde)
-
This just makes the existing behavior explicit, previously we relied on a malloc(numAxes * ...) to return NULL to error out. (cherry picked from commit 7aba2514)
-
If we remove a master device and specify which other master devices attached slaves should be returned to, enforce that those two are indeeed a pointer and a keyboard. Otherwise we can try to attach the keyboards to pointers and vice versa, leading to possible crashes later. (cherry picked from commit 37539cb0)
-
Olivier Fourdan authored
Xwayland uses OEFFIS_DEVICE_ALL_DEVICES to get all possible device types enabled. Be more selective and specify explicitly keyboard and pointer instead of relying on what "all devices" translates to in the stack. See-also: https://gitlab.gnome.org/GNOME/mutter/-/issues/3194 Signed-off-by:
Olivier Fourdan <ofourdan@redhat.com> (cherry picked from commit 1bf4d60a)
-
(cherry picked from commit 5c8f70a1)
-
LOCAL_PEERCRED is similar to SO_PEERCRED but takes SOL_LOCAL. On DragonFly cr_pid isn't supported yet, so fall back to getpeereid(). Based on wayland/wayland@54b237a6 (cherry picked from commit 58e8c967)
-
- Dec 13, 2023
-
-
Peter Hutterer authored
Signed-off-by:
Peter Hutterer <peter.hutterer@who-t.net>
-
Peter Hutterer authored
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 0c1a93d3)
-
Peter Hutterer authored
Affected are ProcRRChangeProviderProperty and ProcRRChangeOutputProperty. See also 8f454b79 where this same bug was fixed for the core protocol and XI. 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->nUnits 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->nUnits bytes, i.e. 4GB. CVE-2023-6478, ZDI-CAN-22561 This vulnerability was discovered by: Jan-Niklas Sohn working with Trend Micro Zero Day Initiative (cherry picked from commit 14f48001)
-
- Dec 04, 2023
-
-
Olivier Fourdan authored
When creating the output with the default "XWAYLAND<n>" name, we use the MAX_OUTPUT_NAME value to allocate a lot more memory than necessary to accommodate for future output names once they get updated, but by doing so, we also send XRandR way too much (zeroed) data since the "nameLength" value is (purposely) set too big. So, instead, let's just update the name after creating the RR output, this way we set both the name and nameLength to their correct values while keeping the initial large allocation. Signed-off-by:
Olivier Fourdan <ofourdan@redhat.com> Fixes: 3c07a01c - xwayland: Use xdg-output name for XRandR Reviewed-by:
Simon Ser <contact@emersion.fr> (cherry picked from commit 83453fb5)
-
Olivier Fourdan authored
At creation, Xwayland uses a generic output name ("XWAYLAND0", etc.) for the XRandR outputs, and later, once the name is known from the Wayland protocols, updates the output names using the actual names from the Wayland compositor. However, when doing so, it simply updates the string, the "nameLength" isn't updated, so the name passed to the clients might either end up being truncated or contain portions of the previous (initial) output name. Note, this is using a fixed size buffer initialized with zeros, so this cannot leak any data other than the previous output name, so this is mainly a cosmetic issue. Update the output's "nameLength" when updating the output name. Signed-off-by:
Olivier Fourdan <ofourdan@redhat.com> Fixes: 3c07a01c - xwayland: Use xdg-output name for XRandR Reviewed-by:
Simon Ser <contact@emersion.fr> (cherry picked from commit 0e314afe)
-
- Nov 29, 2023
-
-
Olivier Fourdan authored
Most X servers, even those which do not have specific configuration files, can use the directory specified by SERVER_MISC_CONFIG_PATH when they have either the XSECURITY or XSELINUX extensions enabled, or when support for DTRACE is enabled at build time, because this is also where the "protocol.txt" file is searched for at runtime. Unfortunately, the SERVER_MISC_CONFIG_PATH is set from serverconfigdir which is hardcoded in the build system to "$prefix/$libdir/xorg", and all X server builds share the same path. That makes it harder for different X servers such as Xwayland to install in the same path without sharing the same server configuration path (and hence the same "protocol.txt" file). Allow for the customization of server configuration path from the build options so that different X servers can use completely different and independent paths. Signed-off-by:
Olivier Fourdan <ofourdan@redhat.com> Reviewed-by:
Peter Hutterer <peter.hutterer@who-t.net> (cherry picked from commit 411a61f5)
-
Olivier Fourdan authored
When running fullscreen, if an X11 client has changed the resolution, Xwayland is using a viewport to emulate the expected resolution. When changing focus, the Wayland compositor will send a configure event with the actual surface size, not the size of the emulated XRandR resolution. As a result, changing focus while XRandR emulation (and hence the viewport) is active in Xwayland will revert the resolution to the actual output size, defeating the XRandR emulation. To avoid that issue, only change the size when not running fullscreen. Fixes: 53b6d4db - xwayland: Apply root toplevel configure dimensions Signed-off-by:
Olivier Fourdan <ofourdan@redhat.com> Reviewed-by:
Kenny Levinsen <kl@kl.wtf> (cherry picked from commit a797776f)
-
Olivier Fourdan authored
Make sure to update the fullscreen rootful window configuration whenever the output setup changes. Signed-off-by:
Olivier Fourdan <ofourdan@redhat.com> Reviewed-by:
Kenny Levinsen <kl@kl.wtf> (cherry picked from commit 06eb7271)
-
Olivier Fourdan authored
Whenever the output configuration changes, if Xwayland is running fullscreen, we may need to update the viewport in use or even update the output on which Xwayland is currently running fullscreen. Add a new helper function xwl_window_rootful_update_fullscreen() that will recompute the fullscreen state and the viewport setup so that the fullscreen Xwayland rootful window matches the new setup. Signed-off-by:
Olivier Fourdan <ofourdan@redhat.com> Reviewed-by:
Kenny Levinsen <kl@kl.wtf> (cherry picked from commit 73b9ff53)
-
Olivier Fourdan authored
No functional change. Signed-off-by:
Olivier Fourdan <ofourdan@redhat.com> Reviewed-by:
Kenny Levinsen <kl@kl.wtf> (cherry picked from commit 2f84e3fe)
-
- Oct 25, 2023
-
-
Peter Hutterer authored
Signed-off-by:
Peter Hutterer <peter.hutterer@who-t.net>
-
Peter Hutterer authored
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 541ab2ec)
-
- Oct 24, 2023
-
-
This is more portable than libbsd as everything Just Works, even on BSD systems, and is the recommended method of consuming libbsd nowadays. It also helpfully lets things work with glibc-provided functions for new enough glibc. Closes: xorg/xserver!973 Co-authored-by:
Guillem Jover <guillem@hadrons.org> Signed-off-by:
Sam James <sam@gentoo.org> (cherry picked from commit 94945a52)
-
- Oct 12, 2023
-
-
Olivier Fourdan authored
Xwayland maintains a connection to EI up for 10 minutes after an X11 client has vanished, to avoid going through the connection phase every time a short lived X11 client comes and goes. However, if the EI client gets freed (through some other event, e.g. the user decides to terminate the EI session), Xwayland would still keep the callback alive and end up trying to free an already freed EI client: Invalid read of size 4 at 0x4C5E6F9: object_unref (util-object.h:89) by 0x4C5E6F9: ei_unref (libei.c:77) by 0x429525: free_ei (xwayland-xtest.c:224) by 0x429A6E: disconnect_timer_cb (xwayland-xtest.c:404) by 0x5E63FF: DoTimer (WaitFor.c:276) by 0x5E6463: DoTimers (WaitFor.c:290) by 0x5E6164: check_timers (WaitFor.c:133) by 0x5E61E9: WaitForSomething (WaitFor.c:195) by 0x4AD50E: Dispatch (dispatch.c:487) by 0x4BBA0B: dix_main (main.c:272) by 0x43615D: main (stubmain.c:34) Address 0x15cc6ee8 is 8 bytes inside a block of size 240 free'd at 0x48452AC: free (vg_replace_malloc.c:974) by 0x4C5E729: object_destroy (util-object.h:73) by 0x4C5E729: object_unref (util-object.h:91) by 0x4C5E729: ei_unref (libei.c:77) by 0x429525: free_ei (xwayland-xtest.c:224) by 0x42A946: xwl_handle_ei_event (xwayland-xtest.c:804) by 0x5EA977: HandleNotifyFd (connection.c:809) by 0x5EE8E3: ospoll_wait (ospoll.c:657) by 0x5E624D: WaitForSomething (WaitFor.c:208) by 0x4AD50E: Dispatch (dispatch.c:487) by 0x4BBA0B: dix_main (main.c:272) by 0x43615D: main (stubmain.c:34) Block was alloc'd at at 0x484782C: calloc (vg_replace_malloc.c:1554) by 0x4C5E777: ei_create (libei.c:73) by 0x4C5E777: ei_create_context (libei.c:97) by 0x42994B: setup_ei (xwayland-xtest.c:366) by 0x42A383: xwayland_xtest_send_events (xwayland-xtest.c:658) by 0x54ED4C: ProcXTestFakeInput (xtest.c:441) by 0x54EE56: ProcXTestDispatch (xtest.c:475) by 0x4AD6E6: Dispatch (dispatch.c:546) by 0x4BBA0B: dix_main (main.c:272) by 0x43615D: main (stubmain.c:34) To avoid that issue, make sure to cancel the timer as soon as a EI client is freed. Signed-off-by:
Olivier Fourdan <ofourdan@redhat.com> Reviewed-by:
Peter Hutterer <peter.hutterer@who-t.net> See-also: https://bugzilla.redhat.com/2243076 (cherry picked from commit 9617de73)
-