From 845eb56df287c9a166891983df9db7c060e6f124 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 20 Apr 2026 16:36:34 +1000 Subject: [PATCH] pyxtest: add test cases for the various XKB CVEs from the last few years Commit 446ff2d31770 ("Check SetMap request length carefully.") Commit 6907b6ea2b4c ("xkb: add request length validation for XkbSetGeometry") Commit 4cd853321094 ("xkb: Fix buffer overflow in _XkbSetCompatMap()") Commit f7cd1276bbd4 ("Correct bounds checking in XkbSetNames()") Assisted-by: Claude:claude-claude-opus-4-6 Part-of: --- test/pyxtest/meson.build | 1 + test/pyxtest/proto/xkb.py | 207 +++++++++++++++++++++++++++++++-- test/pyxtest/test_xkb.py | 234 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 435 insertions(+), 7 deletions(-) create mode 100644 test/pyxtest/test_xkb.py diff --git a/test/pyxtest/meson.build b/test/pyxtest/meson.build index a3b7a1b20..e1e76aa6b 100644 --- a/test/pyxtest/meson.build +++ b/test/pyxtest/meson.build @@ -57,6 +57,7 @@ if pytest.found() tests_pyxtest = [ 'test_randr.py', 'test_xi.py', + 'test_xkb.py', ] test_list_data = configuration_data() diff --git a/test/pyxtest/proto/xkb.py b/test/pyxtest/proto/xkb.py index 22e1bf5a1..5ec73fa15 100644 --- a/test/pyxtest/proto/xkb.py +++ b/test/pyxtest/proto/xkb.py @@ -68,10 +68,10 @@ class UseExtensionRequest: def to_bytes(self, byte_order: str = "<") -> bytes: return struct.pack( - f"{byte_order}BBHHH xx", + f"{byte_order}BBHHH", self.opcode, XkbUseExtension, - 3, # 12 bytes = 3 words + 2, # 8 bytes = 2 words self.major, self.minor, ) @@ -368,17 +368,23 @@ def build_counted_string(s: str | bytes, byte_order: str = "<") -> bytes: @dataclass class ShapeWire: """ - xkbShapeWireDesc (4 bytes) + outline data. - Each outline: header(4 bytes) + nPoints * point(4 bytes). + xkbShapeWireDesc (8 bytes) + outline data. + + Wire layout: + name(4 Atom) + nOutlines(1) + primaryNdx(1) + approxNdx(1) + pad(1) + Each outline: header(4 bytes: nPoints(1) + cornerRadius(1) + pad(2)) + + nPoints * point(4 bytes: x(2) + y(2)). """ + name: int = 0 # Atom n_outlines: int = 1 primary_ndx: int = 0 approx_ndx: int = 0 def to_bytes(self, byte_order: str = "<") -> bytes: header = struct.pack( - f"{byte_order}BBBx", + f"{byte_order}IBBBx", + self.name, self.n_outlines, self.primary_ndx, self.approx_ndx, @@ -395,6 +401,7 @@ class ShapeWire: class SectionWire: """xkbSectionWireDesc (20 bytes) + row data.""" + name: int = 0 # Atom n_rows: int = 1 n_doodads: int = 0 n_overlays: int = 0 @@ -402,7 +409,7 @@ class SectionWire: def to_bytes(self, byte_order: str = "<") -> bytes: header = struct.pack( f"{byte_order}I hh HH h BBBBxx", - 0, # name (Atom) + self.name, # name (Atom) 0, 0, # top, left 100, @@ -423,11 +430,12 @@ class SectionWire: class OverlayWire: """xkbOverlayWireDesc + overlay rows.""" + name: int = 0 # Atom n_rows: int = 1 rows_under: list[int] | None = None def to_bytes(self, byte_order: str = "<") -> bytes: - header = struct.pack(f"{byte_order}IBxxx", 0, self.n_rows) + header = struct.pack(f"{byte_order}IBxxx", self.name, self.n_rows) rows_under = ( self.rows_under if self.rows_under is not None else list(range(self.n_rows)) ) @@ -471,3 +479,188 @@ class GetKbdByNameRequest: self.load, ) return header + self.payload + b"\x00" * pad_len + + +# XkbSetNames 'which' flags +XkbKeycodesNameMask = 1 << 0 +XkbGeometryNameMask = 1 << 1 +XkbSymbolsNameMask = 1 << 2 +XkbPhysSymbolsNameMask = 1 << 3 +XkbTypesNameMask = 1 << 4 +XkbCompatNameMask = 1 << 5 +XkbKeyTypeNamesMask = 1 << 6 +XkbKTLevelNamesMask = 1 << 7 +XkbIndicatorNamesMask = 1 << 8 +XkbKeyNamesMask = 1 << 9 +XkbKeyAliasesMask = 1 << 10 +XkbVirtualModNamesMask = 1 << 11 +XkbGroupNamesMask = 1 << 12 +XkbRGNamesMask = 1 << 13 + +# XkbSetDeviceInfo 'change' flags +XkbXI_ButtonActionsMask = 1 << 0 +XkbXI_IndicatorNamesMask = 1 << 1 +XkbXI_IndicatorMapsMask = 1 << 2 +XkbXI_IndicatorStateMask = 1 << 3 +XkbXI_IndicatorsMask = 0x0E # Names | Maps | State + + +@dataclass +class SetNamesRequest: + """xkbSetNamesReq (minor opcode 18). Header is 28 bytes.""" + + opcode: int + device_spec: int = XkbUseCoreKbd + virtual_mods: int = 0 + which: int = 0 + first_type: int = 0 + n_types: int = 0 + first_kt_level: int = 0 + n_kt_levels: int = 0 + indicators: int = 0 + group_names: int = 0 + n_radio_groups: int = 0 + first_key: int = 8 + n_keys: int = 0 + n_key_aliases: int = 0 + total_kt_level_names: int = 0 + payload: bytes = b"" + length_override: int | None = None + + def to_bytes(self, byte_order: str = "<") -> bytes: + total_bytes = 28 + len(self.payload) + pad_len = (4 - total_bytes % 4) % 4 + total_bytes += pad_len + + length = ( + self.length_override + if self.length_override is not None + else total_bytes // 4 + ) + + header = struct.pack( + f"{byte_order}BBH" # reqType, xkbReqType, length + f"HH" # deviceSpec, virtualMods + f"I" # which + f"BBBB" # firstType, nTypes, firstKTLevel, nKTLevels + f"I" # indicators + f"BB" # groupNames, nRadioGroups + f"BB" # firstKey, nKeys + f"Bx" # nKeyAliases, pad + f"H", # totalKTLevelNames + self.opcode, + XkbSetNames, + length, + self.device_spec, + self.virtual_mods, + self.which, + self.first_type, + self.n_types, + self.first_kt_level, + self.n_kt_levels, + self.indicators, + self.group_names, + self.n_radio_groups, + self.first_key, + self.n_keys, + self.n_key_aliases, + self.total_kt_level_names, + ) + return header + self.payload + b"\x00" * pad_len + + +@dataclass +class SetDeviceInfoRequest: + """xkbSetDeviceInfoReq (minor opcode 25). Header is 12 bytes.""" + + opcode: int + device_spec: int = XkbUseCoreKbd + first_btn: int = 0 + n_btns: int = 0 + change: int = 0 + n_device_led_fbs: int = 0 + payload: bytes = b"" + length_override: int | None = None + + def to_bytes(self, byte_order: str = "<") -> bytes: + total_bytes = 12 + len(self.payload) + pad_len = (4 - total_bytes % 4) % 4 + total_bytes += pad_len + + length = ( + self.length_override + if self.length_override is not None + else total_bytes // 4 + ) + + header = struct.pack( + f"{byte_order}BBH HBB HH", + self.opcode, + XkbSetDeviceInfo, + length, + self.device_spec, + self.first_btn, + self.n_btns, + self.change, + self.n_device_led_fbs, + ) + return header + self.payload + b"\x00" * pad_len + + +# XkbSelectEvents event mask bits +XkbNewKeyboardNotifyMask = 1 << 0 +XkbMapNotifyMask = 1 << 1 +XkbStateNotifyMask = 1 << 2 +XkbControlsNotifyMask = 1 << 3 +XkbIndicatorStateNotifyMask = 1 << 4 +XkbIndicatorMapNotifyMask = 1 << 5 +XkbNamesNotifyMask = 1 << 6 +XkbCompatMapNotifyMask = 1 << 7 +XkbBellNotifyMask = 1 << 8 +XkbActionMessageMask = 1 << 9 +XkbAccessXNotifyMask = 1 << 10 +XkbExtensionDeviceNotifyMask = 1 << 11 + + +@dataclass +class SelectEventsRequest: + """xkbSelectEventsReq (minor opcode 1). Header is 16 bytes. + + The header is followed by per-event affect/detail masks for any + event types set in affectWhich that are NOT in selectAll or clear. + """ + + opcode: int + device_spec: int = XkbUseCoreKbd + affect_which: int = 0 + clear: int = 0 + select_all: int = 0 + affect_map: int = 0 + map: int = 0 + payload: bytes = b"" + length_override: int | None = None + + def to_bytes(self, byte_order: str = "<") -> bytes: + total_bytes = 16 + len(self.payload) + pad_len = (4 - total_bytes % 4) % 4 + total_bytes += pad_len + + length = ( + self.length_override + if self.length_override is not None + else total_bytes // 4 + ) + + header = struct.pack( + f"{byte_order}BBH HH HH HH", + self.opcode, + XkbSelectEvents, + length, + self.device_spec, + self.affect_which, + self.clear, + self.select_all, + self.affect_map, + self.map, + ) + return header + self.payload + b"\x00" * pad_len diff --git a/test/pyxtest/test_xkb.py b/test/pyxtest/test_xkb.py new file mode 100644 index 000000000..ad3f10471 --- /dev/null +++ b/test/pyxtest/test_xkb.py @@ -0,0 +1,234 @@ +# SPDX-License-Identifier: MIT +# +# Security tests for XKB extension vulnerabilities. + +import time + +import pytest + +from proto import xkb +from xclient import X11Error + + +@pytest.fixture +def xkb_xclient(xclient): + """Provide an xclient with XKB initialized.""" + opcode = xclient.xkb_use_extension() + return xclient, opcode + + +class TestXkbSetMapOverflows: + """Tests for XKB SetMap buffer overflow and OOB vulnerabilities.""" + + @pytest.mark.asan + def test_setmap_request_length_check(self, xserver, xkb_xclient): + """ + Missing length validation for variable-length XkbSetMap sub-fields. + Multiple present bits set but insufficient wire data. + + Fixed in commit 446ff2d31770 ("Check SetMap request length + carefully."). + """ + xclient, opcode = xkb_xclient + + all_flags = ( + xkb.XkbKeyTypesMask + | xkb.XkbKeySymsMask + | xkb.XkbModifierMapMask + | xkb.XkbExplicitComponentsMask + | xkb.XkbKeyActionsMask + | xkb.XkbKeyBehaviorsMask + | xkb.XkbVirtualModsMask + | xkb.XkbVirtualModMapMask + ) + + req = xkb.SetMapRequest( + opcode=opcode, + present=all_flags, + first_type=0, + n_types=1, + first_key_sym=8, + n_key_syms=1, + total_syms=1, + first_key_act=8, + n_key_acts=1, + total_acts=1, + first_key_behavior=8, + n_key_behaviors=1, + total_key_behaviors=1, + first_key_explicit=8, + n_key_explicit=1, + total_key_explicit=1, + first_mod_map_key=8, + n_mod_map_keys=1, + total_mod_map_keys=1, + first_vmod_map_key=8, + n_vmod_map_keys=1, + total_vmod_map_keys=1, + virtual_mods=0xFFFF, + min_key_code=8, + max_key_code=255, + payload=b"\x00" * 8, # Way too little data + ) + xclient.send_request(req.to_bytes()) + time.sleep(0.5) + + assert xserver.is_alive, "Server crashed - missing length checks in SetMap" + + +class TestXkbSetGeometry: + """Tests for XKB SetGeometry OOB vulnerabilities.""" + + def _build_colors(self, n, byte_order="<"): + """Build n color strings for geometry payload.""" + data = b"" + colors = [ + "white", + "black", + "red", + "green", + "blue", + "yellow", + "cyan", + "magenta", + ] + for i in range(n): + data += xkb.build_counted_string( + colors[i % len(colors)], byte_order=byte_order + ) + return data + + def test_setgeometry_truncated_sections(self, xserver, xkb_xclient): + """ + Missing bounds checks on nested structures in XkbSetGeometry. + Valid-looking headers but truncated wire data for sections, + rows, and overlays. The request claims 5 sections but provides + none, so the server must reject it with BadLength. + + Fixed in commit 6907b6ea2b4c ("xkb: add request length validation + for XkbSetGeometry"). + """ + xclient, opcode = xkb_xclient + + n_colors = 2 + color_data = self._build_colors(n_colors) + name_atom = xclient.intern_atom("TestGeom6") + + shape_data = xkb.ShapeWire(n_outlines=1).to_bytes() + + req = xkb.SetGeometryRequest( + opcode=opcode, + n_shapes=1, + n_sections=5, # Claim 5 sections but provide none + n_colors=n_colors, + base_color_ndx=0, + label_color_ndx=1, + name_atom=name_atom, + payload=color_data + shape_data, + ) + xclient.send_request(req.to_bytes()) + resp = xclient.recv_response(timeout=2.0) + + assert xserver.is_alive, "Server crashed - truncated sections in SetGeometry" + assert isinstance(resp, X11Error), f"Expected an error, got {resp}" + assert resp.error_code == 16, ( + f"Expected BadLength (16), got error code {resp.error_code} - " + f"missing bounds check in SetGeometry section parsing" + ) + + +class TestXkbSetCompatMap: + """Tests for XKB SetCompatMap vulnerabilities.""" + + @pytest.mark.asan + def test_setcompatmap_size_vs_num_overflow(self, xserver, xkb_xclient): + """ + _XkbSetCompatMap compared firstSI + nSI against compat->num_si + (logical count) instead of compat->size_si (allocated size). + After a truncation that lowered num_si below size_si, a + subsequent non-truncating request with firstSI + nSI between + num_si and size_si would skip the realloc and write past the + allocated buffer. + + Fixed in commit 4cd853321094 ("xkb: Fix buffer overflow in + _XkbSetCompatMap()"). + """ + xclient, opcode = xkb_xclient + + # Step 1: Allocate 10 entries (sets size_si=10, num_si=10) + payload1 = b"".join(xkb.SymInterpretWire().to_bytes() for _ in range(10)) + req1 = xkb.SetCompatMapRequest( + opcode=opcode, + first_si=0, + n_si=10, + truncate_si=1, + payload=payload1, + ) + xclient.send_request(req1.to_bytes()) + xclient.flush_responses(timeout=0.5) + + # Step 2: Truncate to 2 (sets num_si=2, size_si stays 10) + payload2 = b"".join(xkb.SymInterpretWire().to_bytes() for _ in range(2)) + req2 = xkb.SetCompatMapRequest( + opcode=opcode, + first_si=0, + n_si=2, + truncate_si=1, + payload=payload2, + ) + xclient.send_request(req2.to_bytes()) + xclient.flush_responses(timeout=0.5) + + # Step 3: Write at index 8-11 without truncation. + # num_si=2, size_si=10. Old code checks firstSI+nSI > num_si + # and reallocates, but should check size_si. + payload3 = b"".join(xkb.SymInterpretWire().to_bytes() for _ in range(4)) + req3 = xkb.SetCompatMapRequest( + opcode=opcode, + first_si=8, + n_si=4, # 8+4=12 > size_si=10 + truncate_si=0, + payload=payload3, + ) + xclient.send_request(req3.to_bytes()) + time.sleep(0.5) + + assert xserver.is_alive, ( + "Server crashed - SetCompatMap size_si vs num_si overflow" + ) + + +class TestXkbSetNames: + """Tests for XKB SetNames vulnerabilities.""" + + @pytest.mark.asan + def test_setnames_truncated_atoms(self, xserver, xkb_xclient): + """ + XkbSetNames with multiple 'which' bits set but truncated atom + data extends reads past the request buffer. + + Fixed in commit f7cd1276bbd4 ("Correct bounds checking in + XkbSetNames()"). + """ + xclient, opcode = xkb_xclient + + # Set several name categories but provide too little data + which = ( + xkb.XkbKeycodesNameMask + | xkb.XkbTypesNameMask + | xkb.XkbCompatNameMask + | xkb.XkbVirtualModNamesMask + | xkb.XkbRGNamesMask + ) + + req = xkb.SetNamesRequest( + opcode=opcode, + which=which, + virtual_mods=0xFFFF, # 16 virtual mod names expected + n_radio_groups=10, # 10 radio group names expected + payload=b"\x00" * 12, # Way too little for all those atoms + ) + xclient.send_request(req.to_bytes()) + time.sleep(0.5) + + assert xserver.is_alive, "Server crashed - truncated atoms in SetNames"