mirror of
https://github.com/X11Libre/xserver.git
synced 2026-10-26 01:59:57 +00:00
xkb: NULL text pointer after free in _CheckSetDoodad error path
_CheckSetDoodad() allocates doodad->text.text via _GetCountedString()
and then attempts to allocate doodad->text.font. If the font string
allocation fails (e.g. because the font string length extends past
the request boundary), the error handler frees doodad->text.text but
does not NULL the pointer. The doodad was already added to the geometry
by XkbAddGeomDoodad() before the string allocations, so when
XkbFreeGeometry() runs cleanup, _XkbClearDoodad() frees the same
dangling pointer again, resulting in a double-free.
NULL the pointer after freeing it to prevent the double-free.
This vulnerability was discovered by:
Anonymous working with TrendAI Zero Day Initiative
ZDI-CAN-31221
CVE-2026-88812
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Assisted-by: Claude:claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2341>
(cherry picked from commit 0d1b1b0bad)
This commit is contained in:
parent
37f43dc86a
commit
a0f5e66caa
3 changed files with 124 additions and 1 deletions
|
|
@ -5034,7 +5034,8 @@ _CheckSetDoodad(char **wire_inout, xkbSetGeometryReq *req,
|
|||
return status;
|
||||
status = _GetCountedString(&wire, client, &doodad->text.font);
|
||||
if (status != Success) {
|
||||
free (doodad->text.text);
|
||||
free(doodad->text.text);
|
||||
doodad->text.text = NULL;
|
||||
return status;
|
||||
}
|
||||
break;
|
||||
|
|
|
|||
|
|
@ -57,6 +57,13 @@ XkbNumRequiredTypes = 4
|
|||
XkbMaxLegalKeyCode = 255
|
||||
XkbNoShape = 0xFF
|
||||
|
||||
# Doodad types
|
||||
XkbOutlineDoodad = 1
|
||||
XkbSolidDoodad = 2
|
||||
XkbTextDoodad = 3
|
||||
XkbIndicatorDoodad = 4
|
||||
XkbLogoDoodad = 5
|
||||
|
||||
# XkbAllMapComponentsMask
|
||||
XkbAllClientInfoMask = XkbKeyTypesMask | XkbKeySymsMask | XkbModifierMapMask
|
||||
XkbAllServerInfoMask = (
|
||||
|
|
@ -803,6 +810,61 @@ class OverlayWire:
|
|||
return header + rows
|
||||
|
||||
|
||||
@dataclass
|
||||
class TextDoodadWire:
|
||||
"""
|
||||
xkbDoodadWireDesc for XkbTextDoodad (20 bytes header) + text + font strings.
|
||||
|
||||
Wire layout (xkbAnyDoodadWireDesc common):
|
||||
name(4 Atom) + type(1) + priority(1) + top(2) + left(2) + angle(2)
|
||||
Text-specific part of the 20-byte wire desc:
|
||||
width(2) + height(2) + colorNdx(1) + pad1(1) + pad2(2)
|
||||
Followed by: text (counted string) + font (counted string)
|
||||
"""
|
||||
|
||||
name: int = 0 # Atom
|
||||
priority: int = 0
|
||||
top: int = 0
|
||||
left: int = 0
|
||||
angle: int = 0
|
||||
width: int = 50
|
||||
height: int = 20
|
||||
color_ndx: int = 0
|
||||
text: str = "text"
|
||||
font: str = "font"
|
||||
# If set, use this raw value as the font string length (for malformed requests)
|
||||
font_length_override: int | None = None
|
||||
|
||||
def to_bytes(self, byte_order: str = "<") -> bytes:
|
||||
header = struct.pack(
|
||||
f"{byte_order}I" # name (Atom)
|
||||
f"BB" # type, priority
|
||||
f"hhh" # top, left, angle
|
||||
f"HH" # width, height
|
||||
f"Bx" # colorNdx, pad1
|
||||
f"xx", # pad2
|
||||
self.name,
|
||||
XkbTextDoodad,
|
||||
self.priority,
|
||||
self.top,
|
||||
self.left,
|
||||
self.angle,
|
||||
self.width,
|
||||
self.height,
|
||||
self.color_ndx,
|
||||
)
|
||||
text_data = build_counted_string(self.text, byte_order)
|
||||
if self.font_length_override is not None:
|
||||
# Craft a malformed font string: just a 2-byte length with no data
|
||||
font_data = struct.pack(f"{byte_order}H", self.font_length_override)
|
||||
# Pad to 4-byte boundary
|
||||
pad = (4 - len(font_data) % 4) % 4
|
||||
font_data += b"\x00" * pad
|
||||
else:
|
||||
font_data = build_counted_string(self.font, byte_order)
|
||||
return header + text_data + font_data
|
||||
|
||||
|
||||
@dataclass
|
||||
class GetKbdByNameRequest:
|
||||
"""xkbGetKbdByNameReq (minor opcode 23)."""
|
||||
|
|
|
|||
|
|
@ -396,6 +396,66 @@ class TestXkbSetGeometry:
|
|||
"Server crashed - SetGeometry overlay rowUnder off-by-one"
|
||||
)
|
||||
|
||||
@pytest.mark.asan
|
||||
def test_text_doodad_double_free(self, xserver, xkb_xclient):
|
||||
"""
|
||||
ZDI-CAN-31221: _CheckSetDoodad() frees doodad->text.text on error
|
||||
when the subsequent font string parse fails, but does not NULL the
|
||||
pointer. The doodad has already been added to the geometry by
|
||||
XkbAddGeomDoodad, so XkbFreeGeometry() → _XkbClearDoodad() frees
|
||||
the same pointer again, causing a double-free.
|
||||
|
||||
The request provides a valid text string ("AAAA") followed by a
|
||||
font string with length=200 but no data, causing _GetCountedString
|
||||
to fail the bounds check and triggering the error path.
|
||||
"""
|
||||
xclient, opcode = xkb_xclient
|
||||
|
||||
n_colors = 2
|
||||
color_data = self._build_colors(n_colors)
|
||||
name_atom = xclient.intern_atom("TestGeomDoubleFree")
|
||||
shape_atom = xclient.intern_atom("TestShapeDF")
|
||||
doodad_atom = xclient.intern_atom("TestDoodadDF")
|
||||
|
||||
label_font = xkb.build_counted_string("")
|
||||
|
||||
# 1 shape with 0 outlines (minimal valid shape)
|
||||
shape_data = xkb.ShapeWire(
|
||||
name=shape_atom,
|
||||
n_outlines=0,
|
||||
primary_ndx=xkb.XkbNoShape,
|
||||
approx_ndx=xkb.XkbNoShape,
|
||||
).to_bytes()
|
||||
|
||||
# TextDoodad with valid text but malformed font (length=200, no data)
|
||||
doodad_data = xkb.TextDoodadWire(
|
||||
name=doodad_atom,
|
||||
color_ndx=0,
|
||||
text="AAAA",
|
||||
font_length_override=200, # extends past request → bounds check fails
|
||||
).to_bytes()
|
||||
|
||||
payload = label_font + color_data + shape_data + doodad_data
|
||||
|
||||
req = xkb.SetGeometryRequest(
|
||||
opcode=opcode,
|
||||
n_shapes=1,
|
||||
n_sections=0,
|
||||
n_colors=n_colors,
|
||||
n_doodads=1,
|
||||
base_color_ndx=0,
|
||||
label_color_ndx=1,
|
||||
name_atom=name_atom,
|
||||
payload=payload,
|
||||
)
|
||||
xclient.send_request(req)
|
||||
time.sleep(0.5)
|
||||
|
||||
assert xserver.is_alive, (
|
||||
"Server crashed - TextDoodad double-free: _CheckSetDoodad frees "
|
||||
"text.text on error without NULLing, then _XkbClearDoodad frees again"
|
||||
)
|
||||
|
||||
|
||||
class TestXkbSetCompatMap:
|
||||
"""Tests for XKB SetCompatMap vulnerabilities."""
|
||||
|
|
|
|||
Loading…
Reference in a new issue