CVS commit: [netbsd-10] xsrc/external/mit/xorg-server.old/dist

["Martin Husemann" <martin%netbsd.org@localhost>]

Module Name:    xsrc
Committed By:   martin
Date:           Fri Nov  8 12:10:21 UTC 2024

Modified Files:
        xsrc/external/mit/xorg-server.old/dist/Xi [netbsd-10]: exevents.c
            xichangehierarchy.c xipassivegrab.c xiproperty.c xiquerypointer.c
            xiselectev.c
        xsrc/external/mit/xorg-server.old/dist/dix [netbsd-10]: devices.c
            property.c
        xsrc/external/mit/xorg-server.old/dist/glx [netbsd-10]: glxcmds.c
        xsrc/external/mit/xorg-server.old/dist/randr [netbsd-10]: rrproperty.c

Log Message:
Pull up following revision(s) (requested by mrg in ticket #999):

        external/mit/xorg-server.old/dist/Xi/xiselectev.c: revision 1.2
        external/mit/xorg-server.old/dist/Xi/xiquerypointer.c: revision 1.2
        external/mit/xorg-server.old/dist/dix/property.c: revision 1.2
        external/mit/xorg-server.old/dist/Xi/xichangehierarchy.c: revision 1.3
        external/mit/xorg-server.old/dist/Xi/exevents.c: revision 1.3
        external/mit/xorg-server.old/dist/Xi/exevents.c: revision 1.4
        external/mit/xorg-server.old/dist/glx/glxcmds.c: revision 1.2
        external/mit/xorg-server.old/dist/dix/devices.c: revision 1.2
        external/mit/xorg-server.old/dist/randr/rrproperty.c: revision 1.3
        external/mit/xorg-server.old/dist/dix/devices.c: revision 1.3
        external/mit/xorg-server.old/dist/Xi/xipassivegrab.c: revision 1.2
        external/mit/xorg-server.old/dist/Xi/xiproperty.c: revision 1.3

merge upstream change 8f454b793e1f13c99872c15f0eed1d7f3b823fe8:

Subject: [PATCH] Xi: avoid integer truncation in length check of
 ProcXIChangeProperty

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

merge upstream change 14f480010a93ff962fef66a16412fafff81ad632:

Subject: [PATCH] randr: avoid integer truncation in length check of
 ProcRRChange*Property

Affected are ProcRRChangeProviderProperty and ProcRRChangeOutputProperty.
See also xserver@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

merge upstream change 9e2ecb2af8302dedc49cb6a63ebe063c58a9e7e3 though the
enterleave.c portion msut be applied to Xi/exevents.c:DeviceFocusEvent().

Subject: [PATCH] dix: allocate enough space for logical button maps

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

merge upstream change ece23be888a93b741aa1209d1dbf64636109d6a5 but into
Xi/exevents.c instead of dix/enterleave.c.

Subject: [PATCH] dix: Allocate sufficient xEvents for our DeviceStateNotify
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

merge upstream change e5e8586a12a3ec915673edffa10dc8fe5e15dac3

Subject: [PATCH] glx: Call XACE hooks on the GLX buffer

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@localhost> for providing the patch.
CVE-2024-0408

merge upstream change 4a5e9b1895627d40d26045bd0b7ef3dce503cbd1

Subject: [PATCH] Xi: flush hierarchy events after adding/removing master
 devices

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

merge upstream change bc1fdbe46559dd947674375946bbef54dd0ce36b

Subject: [PATCH] Xi: do not keep linked list pointer during recursion

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

apply upstream change 96798fc1967491c80a4d0c8d9e0a80586cb2152b

Subject: [PATCH] Xi: ProcXIGetSelectedEvents needs to use unswapped length to
 send reply

CVE-2024-31080

apply upstream change 3e77295f888c67fc7645db5d0c00926a29ffecee
Subject: [PATCH] Xi: ProcXIPassiveGrabDevice needs to use unswapped length to
 send reply

CVE-2024-31081

port xorg-server change 26769aa71fcbe0a8403b7fb13b7c9010cc07c3a8
there are two chunks in this change, and while they apply fine to the old
xorg-server 1.10 tree (unlike _most_), they do not build due to using new
identifiers.
the first chunk uses a new MASTER_ATTACHED argument to GetMaster(), which
avoids finding paired devices.  the only answer it can give with the setup
of already testing !IsMaster(other) is "other->u.master", so just use that
directly instead if calling GetMaster().
the second chunk uses a new single-line IsFloating() function, and if you
expand it's use here to the full expression, it ends up just being:
        if (!IsMaster(dev) && dev->u.master)
(which just happens to match the same line a few above, for the loop of
not-off "devices".)
testing this code path is a little tricky.

Subject: [PATCH] dix: when disabling a master, float disabled slaved devices
 too

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


To generate a diff of this commit:
cvs rdiff -u -r1.1.1.1.6.1 -r1.1.1.1.6.2 \
    xsrc/external/mit/xorg-server.old/dist/Xi/exevents.c \
    xsrc/external/mit/xorg-server.old/dist/Xi/xiproperty.c
cvs rdiff -u -r1.2 -r1.2.4.1 \
    xsrc/external/mit/xorg-server.old/dist/Xi/xichangehierarchy.c
cvs rdiff -u -r1.1.1.1 -r1.1.1.1.6.1 \
    xsrc/external/mit/xorg-server.old/dist/Xi/xipassivegrab.c \
    xsrc/external/mit/xorg-server.old/dist/Xi/xiquerypointer.c \
    xsrc/external/mit/xorg-server.old/dist/Xi/xiselectev.c
cvs rdiff -u -r1.1.1.1 -r1.1.1.1.6.1 \
    xsrc/external/mit/xorg-server.old/dist/dix/devices.c \
    xsrc/external/mit/xorg-server.old/dist/dix/property.c
cvs rdiff -u -r1.1.1.1 -r1.1.1.1.6.1 \
    xsrc/external/mit/xorg-server.old/dist/glx/glxcmds.c
cvs rdiff -u -r1.1.1.1.6.1 -r1.1.1.1.6.2 \
    xsrc/external/mit/xorg-server.old/dist/randr/rrproperty.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.