Skip to content

Commit

Permalink
dix: Allocate sufficient xEvents for our DeviceStateNotify
Browse files Browse the repository at this point in the history
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 ece23be888a93b741aa1209d1dbf64636109d6a5)
  • Loading branch information
whot authored and dcommander committed Dec 19, 2024
1 parent b8690ba commit b1c144b
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions unix/Xvnc/programs/Xserver/dix/enterleave.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -770,7 +771,6 @@ DeliverStateNotifyEvent(DeviceIntPtr dev, WindowPtr win)

DeliverEventsToWindow(dev, win, (xEvent *) sev, evcount,
DeviceStateNotifyMask, NullGrab);
free(sev);
}

void
Expand Down

0 comments on commit b1c144b

Please sign in to comment.