From 375d65aa2e47478331f407f5b95a4fef6200fc26 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 20 Apr 2026 18:41:52 +1000 Subject: [PATCH] test/pyxtest: add test for XKB num_levels stack overflow (ZDI-CAN-30160) Add a regression test that reproduces the XKB num_levels stack overflow. The test sends an XkbSetMap request with XkbSetMapResizeTypes that includes a non-canonical key type with numLevels=255, exceeding XkbMaxShiftLevel (63). Without the fix, this type would be accepted and stored in the server's type table. A subsequent ChangeKeyboardMapping would trigger XkbUpdateKeyTypesFromCore -> XkbKeyTypesForCoreSymbols, where the oversized num_levels is used as groupsWidth, causing indices into the tsyms[252] stack buffer to reach up to 1019 and overflow. Assisted-by: Claude:claude-opus-4-6 Part-of: --- test/pyxtest/proto/x11.py | 33 ++++ test/pyxtest/proto/xkb.py | 307 ++++++++++++++++++++++++++++++++++++++ test/pyxtest/test_xkb.py | 180 ++++++++++++++++++++++ test/pyxtest/xclient.py | 31 ++++ 4 files changed, 551 insertions(+) diff --git a/test/pyxtest/proto/x11.py b/test/pyxtest/proto/x11.py index 6a0b40dc5..c1872ff91 100644 --- a/test/pyxtest/proto/x11.py +++ b/test/pyxtest/proto/x11.py @@ -10,6 +10,7 @@ CreateWindow = 1 CreatePixmap = 53 InternAtom = 16 QueryExtension = 98 +ChangeKeyboardMapping = 100 ForceScreenSaverOpcode = 115 @@ -123,6 +124,38 @@ class QueryExtensionRequest: ) +@dataclass +class ChangeKeyboardMappingRequest: + """X11 ChangeKeyboardMapping request (opcode 100). + + Followed by keyCodes * keySymsPerKeyCode KeySym (CARD32) values. + """ + + first_keycode: int + keysyms_per_keycode: int + keycodes: int = 1 + keysyms: list[int] | None = None + + def to_bytes(self, byte_order: str = "<") -> bytes: + if self.keysyms is None: + syms = [0] * (self.keycodes * self.keysyms_per_keycode) + else: + syms = self.keysyms + + n_syms = len(syms) + req_len = 2 + n_syms # 8 bytes header = 2 words, plus 1 word per KeySym + header = struct.pack( + f"{byte_order}BBH BB xx", + ChangeKeyboardMapping, + self.keycodes, + req_len, + self.first_keycode, + self.keysyms_per_keycode, + ) + sym_data = b"".join(struct.pack(f"{byte_order}I", s) for s in syms) + return header + sym_data + + @dataclass class ForceScreenSaver: """X11 ForceScreenSaver request.""" diff --git a/test/pyxtest/proto/xkb.py b/test/pyxtest/proto/xkb.py index 48bd2ece0..433f2884e 100644 --- a/test/pyxtest/proto/xkb.py +++ b/test/pyxtest/proto/xkb.py @@ -57,6 +57,19 @@ XkbNumRequiredTypes = 4 XkbMaxLegalKeyCode = 255 XkbNoShape = 0xFF +# XkbAllMapComponentsMask +XkbAllClientInfoMask = XkbKeyTypesMask | XkbKeySymsMask | XkbModifierMapMask +XkbAllServerInfoMask = ( + XkbExplicitComponentsMask + | XkbKeyActionsMask + | XkbKeyBehaviorsMask + | XkbVirtualModsMask + | XkbVirtualModMapMask +) +XkbAllMapComponentsMask = XkbAllClientInfoMask | XkbAllServerInfoMask + +XkbNumKbdGroups = 4 + @dataclass class UseExtensionRequest: @@ -139,6 +152,300 @@ class GetMapRequest: ) +@dataclass +class GetMapReply: + """Parsed xkbGetMapReply (40-byte header + variable-length payload).""" + + device_id: int = 0 + min_key_code: int = 0 + max_key_code: int = 0 + present: int = 0 + first_type: int = 0 + n_types: int = 0 + total_types: int = 0 + first_key_sym: int = 0 + total_syms: int = 0 + n_key_syms: int = 0 + first_key_act: int = 0 + total_acts: int = 0 + n_key_acts: int = 0 + first_key_behavior: int = 0 + n_key_behaviors: int = 0 + total_key_behaviors: int = 0 + first_key_explicit: int = 0 + n_key_explicit: int = 0 + total_key_explicit: int = 0 + first_mod_map_key: int = 0 + n_mod_map_keys: int = 0 + total_mod_map_keys: int = 0 + first_vmod_map_key: int = 0 + n_vmod_map_keys: int = 0 + total_vmod_map_keys: int = 0 + virtual_mods: int = 0 + + # Parsed variable-length payload sections. + types: list["ParsedKeyType"] | None = None + sym_maps: list["ParsedSymMap"] | None = None + explicit_map: dict[int, int] | None = None + + @classmethod + def from_bytes(cls, header_data: bytes, extra_data: bytes) -> "GetMapReply": + """Parse from a 32-byte reply header + extra data. + + The standard X11 reply header is 32 bytes. The xkbGetMapReply + is 40 bytes, so bytes 32-39 spill into extra_data. After the + 40-byte header the variable-length component data follows in a + fixed order: types, syms, actions, behaviors, virtual mods, + explicit, modifier map, virtual mod map. + """ + if len(header_data) < 32: + raise ValueError(f"Header too short: {len(header_data)}") + + # Parse the first 28 bytes (bytes 0-27 of the 40-byte reply) + ( + _type, + device_id, + _seq, + _length, + min_key_code, + max_key_code, + present, + first_type, + n_types, + total_types, + first_key_sym, + total_syms, + n_key_syms, + first_key_act, + total_acts, + n_key_acts, + first_key_behavior, + n_key_behaviors, + total_key_behaviors, + ) = struct.unpack_from(" 0: + acts_counts_size = (n_key_acts + 3) & ~3 + offset += acts_counts_size + total_acts * 8 + + # 4. Key behaviors (skip) + if total_key_behaviors > 0: + offset += total_key_behaviors * 4 + + # 5. Virtual mods (skip) + if virtual_mods: + n_vmods = bin(virtual_mods).count("1") + offset += (n_vmods + 3) & ~3 + + # 6. Explicit components + explicit_map, consumed = parse_explicit_map(data[offset:], total_key_explicit) + offset += consumed + + return cls( + device_id=device_id, + min_key_code=min_key_code, + max_key_code=max_key_code, + present=present, + first_type=first_type, + n_types=n_types, + total_types=total_types, + first_key_sym=first_key_sym, + total_syms=total_syms, + n_key_syms=n_key_syms, + first_key_act=first_key_act, + total_acts=total_acts, + n_key_acts=n_key_acts, + first_key_behavior=first_key_behavior, + n_key_behaviors=n_key_behaviors, + total_key_behaviors=total_key_behaviors, + first_key_explicit=first_key_explicit, + n_key_explicit=n_key_explicit, + total_key_explicit=total_key_explicit, + first_mod_map_key=first_mod_map_key, + n_mod_map_keys=n_mod_map_keys, + total_mod_map_keys=total_mod_map_keys, + first_vmod_map_key=first_vmod_map_key, + n_vmod_map_keys=n_vmod_map_keys, + total_vmod_map_keys=total_vmod_map_keys, + virtual_mods=virtual_mods, + types=types, + sym_maps=sym_maps, + explicit_map=explicit_map, + ) + + +@dataclass +class ParsedKeyType: + """A parsed key type from XkbGetMap reply data.""" + + mask: int + real_mods: int + virtual_mods: int + num_levels: int + n_map_entries: int + has_preserve: bool + raw_wire: bytes # The complete wire data (header + entries + preserve) + + def to_set_map_wire(self, num_levels: int | None = None) -> bytes: + """Convert to SetMap wire format. + + GetMap uses xkbKTMapEntryWireDesc (8 bytes per entry): + active(1), mask(1), level(1), realMods(1), virtualMods(2), pad(2) + SetMap uses xkbKTSetMapEntryWireDesc (4 bytes per entry): + level(1), realMods(1), virtualMods(2) + Preserve entries (xkbModsWireDesc, 4 bytes) are the same in both. + """ + if num_levels is None: + num_levels = self.num_levels + # Rebuild the 8-byte header with the (possibly new) num_levels + header = struct.pack( + " tuple[list[ParsedKeyType], int]: + """Parse n_types key type records from wire data. + + Returns (list of ParsedKeyType, bytes consumed). + """ + types = [] + offset = 0 + for _ in range(n_types): + if offset + 8 > len(data): + break + mask, real_mods, virtual_mods, num_levels, n_map_entries, preserve, _pad = ( + struct.unpack_from(" tuple[list[ParsedSymMap], int]: + """Parse n_key_syms sym map records from wire data. + + Returns (list of ParsedSymMap, bytes consumed). + """ + maps = [] + offset = 0 + for _ in range(n_key_syms): + if offset + 8 > len(data): + break + kt0, kt1, kt2, kt3, group_info, width, n_syms = struct.unpack_from( + " tuple[dict[int, int], int]: + """Parse explicit component (keycode, explicit) byte pairs. + + Returns (dict mapping keycode -> explicit flags, bytes consumed). + """ + explicit = {} + offset = 0 + for _ in range(total_key_explicit): + if offset + 2 > len(data): + break + keycode = data[offset] + flags = data[offset + 1] + explicit[keycode] = flags + offset += 2 + # Pad to 4-byte boundary + padded = (offset + 3) & ~3 + return explicit, padded + + @dataclass class SetMapRequest: """ diff --git a/test/pyxtest/test_xkb.py b/test/pyxtest/test_xkb.py index debcadc2c..0908dc696 100644 --- a/test/pyxtest/test_xkb.py +++ b/test/pyxtest/test_xkb.py @@ -2,6 +2,7 @@ # # Security tests for XKB extension vulnerabilities. +import logging import struct import time @@ -490,3 +491,182 @@ class TestXkbSetNames: time.sleep(0.5) assert xserver.is_alive, "Server crashed - truncated atoms in SetNames" + + +class TestXkbSetMapNumLevels: + """Tests for XKB SetMap num_levels validation.""" + + @pytest.mark.asan + def test_num_levels_exceeds_max_shift_level(self, xserver, xkb_xclient): + """ + ZDI-CAN-30160: CheckKeyTypes did not enforce an upper bound on + numLevels for non-canonical key types. A client could set + numLevels up to 255 via XkbSetMap. When ChangeKeyboardMapping + later triggers XkbUpdateKeyTypesFromCore, the function + XkbKeyTypesForCoreSymbols uses num_levels as groupsWidth and + indexes into tsyms[], a stack buffer of XkbMaxSymsPerKey (252) + entries. With num_levels=255, indices reach up to 1019, + overflowing the 252-element stack buffer. + + Fixed by rejecting numLevels > XkbMaxShiftLevel (63) in + CheckKeyTypes alongside the existing check for numLevels < 1. + """ + EVIL_NUM_LEVELS = 255 + + xclient, opcode = xkb_xclient + + # Step 1: Get the full XKB map to discover types and key mappings. + map_reply = xclient.xkb_get_map(opcode, full=xkb.XkbAllMapComponentsMask) + assert map_reply is not None, "XkbGetMap failed" + + types = map_reply.types + sym_maps = map_reply.sym_maps + explicit_map = map_reply.explicit_map + + logging.debug( + f"We have {len(types)} types, keycodes are {map_reply.min_key_code}-{map_reply.max_key_code}" + ) + logging.debug("Types:") + for idx, t in enumerate(types): + logging.debug( + f"type[{idx:02d}]: num_levels={t.num_levels} {'canonical' if idx < 4 else ''}" + ) + + # Step 2: Find a key with a non-canonical type (kt_index >= 4). + # Prefer one that already has the explicit flag set. + target_key = -1 + target_type = -1 + target_group = -1 + has_explicit = False + + logging.debug("Scanning keys for non-canonical type with explicit flag") + for i, sm in enumerate(sym_maps): + keycode = map_reply.first_key_sym + i + n_groups = sm.group_info & 0x0F + expl = explicit_map.get(keycode, 0) + + for g in range(min(n_groups, 4)): + kt = sm.kt_index[g] + if kt >= 4 and (expl & (1 << g)): + logging.debug( + f"FOUND: key={keycode} group={g} kt_index={kt} num_levels={types[kt].num_levels} explicit=0x{expl:02x}" + ) + # Found a key with explicit flag already set. + if not has_explicit: + target_key = keycode + target_type = kt + target_group = g + has_explicit = True + + # If none found with explicit flag, find any key using type >= 4. + if target_key < 0: + logging.debug( + "No key with explicit + non-canonical type found. Scanning for any type >=4" + ) + for i, sm in enumerate(sym_maps): + keycode = map_reply.first_key_sym + i + n_groups = sm.group_info & 0x0F + for g in range(min(n_groups, 4)): + kt = sm.kt_index[g] + if kt >= 4: + logging.debug( + f"FOUND: key={keycode} group={g} type={kt} num_levels={types[kt].num_levels}" + ) + target_key = keycode + target_type = kt + target_group = g + break + if target_key >= 0: + break + + if target_key < 0: + pytest.skip("No key using non-canonical type found") + + logging.debug( + f"Target: key={target_key} group={target_group} type={target_type}" + ) + + # Step 2b: Set the explicit flag on the target key if needed. + if not has_explicit: + expl_flags = explicit_map.get(target_key, 0) + expl_flags |= 1 << target_group + # Build explicit component payload: (keycode, flags) pairs + # for all keys that have non-zero explicit, plus our target. + explicit_map[target_key] = expl_flags + expl_payload = b"" + for kc in sorted(explicit_map): + expl_payload += bytes([kc, explicit_map[kc]]) + pad_len = (4 - len(expl_payload) % 4) % 4 + expl_payload += b"\x00" * pad_len + + req = xkb.SetMapRequest( + opcode=opcode, + present=xkb.XkbExplicitComponentsMask, + first_key_explicit=map_reply.first_key_explicit, + n_key_explicit=map_reply.n_key_explicit, + total_key_explicit=len(explicit_map), + min_key_code=map_reply.min_key_code, + max_key_code=map_reply.max_key_code, + payload=expl_payload, + ) + xclient.send_request(req.to_bytes()) + resps = xclient.flush_responses(timeout=0.5) + errors = [r for r in resps if isinstance(r, X11Error)] + assert not errors, f"SetMap(explicit) failed: {errors}" + + # Step 3: Modify the target type's num_levels to 255 via + # XkbSetMap with XkbKeyTypesMask. Send all types (like Xlib + # does), with the target type's num_levels changed. + # Note: GetMap and SetMap use different wire formats for map entries + # (8-byte xkbKTMapEntryWireDesc vs 4-byte xkbKTSetMapEntryWireDesc), + # so we must convert via to_set_map_wire(). + types_payload = b"" + for i, t in enumerate(types): + if i == target_type: + logging.debug( + f"Modifying key type {i} to have num_levels {EVIL_NUM_LEVELS}" + ) + types_payload += t.to_set_map_wire(num_levels=EVIL_NUM_LEVELS) + else: + types_payload += t.to_set_map_wire() + + req = xkb.SetMapRequest( + opcode=opcode, + present=xkb.XkbKeyTypesMask, + first_type=0, + n_types=len(types), + min_key_code=map_reply.min_key_code, + max_key_code=map_reply.max_key_code, + payload=types_payload, + ) + xclient.send_request(req.to_bytes()) + resps = xclient.flush_responses(timeout=0.5) + errors = [r for r in resps if isinstance(r, X11Error)] + + # On a patched server, CheckKeyTypes rejects numLevels > XkbMaxShiftLevel (63). + # The SetMap must fail with an error. + assert errors, ( + f"SetMap with num_levels={EVIL_NUM_LEVELS} was accepted - " + "server is missing the numLevels upper bound check (ZDI-CAN-30160)" + ) + logging.debug(f"SetMap correctly rejected: {errors}") + + # Step 4: Trigger via ChangeKeyboardMapping on the target key. + # On an unpatched server where the evil num_levels was accepted, + # XkbUpdateKeyTypesFromCore would use it as groupsWidth, + # overflowing the stack-allocated tsyms[252] buffer. + # On a patched server the SetMap was rejected above, so this + # is harmless — but we send it anyway to confirm the server + # stays alive regardless. + keysyms = [0x41414141 + i for i in range(8)] + xclient.change_keyboard_mapping( + first_keycode=target_key, + keysyms_per_keycode=8, + keycodes=1, + keysyms=keysyms, + ) + time.sleep(0.5) + + assert xserver.is_alive, ( + "Server crashed - numLevels > XkbMaxShiftLevel (ZDI-CAN-30160)" + ) diff --git a/test/pyxtest/xclient.py b/test/pyxtest/xclient.py index 075a855e3..8ef1e5e7b 100644 --- a/test/pyxtest/xclient.py +++ b/test/pyxtest/xclient.py @@ -12,6 +12,7 @@ from typing import Protocol, runtime_checkable from proto.bigrequests import BigRequestsEnableRequest from proto.x11 import ( + ChangeKeyboardMappingRequest, CreatePixmapRequest, CreateWindowRequest, InternAtomRequest, @@ -463,6 +464,36 @@ class RawX11Connection: return struct.unpack_from(f"{self._byte_order}I", resp.data, 8)[0] return 0 + def xkb_get_map( + self, opcode: int, full: int = 0, partial: int = 0, **kwargs + ) -> xkb.GetMapReply | None: + """Send XkbGetMap and return the parsed reply.""" + req = xkb.GetMapRequest(opcode=opcode, full=full, partial=partial, **kwargs) + self.send_request(req.to_bytes(self._byte_order)) + resp = self.recv_response(timeout=5.0) + if isinstance(resp, X11Error) or resp is None: + return None + # resp.data is the full reply: 32 standard header + extra + header_data = resp.data[:32] + extra_data = resp.data[32:] + return xkb.GetMapReply.from_bytes(header_data, extra_data) + + def change_keyboard_mapping( + self, + first_keycode: int, + keysyms_per_keycode: int, + keycodes: int = 1, + keysyms: list[int] | None = None, + ) -> None: + """Send ChangeKeyboardMapping request.""" + req = ChangeKeyboardMappingRequest( + first_keycode=first_keycode, + keysyms_per_keycode=keysyms_per_keycode, + keycodes=keycodes, + keysyms=keysyms, + ) + self.send_request(req.to_bytes(self._byte_order)) + def get_fd(self) -> int: assert self.sock return self.sock.fileno()