diff --git a/dix/devices.c b/dix/devices.c index fd7c0542d9..c35e2b41a1 100644 --- a/dix/devices.c +++ b/dix/devices.c @@ -1162,6 +1162,40 @@ CloseOneDevice(const DeviceIntPtr dev, DeviceIntPtr *listHead) return BadMatch; } +static Bool +GrabReferencesDevice(void *value, XID id, void *cdata) +{ + GrabPtr grab = value; + DeviceIntPtr dev = cdata; + + return grab->device == dev || grab->modifierDevice == dev; +} + +/** + * Remove all passive grabs that reference the given device, either as + * the grabbed device or as the modifier device. This must be called + * before the device is freed to avoid dangling pointers in GrabRec. + */ +static void +RemovePassiveGrabsForDevice(DeviceIntPtr dev) +{ + int i; + + for (i = 0; i < currentMaxClients; i++) { + GrabPtr grab; + + if (!clients[i]) + continue; + + while ((grab = LookupClientResourceComplex(clients[i], + X11_RESTYPE_PASSIVEGRAB, + GrabReferencesDevice, + dev))) { + FreeResource(grab->resource, X11_RESTYPE_NONE); + } + } +} + /** * Remove a device from the device list, closes it and thus frees all * resources. @@ -1197,6 +1231,8 @@ RemoveDevice(DeviceIntPtr dev, BOOL sendevent) flags[dev->id] = XIDeviceDisabled; } + RemovePassiveGrabsForDevice(dev); + int flag = InputDevIsMaster(dev) ? XIMasterRemoved : XISlaveRemoved; input_lock(); diff --git a/test/pyxtest/proto/xi.py b/test/pyxtest/proto/xi.py index f1cdcbe231..3f48a2f0a4 100644 --- a/test/pyxtest/proto/xi.py +++ b/test/pyxtest/proto/xi.py @@ -6,18 +6,41 @@ import struct from dataclasses import dataclass # XI (v1) minor opcodes +XGrabDeviceButton = 17 XChangeDeviceControl = 35 XChangeDeviceProperty = 37 XGetDeviceProperty = 39 # XI2 minor opcodes +XIChangeCursor = 42 +XIChangeHierarchy = 43 XIQueryVersion = 47 +XIQueryDevice = 48 XIPassiveGrabDevice = 54 XIPassiveUngrabDevice = 55 XIChangeProperty = 57 XIGetProperty = 59 XIBarrierReleasePointer = 61 +# XIChangeHierarchy change types +XIAddMaster = 1 +XIRemoveMaster = 2 + +# XIRemoveMaster return modes +XIAttachToMaster = 1 +XIFloating = 2 + +# XIQueryDevice special device IDs +# XIAllDevices = 0 (already defined below) +# XIAllMasterDevices = 1 (already defined below) + +# XI device use types (from xXIDeviceInfo) +XIMasterPointer = 1 +XIMasterKeyboard = 2 +XISlavePointer = 3 +XISlaveKeyboard = 4 +XIFloatingSlave = 5 + XI2_MAJOR = 2 XI2_MINOR = 4 @@ -374,7 +397,6 @@ class DeviceResolutionCtl: return header + val_data - @dataclass class XIAddMasterInfo: """XIAddMaster change body for XIChangeHierarchy. diff --git a/test/pyxtest/test_xi.py b/test/pyxtest/test_xi.py index e1cdbd242f..8c6c86cb86 100644 --- a/test/pyxtest/test_xi.py +++ b/test/pyxtest/test_xi.py @@ -10,6 +10,25 @@ from proto import x11, xfixes, xi, xtest from xclient import BadLength, BadValue, Extension, X11Error, X11Reply +def find_device( + resp, name_prefix: str, use: int | None = None, byte_order: str = "<" +) -> int | None: + """Parse an XIQueryDevice reply to find a master device by name. + + Returns the device ID or None if not found. + """ + if not isinstance(resp, X11Reply): + return None + + reply = xi.XIQueryDeviceReply.from_reply(resp.data, byte_order) + + for device in reply.devices: + if (use is None or device.use == use) and device.name.startswith(name_prefix): + return device.deviceid + + return None + + @pytest.fixture def xi_xclient(xclient): """Provide an xclient with XI2 initialized.""" @@ -170,6 +189,96 @@ class TestXIPassiveGrab: f"Expected BadValue ({x11.BadValue}), got error code {resp.error_code}" ) + @pytest.mark.asan + def test_passive_grab_modifier_device_uaf(self, xserver, xi_xclient): + """ + ZDI-CAN-31832: Use-after-free in CheckPassiveGrab via + grab->modifierDevice after device removal. + + XGrabDeviceButton creates a passive grab with grabtype XI. The grab + stores a raw DeviceIntPtr in grab->modifierDevice without any reference + counting or lifetime management. + + When the modifier device (a master keyboard) is removed via + XIChangeHierarchy(XIRemoveMaster), CloseDevice frees + the device struct, but doesn't clear the now-dangling + grab->modifierDevice pointer. + + A subsequent pointer event routed to the grab window triggers + CheckPassiveGrabsOnWindow -> CheckPassiveGrab, which + dereferences the freed device via gdev = grab->modifierDevice + then reads gdev->key. + """ + conn = xi_xclient + opcode = conn.query_extension(Extension.XI).opcode + + # 1. Create a window for the grab target + wid = conn.create_window() + req = x11.MapWindowRequest(window=wid) + conn.send_request(req) + conn.flush_responses(timeout=0.5) + + # 2. Create a new master device pair "uaf" + add_info = xi.XIAddMasterInfo(name="uaf", send_core=1, enable=1) + req = xi.XIChangeHierarchyRequest( + opcode=opcode, + num_changes=1, + changes_data=add_info.to_bytes("<"), + ) + conn.send_request(req) + conn.flush_responses(timeout=1.0) + + req = xi.XIQueryDeviceRequest(opcode=opcode, deviceid=xi.XIAllDevices) + conn.send_request(req) + resp = conn.recv_response(timeout=2.0) + kbd_id = find_device(resp, name_prefix="uaf", use=xi.XIMasterKeyboard) + if kbd_id is None: + pytest.fail("Could not find 'uaf' master keyboard device") + + # 3. XGrabDeviceButton: passive grab on VCP with modifierDevice=kbd_id. + # This creates a grabtype=XI grab with modifierDevice pointing + # to the new master keyboard. + req = xi.XGrabDeviceButtonRequest( + opcode=opcode, + grab_window=wid, + grabbed_device=xi.VirtualCorePointer, + modifier_device=kbd_id, + button=0, # AnyButton + modifiers=0x8000, # AnyModifier + ) + conn.send_request(req) + conn.flush_responses(timeout=0.5) + + # 4. Remove the master device - this frees the device struct + rem_info = xi.XIRemoveMasterInfo( + deviceid=kbd_id, + return_mode=xi.XIFloating, + ) + req = xi.XIChangeHierarchyRequest( + opcode=opcode, + num_changes=1, + changes_data=rem_info.to_bytes("<"), + ) + conn.send_request(req) + conn.flush_responses(timeout=1.0) + + # 6. Warp the pointer into the grab window to trigger + # CheckMotion -> ActivateEnterGrab -> CheckPassiveGrab -> UAF + req = x11.WarpPointerRequest( + dst_window=wid, + dst_x=50, + dst_y=50, + ) + conn.send_request(req) + conn.flush_responses(timeout=0.5) + + time.sleep(0.5) + + assert xserver.is_alive, ( + "Server crashed - passive grab modifierDevice " + "use-after-free on device removal (ZDI-CAN-31832)" + ) + class TestXIChangeProperty: """Tests for XIChangeProperty vulnerabilities."""