mirror of
https://gitlab.freedesktop.org/xorg/xserver.git
synced 2026-06-07 02:58:22 +02:00
pyxtest: add test cases for the various XKB CVEs from the last few years
Commit446ff2d317("Check SetMap request length carefully.") Commit6907b6ea2b("xkb: add request length validation for XkbSetGeometry") Commit4cd8533210("xkb: Fix buffer overflow in _XkbSetCompatMap()") Commitf7cd1276bb("Correct bounds checking in XkbSetNames()") Assisted-by: Claude:claude-claude-opus-4-6 Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2187>
This commit is contained in:
parent
7d89596e6c
commit
845eb56df2
3 changed files with 435 additions and 7 deletions
|
|
@ -57,6 +57,7 @@ if pytest.found()
|
|||
tests_pyxtest = [
|
||||
'test_randr.py',
|
||||
'test_xi.py',
|
||||
'test_xkb.py',
|
||||
]
|
||||
|
||||
test_list_data = configuration_data()
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
234
test/pyxtest/test_xkb.py
Normal file
234
test/pyxtest/test_xkb.py
Normal file
|
|
@ -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"
|
||||
Loading…
Add table
Reference in a new issue