From 219c54b8a3337456ce5270ded6a67bcde53553d5 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 18 Dec 2023 12:26:20 +1000 Subject: [PATCH] dix: fix DeviceStateNotify event calculation 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 --- dix/enterleave.c | 121 ++++++++++++++++++++--------------------------- 1 file changed, 52 insertions(+), 69 deletions(-) diff --git a/dix/enterleave.c b/dix/enterleave.c index 17964b00a4..7b7ba1098b 100644 --- a/dix/enterleave.c +++ b/dix/enterleave.c @@ -615,9 +615,15 @@ FixDeviceValuator(DeviceIntPtr dev, deviceValuator * ev, ValuatorClassPtr v, ev->type = DeviceValuator; ev->deviceid = dev->id; - ev->num_valuators = nval < 3 ? nval : 3; + ev->num_valuators = nval < 6 ? nval : 6; ev->first_valuator = first; switch (ev->num_valuators) { + case 6: + ev->valuator2 = v->axisVal[first + 5]; + case 5: + ev->valuator2 = v->axisVal[first + 4]; + case 4: + ev->valuator2 = v->axisVal[first + 3]; case 3: ev->valuator2 = v->axisVal[first + 2]; case 2: @@ -626,7 +632,6 @@ FixDeviceValuator(DeviceIntPtr dev, deviceValuator * ev, ValuatorClassPtr v, ev->valuator0 = v->axisVal[first]; break; } - first += ev->num_valuators; } static void @@ -646,7 +651,7 @@ FixDeviceStateNotify(DeviceIntPtr dev, deviceStateNotify * ev, KeyClassPtr k, ev->num_buttons = b->numButtons; memcpy((char *) ev->buttons, (char *) b->down, 4); } - else if (k) { + if (k) { ev->classes_reported |= (1 << KeyClass); ev->num_keys = k->xkbInfo->desc->max_key_code - k->xkbInfo->desc->min_key_code; @@ -670,15 +675,26 @@ FixDeviceStateNotify(DeviceIntPtr dev, deviceStateNotify * ev, KeyClassPtr k, } } - +/** + * The device state notify event is split across multiple 32-byte events. + * The first one contains the first 32 button state bits, the first 32 + * key state bits, and the first 3 valuator values. + * + * If a device has more than that, the server sends out: + * - one deviceButtonStateNotify for buttons 32 and above + * - one deviceKeyStateNotify for keys 32 and above + * - one deviceValuator event per 6 valuators above valuator 4 + * + * All events but the last one have the deviceid binary ORed with MORE_EVENTS, + */ static void DeliverStateNotifyEvent(DeviceIntPtr dev, WindowPtr win) { + /* deviceStateNotify, deviceKeyStateNotify, deviceButtonStateNotify + * and one deviceValuator for each 6 valuators */ + deviceStateNotify sev[3 + (MAX_VALUATORS + 6)/6]; int evcount = 1; - deviceStateNotify sev[6 + (MAX_VALUATORS + 2)/3]; - deviceStateNotify *ev; - deviceKeyStateNotify *kev; - deviceButtonStateNotify *bev; + deviceStateNotify *ev = sev; KeyClassPtr k; ButtonClassPtr b; @@ -691,82 +707,49 @@ DeliverStateNotifyEvent(DeviceIntPtr dev, WindowPtr win) if ((b = dev->button) != NULL) { nbuttons = b->numButtons; - if (nbuttons > 32) + if (nbuttons > 32) /* first 32 are encoded in deviceStateNotify */ evcount++; } if ((k = dev->key) != NULL) { nkeys = k->xkbInfo->desc->max_key_code - k->xkbInfo->desc->min_key_code; - if (nkeys > 32) + if (nkeys > 32) /* first 32 are encoded in deviceStateNotify */ evcount++; - if (nbuttons > 0) { - evcount++; - } } if ((v = dev->valuator) != NULL) { nval = v->numAxes; - - if (nval > 3) - evcount++; - if (nval > 6) { - if (!(k && b)) - evcount++; - if (nval > 9) - evcount += ((nval - 7) / 3); - } + /* first three are encoded in deviceStateNotify, then + * it's 6 per deviceValuator event */ + evcount += ((nval - 3) + 6)/6; } - ev = sev; - FixDeviceStateNotify(dev, ev, NULL, NULL, NULL, first); - - if (b != NULL) { - FixDeviceStateNotify(dev, ev++, NULL, b, v, first); - first += 3; - nval -= 3; - if (nbuttons > 32) { - (ev - 1)->deviceid |= MORE_EVENTS; - bev = (deviceButtonStateNotify *) ev++; - bev->type = DeviceButtonStateNotify; - bev->deviceid = dev->id; - memcpy((char *) &bev->buttons[4], (char *) &b->down[4], - DOWN_LENGTH - 4); - } - if (nval > 0) { - (ev - 1)->deviceid |= MORE_EVENTS; - FixDeviceValuator(dev, (deviceValuator *) ev++, v, first); - first += 3; - nval -= 3; - } + BUG_RETURN(evcount <= ARRAY_SIZE(sev)); + + FixDeviceStateNotify(dev, ev, k, b, v, first); + + if (b != NULL && nbuttons > 32) { + deviceButtonStateNotify *bev = (deviceButtonStateNotify *) ++ev; + (ev - 1)->deviceid |= MORE_EVENTS; + bev->type = DeviceButtonStateNotify; + bev->deviceid = dev->id; + memcpy((char *) &bev->buttons[4], (char *) &b->down[4], + DOWN_LENGTH - 4); } - if (k != NULL) { - FixDeviceStateNotify(dev, ev++, k, NULL, v, first); - first += 3; - nval -= 3; - if (nkeys > 32) { - (ev - 1)->deviceid |= MORE_EVENTS; - kev = (deviceKeyStateNotify *) ev++; - kev->type = DeviceKeyStateNotify; - kev->deviceid = dev->id; - memmove((char *) &kev->keys[0], (char *) &k->down[4], 28); - } - if (nval > 0) { - (ev - 1)->deviceid |= MORE_EVENTS; - FixDeviceValuator(dev, (deviceValuator *) ev++, v, first); - first += 3; - nval -= 3; - } + if (k != NULL && nkeys > 32) { + deviceKeyStateNotify *kev = (deviceKeyStateNotify *) ++ev; + (ev - 1)->deviceid |= MORE_EVENTS; + kev->type = DeviceKeyStateNotify; + kev->deviceid = dev->id; + memmove((char *) &kev->keys[0], (char *) &k->down[4], 28); } + first = 3; + nval -= 3; while (nval > 0) { - FixDeviceStateNotify(dev, ev++, NULL, NULL, v, first); - first += 3; - nval -= 3; - if (nval > 0) { - (ev - 1)->deviceid |= MORE_EVENTS; - FixDeviceValuator(dev, (deviceValuator *) ev++, v, first); - first += 3; - nval -= 3; - } + ev->deviceid |= MORE_EVENTS; + FixDeviceValuator(dev, (deviceValuator *) ++ev, v, first); + first += 6; + nval -= 6; } DeliverEventsToWindow(dev, win, (xEvent *) sev, evcount, -- GitLab