test/pyxtest: add test for GLX ChangeDrawableAttributes OOB read (ZDI-CAN-30165)

Add GLX extension protocol builders (proto/glx.py) and a regression test
that reproduces the reversed length check in ChangeDrawableAttributes.

The test creates a GLX context on the root visual, binds it with
MakeCurrent (which auto-creates a GLXDrawable), then sends a
ChangeDrawableAttributes request with length=3 (12 bytes) but
numAttribs=2100. Without the fix, the reversed comparison operator (<
instead of >) would let this undersized request pass validation, and
DoChangeDrawableAttributes would iterate 2100 attribute pairs, reading
far past the 12-byte request buffer.

Assisted-by: Claude:claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2228>

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',

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

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"
+        )