summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPeter Hutterer <peter.hutterer@who-t.net>2022-11-29 13:55:32 +1000
committerPeter Hutterer <peter.hutterer@who-t.net>2022-12-14 11:30:10 +1000
commitaf8612df4e27daf0b0611f014a5105b926f19c2c (patch)
treeaddc1d0061673ba249a09a88e5a5d3845cc8baa2
parentadaf080bd55a7f08711a612c5562ef10f6c3223c (diff)
downloadxserver-af8612df4e27daf0b0611f014a5105b926f19c2c.tar.gz
Xi: disallow passive grabs with a detail > 255
The XKB protocol effectively prevents us from ever using keycodes above 255. For buttons it's theoretically possible but realistically too niche to worry about. For all other passive grabs, the detail must be zero anyway. This fixes an OOB write: ProcXIPassiveUngrabDevice() calls DeletePassiveGrabFromList with a temporary grab struct which contains tempGrab->detail.exact = stuff->detail. For matching existing grabs, DeleteDetailFromMask is called with the stuff->detail value. This function creates a new mask with the one bit representing stuff->detail cleared. However, the array size for the new mask is 8 * sizeof(CARD32) bits, thus any detail above 255 results in an OOB array write. CVE-2022-46341, ZDI-CAN 19381 This vulnerability was discovered by: Jan-Niklas Sohn working with Trend Micro Zero Day Initiative Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net> Acked-by: Olivier Fourdan <ofourdan@redhat.com> (cherry picked from commit 51eb63b0ee1509c6c6b8922b0e4aa037faa6f78b)
-rw-r--r--Xi/xipassivegrab.c22
1 files changed, 14 insertions, 8 deletions
diff --git a/Xi/xipassivegrab.c b/Xi/xipassivegrab.c
index d30f51f3c..89a591098 100644
--- a/Xi/xipassivegrab.c
+++ b/Xi/xipassivegrab.c
@@ -133,6 +133,12 @@ ProcXIPassiveGrabDevice(ClientPtr client)
return BadValue;
}
+ /* XI2 allows 32-bit keycodes but thanks to XKB we can never
+ * implement this. Just return an error for all keycodes that
+ * cannot work anyway, same for buttons > 255. */
+ if (stuff->detail > 255)
+ return XIAlreadyGrabbed;
+
if (XICheckInvalidMaskBits(client, (unsigned char *) &stuff[1],
stuff->mask_len * 4) != Success)
return BadValue;
@@ -203,14 +209,8 @@ ProcXIPassiveGrabDevice(ClientPtr client)
&param, XI2, &mask);
break;
case XIGrabtypeKeycode:
- /* XI2 allows 32-bit keycodes but thanks to XKB we can never
- * implement this. Just return an error for all keycodes that
- * cannot work anyway */
- if (stuff->detail > 255)
- status = XIAlreadyGrabbed;
- else
- status = GrabKey(client, dev, mod_dev, stuff->detail,
- &param, XI2, &mask);
+ status = GrabKey(client, dev, mod_dev, stuff->detail,
+ &param, XI2, &mask);
break;
case XIGrabtypeEnter:
case XIGrabtypeFocusIn:
@@ -319,6 +319,12 @@ ProcXIPassiveUngrabDevice(ClientPtr client)
return BadValue;
}
+ /* We don't allow passive grabs for details > 255 anyway */
+ if (stuff->detail > 255) {
+ client->errorValue = stuff->detail;
+ return BadValue;
+ }
+
rc = dixLookupWindow(&win, stuff->grab_window, client, DixSetAttrAccess);
if (rc != Success)
return rc;