From 45ea0a93934c8a3760a4d68ba4ffc932375f60de Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 18 Dec 2023 14:27:50 +1000 Subject: [PATCH xserver 1/2] 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 --- dix/enterleave.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dix/enterleave.c b/dix/enterleave.c index ded8679d76..17964b00a4 100644 --- a/dix/enterleave.c +++ b/dix/enterleave.c @@ -675,7 +675,8 @@ static void DeliverStateNotifyEvent(DeviceIntPtr dev, WindowPtr win) { int evcount = 1; - deviceStateNotify *ev, *sev; + deviceStateNotify sev[6 + (MAX_VALUATORS + 2)/3]; + deviceStateNotify *ev; deviceKeyStateNotify *kev; deviceButtonStateNotify *bev; @@ -714,7 +715,7 @@ DeliverStateNotifyEvent(DeviceIntPtr dev, WindowPtr win) } } - sev = ev = xallocarray(evcount, sizeof(xEvent)); + ev = sev; FixDeviceStateNotify(dev, ev, NULL, NULL, NULL, first); if (b != NULL) { @@ -770,7 +771,6 @@ DeliverStateNotifyEvent(DeviceIntPtr dev, WindowPtr win) DeliverEventsToWindow(dev, win, (xEvent *) sev, evcount, DeviceStateNotifyMask, NullGrab); - free(sev); } void -- 2.43.0