Skip to content
Snippets Groups Projects
  1. Apr 03, 2024
  2. Mar 27, 2024
  3. Mar 23, 2024
    • Peter Hutterer's avatar
      Allow disabling byte-swapped clients · 5d7272f0
      Peter Hutterer authored and Alan Coopersmith's avatar Alan Coopersmith committed
      
      The X server swapping code is a huge attack surface, much of this code
      is untested and prone to security issues. The use-case of byte-swapped
      clients is very niche, so allow users to disable this if they don't
      need it, using either a config option or commandline flag.
      
      For Xorg, this adds the ServerFlag "AllowByteSwappedClients" "off".
      For all DDX, this adds the commandline options +byteswappedclients and
      -byteswappedclients to enable or disable, respectively.
      
      Fixes #1201
      Signed-off-by: default avatarPeter Hutterer <peter.hutterer@who-t.net>
      ---
      (cherry picked from commit 41277766)
      (cherry picked from commit af5cd5ac)
      Backport to server-21.1-branch modified to keep byte-swapping enabled
      by default but easy to disable by users or admins (or even by distros
      shipping an xorg.conf.d fragment in their packages).
      
      Signed-off-by: default avatarAlan Coopersmith <alan.coopersmith@oracle.com>
      Part-of: <!1440>
      5d7272f0
  4. Feb 23, 2024
  5. Feb 19, 2024
  6. Jan 22, 2024
    • Povilas Kanapickas's avatar
      dix: Fix use after free in input device shutdown · 8b75ec34
      Povilas Kanapickas authored and Peter Hutterer's avatar Peter Hutterer committed
      
      This fixes access to freed heap memory via dev->master. E.g. when
      running BarrierNotify.ReceivesNotifyEvents/7 test from
      xorg-integration-tests:
      
      ==24736==ERROR: AddressSanitizer: heap-use-after-free on address
      0x619000065020 at pc 0x55c450e2b9cf bp 0x7fffc532fd20 sp 0x7fffc532fd10
      READ of size 4 at 0x619000065020 thread T0
          #0 0x55c450e2b9ce in GetMaster ../../../dix/devices.c:2722
          #1 0x55c450e9d035 in IsFloating ../../../dix/events.c:346
          #2 0x55c4513209c6 in GetDeviceUse ../../../Xi/xiquerydevice.c:525
      ../../../Xi/xichangehierarchy.c:95
          #4 0x55c450e3455c in RemoveDevice ../../../dix/devices.c:1204
      ../../../hw/xfree86/common/xf86Xinput.c:1142
          #6 0x55c450e17b04 in CloseDeviceList ../../../dix/devices.c:1038
          #7 0x55c450e1de85 in CloseDownDevices ../../../dix/devices.c:1068
          #8 0x55c450e837ef in dix_main ../../../dix/main.c:302
          #9 0x55c4517a8d93 in main ../../../dix/stubmain.c:34
      (/lib/x86_64-linux-gnu/libc.so.6+0x28564)
          #11 0x55c450d0113d in _start (/usr/lib/xorg/Xorg+0x117713d)
      
      0x619000065020 is located 160 bytes inside of 912-byte region
      [0x619000064f80,0x619000065310)
      freed by thread T0 here:
      (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10d7cf)
          #1 0x55c450e19f1c in CloseDevice ../../../dix/devices.c:1014
          #2 0x55c450e343a4 in RemoveDevice ../../../dix/devices.c:1186
      ../../../hw/xfree86/common/xf86Xinput.c:1142
          #4 0x55c450e17b04 in CloseDeviceList ../../../dix/devices.c:1038
          #5 0x55c450e1de85 in CloseDownDevices ../../../dix/devices.c:1068
          #6 0x55c450e837ef in dix_main ../../../dix/main.c:302
          #7 0x55c4517a8d93 in main ../../../dix/stubmain.c:34
      (/lib/x86_64-linux-gnu/libc.so.6+0x28564)
      
      previously allocated by thread T0 here:
      (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10ddc6)
          #1 0x55c450e1c57b in AddInputDevice ../../../dix/devices.c:259
          #2 0x55c450e34840 in AllocDevicePair ../../../dix/devices.c:2755
          #3 0x55c45130318f in add_master ../../../Xi/xichangehierarchy.c:152
      ../../../Xi/xichangehierarchy.c:465
          #5 0x55c4512cb9f5 in ProcIDispatch ../../../Xi/extinit.c:390
          #6 0x55c450e6a92b in Dispatch ../../../dix/dispatch.c:551
          #7 0x55c450e834b7 in dix_main ../../../dix/main.c:272
          #8 0x55c4517a8d93 in main ../../../dix/stubmain.c:34
      (/lib/x86_64-linux-gnu/libc.so.6+0x28564)
      
      The problem is caused by dev->master being not reset when disabling the
      device, which then causes dangling pointer when the master device itself
      is being deleted when exiting whole server.
      
      Note that RecalculateMasterButtons() requires dev->master to be still
      valid, so we can reset it only at the end of function.
      
      Signed-off-by: default avatarPovilas Kanapickas <povilas@radix.lt>
      (cherry picked from commit 1801fe0a)
      8b75ec34
  7. Jan 16, 2024
    • José Expósito's avatar
      xserver 21.1.11 · 31407c01
      José Expósito authored
      
      Signed-off-by: default avatarJosé Expósito <jose.exposito89@gmail.com>
    • Olivier Fourdan's avatar
      ephyr,xwayland: Use the proper private key for cursor · a4f0e946
      Olivier Fourdan authored and José Expósito's avatar José Expósito committed
      
      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: default avatarOlivier Fourdan <ofourdan@redhat.com>
      Reviewed-by: default avatarPeter Hutterer <peter.hutterer@who-t.net>
      (cherry picked from commit 2ef0f111)
      a4f0e946
    • Olivier Fourdan's avatar
      glx: Call XACE hooks on the GLX buffer · 8d825f72
      Olivier Fourdan authored and José Expósito's avatar José Expósito committed
      
      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: default avatarOlivier Fourdan <ofourdan@redhat.com>
      Acked-by: default avatarPeter Hutterer <peter.hutterer@who-t.net>
      (cherry picked from commit e5e8586a)
      8d825f72
    • Peter Hutterer's avatar
      dix: when disabling a master, float disabled slaved devices too · 5c4816af
      Peter Hutterer authored and José Expósito's avatar José Expósito committed
      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)
      5c4816af
    • José Expósito's avatar
      Xi: do not keep linked list pointer during recursion · 7b569436
      José Expósito authored and José Expósito's avatar José Expósito committed
      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)
      7b569436
    • Peter Hutterer's avatar
      Xi: flush hierarchy events after adding/removing master devices · 62363421
      Peter Hutterer authored and José Expósito's avatar José Expósito committed
      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)
      62363421
    • Peter Hutterer's avatar
      Xi: when creating a new ButtonClass, set the number of buttons · 8887cb1f
      Peter Hutterer authored and José Expósito's avatar José Expósito committed
      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)
      8887cb1f
    • Peter Hutterer's avatar
      dix: fix DeviceStateNotify event calculation · 7173a891
      Peter Hutterer authored and José Expósito's avatar José Expósito committed
      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)
      7173a891
    • Peter Hutterer's avatar
      dix: Allocate sufficient xEvents for our DeviceStateNotify · c494deba
      Peter Hutterer authored and José Expósito's avatar José Expósito committed
      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)
      c494deba
    • Peter Hutterer's avatar
      dix: allocate enough space for logical button maps · 4e78bc3a
      Peter Hutterer authored and José Expósito's avatar José Expósito committed
      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)
      4e78bc3a
  8. Jan 03, 2024
    • Michael Wyraz's avatar
      Removing the code that deletes an existing monitor in RRMonitorAdd · c338d19f
      Michael Wyraz authored
      In commit 7e1f86d4 monitor support was added to randr. At this time it seemed to be reasonable not to have
      more than one (virtual) monitor on a particular physical display. The code was never changed since.
      
      Nowadays, extremely large displays exists (4k displays, ultra-wide displays). In some use cases it makes sense to
      split these large physical displays into multiple virtual monitors. An example are ultra-wide screens that can be
      split into 2 monitors. The change in this commit makes this work.
      
      Besides that, removing a monitor in a function that is called "RRMonitorAdd" is bad practice and causes
      unexpected behaviour.
      c338d19f
  9. Dec 13, 2023
    • Peter Hutterer's avatar
      xserver 21.1.10 · 15e24097
      Peter Hutterer authored
      
      Signed-off-by: default avatarPeter Hutterer <peter.hutterer@who-t.net>
    • Peter Hutterer's avatar
      Xi: allocate enough XkbActions for our buttons · a7bda308
      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)
      a7bda308
    • Peter Hutterer's avatar
      randr: avoid integer truncation in length check of ProcRRChange*Property · 58e83c68
      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)
      58e83c68
  10. Oct 25, 2023
    • nerdopolis's avatar
      xephyr: Don't check for SeatId anymore · c1ad8df2
      nerdopolis authored and Matt Turner's avatar Matt Turner committed
      After a change for the xserver to automatically determine the seat
      based on the XDG_SEAT variable, xephyr stopped working. This was
      because of an old feature where xephyr used to handle evdev
      directly. This was dropped some time ago, and now this check is
      not needed
      
      (cherry picked from commit 4c03b67d)
      c1ad8df2
    • Peter Hutterer's avatar
      xserver 21.1.9 · 6197bea0
      Peter Hutterer authored
      
      Signed-off-by: default avatarPeter Hutterer <peter.hutterer@who-t.net>
    • Peter Hutterer's avatar
      mi: reset the PointerWindows reference on screen switch · 3e290b3c
      Peter Hutterer authored
      
      PointerWindows[] keeps a reference to the last window our sprite
      entered - changes are usually handled by CheckMotion().
      
      If we switch between screens via XWarpPointer our
      dev->spriteInfo->sprite->win is set to the new screen's root window.
      If there's another window at the cursor location CheckMotion() will
      trigger the right enter/leave events later. If there is not, it skips
      that process and we never trigger LeaveWindow() - PointerWindows[] for
      the device still refers to the previous window.
      
      If that window is destroyed we have a dangling reference that will
      eventually cause a use-after-free bug when checking the window hierarchy
      later.
      
      To trigger this, we require:
      - two protocol screens
      - XWarpPointer to the other screen's root window
      - XDestroyWindow before entering any other window
      
      This is a niche bug so we hack around it by making sure we reset the
      PointerWindows[] entry so we cannot have a dangling pointer. This
      doesn't handle Enter/Leave events correctly but the previous code didn't
      either.
      
      CVE-2023-5380, ZDI-CAN-21608
      
      This vulnerability was discovered by:
      Sri working with Trend Micro Zero Day Initiative
      
      Signed-off-by: default avatarPeter Hutterer <peter.hutterer@who-t.net>
      Reviewed-by: Adam Jackson's avatarAdam Jackson <ajax@redhat.com>
      (cherry picked from commit 564ccf2c)
      3e290b3c
    • Peter Hutterer's avatar
      Xi/randr: fix handling of PropModeAppend/Prepend · f2922f6f
      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: default avatarPeter Hutterer <peter.hutterer@who-t.net>
      (cherry picked from commit 541ab2ec)
      f2922f6f
  11. Oct 24, 2023
  12. Apr 24, 2023
  13. Mar 29, 2023
  14. Feb 07, 2023
  15. Jan 26, 2023
Loading