diff --git a/test/pyxtest/meson.build b/test/pyxtest/meson.build index b70bd5576..a66828647 100644 --- a/test/pyxtest/meson.build +++ b/test/pyxtest/meson.build @@ -55,6 +55,7 @@ if pytest.found() # This needs to be kept in sync with the test_foo.py files in the tree tests_pyxtest = [ + 'test_glx.py', 'test_present.py', 'test_randr.py', 'test_record.py', diff --git a/test/pyxtest/proto/glx.py b/test/pyxtest/proto/glx.py new file mode 100644 index 000000000..8b59d29d4 --- /dev/null +++ b/test/pyxtest/proto/glx.py @@ -0,0 +1,106 @@ +# SPDX-License-Identifier: MIT +# +# GLX extension protocol request builders for security testing. + +import struct +from dataclasses import dataclass + +# GLX minor opcodes +GLXCreateContext = 3 +GLXMakeCurrent = 5 +GLXChangeDrawableAttributes = 30 + +# GLX drawable attribute keys +GLX_EVENT_MASK = 0x801F + + +@dataclass +class CreateContextRequest: + """glxCreateContext request (24 bytes = 6 words).""" + + opcode: int + context_id: int + visual: int + screen: int = 0 + share_list: int = 0 + is_direct: int = 1 + + def to_bytes(self, byte_order: str = "<") -> bytes: + return struct.pack( + f"{byte_order}BBH IIII B xxx", + self.opcode, + GLXCreateContext, + 6, # length = 6 words + self.context_id, + self.visual, + self.screen, + self.share_list, + self.is_direct, + ) + + +@dataclass +class MakeCurrentRequest: + """glxMakeCurrent request (16 bytes = 4 words).""" + + opcode: int + drawable: int + context_id: int + old_context_tag: int = 0 + + def to_bytes(self, byte_order: str = "<") -> bytes: + return struct.pack( + f"{byte_order}BBH III", + self.opcode, + GLXMakeCurrent, + 4, # length = 4 words + self.drawable, + self.context_id, + self.old_context_tag, + ) + + +@dataclass +class ChangeDrawableAttributesRequest: + """glxChangeDrawableAttributes request. + + Header is 12 bytes (3 words): opcode, glxCode, length, drawable, numAttribs. + Followed by numAttribs * 2 CARD32 values (key/value pairs). + + The length_override and num_attribs_override fields allow crafting + intentionally malformed requests for security testing. + """ + + opcode: int + drawable: int + num_attribs: int = 0 + attribs: bytes = b"" + length_override: int | None = None + num_attribs_override: int | None = None + + def to_bytes(self, byte_order: str = "<") -> bytes: + total_bytes = 12 + len(self.attribs) + 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 + ) + + num_attribs = ( + self.num_attribs_override + if self.num_attribs_override is not None + else self.num_attribs + ) + + header = struct.pack( + f"{byte_order}BBH II", + self.opcode, + GLXChangeDrawableAttributes, + length, + self.drawable, + num_attribs, + ) + return header + self.attribs + b"\x00" * pad_len diff --git a/test/pyxtest/test_glx.py b/test/pyxtest/test_glx.py new file mode 100644 index 000000000..6adf3f465 --- /dev/null +++ b/test/pyxtest/test_glx.py @@ -0,0 +1,168 @@ +# SPDX-License-Identifier: MIT +# +# Security tests for GLX extension vulnerabilities. + +import time + +import pytest + +from proto import glx +from xclient import Extension, X11Error + + +@pytest.fixture +def glx_xclient(xclient): + """Provide an xclient with GLX extension available.""" + ext = xclient.query_extension(Extension.GLX) + if not ext: + pytest.skip("GLX extension not available") + return xclient, ext.opcode + + +class TestGLXChangeDrawableAttributes: + """Tests for GLX ChangeDrawableAttributes length validation.""" + + @pytest.mark.asan + def test_reversed_length_check_oob_read(self, xserver, glx_xclient): + """ + ZDI-CAN-30165: __glXDisp_ChangeDrawableAttributes used a + reversed comparison operator (<) instead of (>) when validating + the request length against numAttribs. The check tested whether + the computed request size was LESS THAN client->req_len, but + should have tested GREATER THAN. With the reversed operator, a + request claiming many more attribute pairs than actually present + passes validation. + + DoChangeDrawableAttributes then iterates numAttribs attribute + pairs starting past the request header, reading beyond the + actual request data (OOB read). If a GLX_EVENT_MASK key is + found in the overread data, its value is written to + pGlxDraw->eventMask (OOB write). + + Attack: Create a GLX context bound to the root window (which + auto-creates a GLXDrawable), then send ChangeDrawableAttributes + with length=3 (12 bytes) but numAttribs=2100. The computed + size 4203 > 3 would fail with the correct check, but 4203 < 3 + is false, so the reversed check passes. + + Fixed by changing < to > in both glxcmds.c and glxcmdsswap.c. + """ + xclient, opcode = glx_xclient + + # Step 1: Create a direct context on the root visual. + ctx_id = xclient.alloc_id() + req = glx.CreateContextRequest( + opcode=opcode, + context_id=ctx_id, + visual=xclient.root_visual, + screen=0, + share_list=0, + is_direct=1, + ) + xclient.send_request(req.to_bytes()) + resp = xclient.recv_response(timeout=2.0) + if isinstance(resp, X11Error): + pytest.skip("GLX CreateContext failed (no GLX support for root visual)") + + # Step 2: MakeCurrent to bind the context and auto-create + # a GLXDrawable for the root window. + req = glx.MakeCurrentRequest( + opcode=opcode, + drawable=xclient.root_window, + context_id=ctx_id, + old_context_tag=0, + ) + xclient.send_request(req.to_bytes()) + resp = xclient.recv_response(timeout=2.0) + if isinstance(resp, X11Error): + pytest.skip("GLX MakeCurrent failed") + + # Step 3: ChangeDrawableAttributes with reversed length check. + # length=3 (12 bytes) but numAttribs=2100. + # Computed size: (12 + 2100*8) / 4 = 4203 words. + # Reversed check: 4203 < 3 → FALSE → passes! + # Correct check: 4203 > 3 → TRUE → BadLength. + req = glx.ChangeDrawableAttributesRequest( + opcode=opcode, + drawable=xclient.root_window, + num_attribs=0, + attribs=b"", + length_override=3, + num_attribs_override=2100, + ) + xclient.send_request(req.to_bytes()) + time.sleep(0.5) + + assert xserver.is_alive, ( + "Server crashed - GLX ChangeDrawableAttributes reversed " + "length check (ZDI-CAN-30165)" + ) + + +class TestGLXChangeDrawableAttributesSwap: + """Tests for GLX ChangeDrawableAttributes swap handler length validation.""" + + @pytest.mark.swapped_client + @pytest.mark.asan + def test_swap_oversized_request_rejected(self, xserver, xclient_swapped): + """ + __glXDispSwap_ChangeDrawableAttributes used a manual '>' + comparison instead of REQUEST_FIXED_SIZE. The manual check + only rejected undersized requests, silently accepting oversized + ones. REQUEST_FIXED_SIZE rejects both. + + Send a request with correct numAttribs=0 but extra trailing + data (oversized). On a fixed server with REQUEST_FIXED_SIZE, + this is rejected with BadLength. On the old code with the + manual '>' check, the oversized request was accepted. + + Fixed by replacing the manual check with REQUEST_FIXED_SIZE. + """ + conn = xclient_swapped + + ext = conn.query_extension(Extension.GLX) + if not ext: + pytest.skip("GLX extension not available") + + # Step 1: Create a direct context on the root visual. + ctx_id = conn.alloc_id() + req = glx.CreateContextRequest( + opcode=ext.opcode, + context_id=ctx_id, + visual=conn.root_visual, + screen=0, + share_list=0, + is_direct=1, + ) + conn.send_request(req.to_bytes(">")) + resp = conn.recv_response(timeout=2.0) + if isinstance(resp, X11Error): + pytest.skip("GLX CreateContext failed") + + # Step 2: MakeCurrent to create a GLXDrawable for root window. + req = glx.MakeCurrentRequest( + opcode=ext.opcode, + drawable=conn.root_window, + context_id=ctx_id, + old_context_tag=0, + ) + conn.send_request(req.to_bytes(">")) + resp = conn.recv_response(timeout=2.0) + if isinstance(resp, X11Error): + pytest.skip("GLX MakeCurrent failed") + + # Step 3: ChangeDrawableAttributes with numAttribs=0 but extra + # 8 bytes of trailing data. The correct length for numAttribs=0 + # is 3 words (12 bytes). We send 5 words (20 bytes). + req = glx.ChangeDrawableAttributesRequest( + opcode=ext.opcode, + drawable=conn.root_window, + num_attribs=0, + attribs=b"\x00" * 8, # 8 extra bytes + ) + conn.send_request(req.to_bytes(">")) + time.sleep(0.5) + + assert xserver.is_alive, ( + "Server crashed - GLX ChangeDrawableAttributes swap handler" + )