From f96e21359f03121daf97d7a10f8e56195f1e8b4d Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 20 Apr 2026 11:18:48 +1000 Subject: [PATCH xserver 5/9] glx: fix reversed length check in ChangeDrawableAttributes The request length validation in __glXDisp_ChangeDrawableAttributes and __glXDispSwap_ChangeDrawableAttributes uses the wrong comparison direction. The check tests whether the computed request size is LESS THAN client->req_len, but should test whether it is GREATER THAN. With the reversed operator, an undersized request (where numAttribs claims more attribute pairs than the request actually contains) passes validation. DoChangeDrawableAttributes then iterates numAttribs attribute pairs starting from the end of the request header, reading past the actual request data into adjacent memory. This is an out-of-bounds read that can also cause an out-of-bounds write when a GLX_EVENT_MASK attribute key is found in the overread data and its corresponding value is written to pGlxDraw->eventMask. This patch effectively reverts commit 402b329c3aa8 ("glx: Work around wrong request lengths sent by mesa"). This was fixed in mesa commit 4324d6fdfbba1 in 2011 (mesa 7.11). Fixes: 402b329c3aa8 ("glx: Work around wrong request lengths sent by mesa") This vulnerability was discovered by: Anonymous working with TrendAI Zero Day Initiative ZDI-CAN-30165 Assisted-by: Claude:claude-opus-4-6 --- glx/glxcmds.c | 21 +++++---------------- glx/glxcmdsswap.c | 12 +++++------- 2 files changed, 10 insertions(+), 23 deletions(-) diff --git ./glx/glxcmds.c ../glx/glxcmds.c index 3971809d4e2c..c7d32ab3a416 100644 --- ./glx/glxcmds.c +++ ../glx/glxcmds.c @@ -1135,18 +1135,17 @@ __glXDisp_GetFBConfigs(__GLXclientState * cl, GLbyte * pc) } int __glXDisp_GetFBConfigsSGIX(__GLXclientState * cl, GLbyte * pc) { ClientPtr client = cl->client; xGLXGetFBConfigsSGIXReq *req = (xGLXGetFBConfigsSGIXReq *) pc; - /* work around mesa bug, don't use REQUEST_SIZE_MATCH */ - REQUEST_AT_LEAST_SIZE(xGLXGetFBConfigsSGIXReq); + REQUEST_SIZE_MATCH(xGLXGetFBConfigsSGIXReq); return DoGetFBConfigs(cl, req->screen); } GLboolean __glXDrawableInit(__GLXdrawable * drawable, __GLXscreen * screen, DrawablePtr pDraw, int type, XID drawId, __GLXconfig * config) { @@ -1357,19 +1356,17 @@ __glXDisp_DestroyGLXPixmap(__GLXclientState * cl, GLbyte * pc) } int __glXDisp_DestroyPixmap(__GLXclientState * cl, GLbyte * pc) { ClientPtr client = cl->client; xGLXDestroyPixmapReq *req = (xGLXDestroyPixmapReq *) pc; - /* should be REQUEST_SIZE_MATCH, but mesa's glXDestroyPixmap used to set - * length to 3 instead of 2 */ - REQUEST_AT_LEAST_SIZE(xGLXDestroyPixmapReq); + REQUEST_SIZE_MATCH(xGLXDestroyPixmapReq); return DoDestroyDrawable(cl, req->glxpixmap, GLX_DRAWABLE_PIXMAP); } static int DoCreatePbuffer(ClientPtr client, int screenNum, XID fbconfigId, int width, int height, XID glxDrawableId) { @@ -1515,24 +1512,18 @@ __glXDisp_ChangeDrawableAttributes(__GLXclientState * cl, GLbyte * pc) xGLXChangeDrawableAttributesReq *req = (xGLXChangeDrawableAttributesReq *) pc; REQUEST_AT_LEAST_SIZE(xGLXChangeDrawableAttributesReq); if (req->numAttribs > (UINT32_MAX >> 3)) { client->errorValue = req->numAttribs; return BadValue; } -#if 0 - /* mesa sends an additional 8 bytes */ + REQUEST_FIXED_SIZE(xGLXChangeDrawableAttributesReq, req->numAttribs << 3); -#else - if (((sizeof(xGLXChangeDrawableAttributesReq) + - (req->numAttribs << 3)) >> 2) < client->req_len) - return BadLength; -#endif return DoChangeDrawableAttributes(cl->client, req->drawable, req->numAttribs, (CARD32 *) (req + 1)); } int __glXDisp_ChangeDrawableAttributesSGIX(__GLXclientState * cl, GLbyte * pc) { @@ -1589,18 +1580,17 @@ __glXDisp_CreateWindow(__GLXclientState * cl, GLbyte * pc) } int __glXDisp_DestroyWindow(__GLXclientState * cl, GLbyte * pc) { ClientPtr client = cl->client; xGLXDestroyWindowReq *req = (xGLXDestroyWindowReq *) pc; - /* mesa's glXDestroyWindow used to set length to 3 instead of 2 */ - REQUEST_AT_LEAST_SIZE(xGLXDestroyWindowReq); + REQUEST_SIZE_MATCH(xGLXDestroyWindowReq); return DoDestroyDrawable(cl, req->glxwindow, GLX_DRAWABLE_WINDOW); } /*****************************************************************************/ /* ** NOTE: There is no portable implementation for swap buffers as of @@ -1952,18 +1942,17 @@ DoGetDrawableAttributes(__GLXclientState * cl, XID drawId) } int __glXDisp_GetDrawableAttributes(__GLXclientState * cl, GLbyte * pc) { ClientPtr client = cl->client; xGLXGetDrawableAttributesReq *req = (xGLXGetDrawableAttributesReq *) pc; - /* this should be REQUEST_SIZE_MATCH, but mesa sends an additional 4 bytes */ - REQUEST_AT_LEAST_SIZE(xGLXGetDrawableAttributesReq); + REQUEST_SIZE_MATCH(xGLXGetDrawableAttributesReq); return DoGetDrawableAttributes(cl, req->drawable); } int __glXDisp_GetDrawableAttributesSGIX(__GLXclientState * cl, GLbyte * pc) { ClientPtr client = cl->client; diff --git ./glx/glxcmdsswap.c ../glx/glxcmdsswap.c index dc38ff3ad87e..3f7880f41c3c 100644 --- ./glx/glxcmdsswap.c +++ ../glx/glxcmdsswap.c @@ -231,17 +231,17 @@ __glXDispSwap_GetFBConfigs(__GLXclientState * cl, GLbyte * pc) int __glXDispSwap_GetFBConfigsSGIX(__GLXclientState * cl, GLbyte * pc) { ClientPtr client = cl->client; xGLXGetFBConfigsSGIXReq *req = (xGLXGetFBConfigsSGIXReq *) pc; __GLX_DECLARE_SWAP_VARIABLES; - REQUEST_AT_LEAST_SIZE(xGLXGetFBConfigsSGIXReq); + REQUEST_SIZE_MATCH(xGLXGetFBConfigsSGIXReq); __GLX_SWAP_INT(&req->screen); return __glXDisp_GetFBConfigsSGIX(cl, pc); } int __glXDispSwap_CreateGLXPixmap(__GLXclientState * cl, GLbyte * pc) { @@ -323,17 +323,17 @@ __glXDispSwap_DestroyGLXPixmap(__GLXclientState * cl, GLbyte * pc) int __glXDispSwap_DestroyPixmap(__GLXclientState * cl, GLbyte * pc) { ClientPtr client = cl->client; xGLXDestroyGLXPixmapReq *req = (xGLXDestroyGLXPixmapReq *) pc; __GLX_DECLARE_SWAP_VARIABLES; - REQUEST_AT_LEAST_SIZE(xGLXDestroyGLXPixmapReq); + REQUEST_SIZE_MATCH(xGLXDestroyGLXPixmapReq); __GLX_SWAP_SHORT(&req->length); __GLX_SWAP_INT(&req->glxpixmap); return __glXDisp_DestroyGLXPixmap(cl, pc); } int @@ -436,19 +436,17 @@ __glXDispSwap_ChangeDrawableAttributes(__GLXclientState * cl, GLbyte * pc) __GLX_SWAP_INT(&req->drawable); __GLX_SWAP_INT(&req->numAttribs); if (req->numAttribs > (UINT32_MAX >> 3)) { client->errorValue = req->numAttribs; return BadValue; } - if (((sizeof(xGLXChangeDrawableAttributesReq) + - (req->numAttribs << 3)) >> 2) < client->req_len) - return BadLength; + REQUEST_FIXED_SIZE(xGLXChangeDrawableAttributesReq, req->numAttribs << 3); attribs = (CARD32 *) (req + 1); __GLX_SWAP_INT_ARRAY(attribs, req->numAttribs << 1); return __glXDisp_ChangeDrawableAttributes(cl, pc); } int @@ -510,17 +508,17 @@ __glXDispSwap_CreateWindow(__GLXclientState * cl, GLbyte * pc) int __glXDispSwap_DestroyWindow(__GLXclientState * cl, GLbyte * pc) { ClientPtr client = cl->client; xGLXDestroyWindowReq *req = (xGLXDestroyWindowReq *) pc; __GLX_DECLARE_SWAP_VARIABLES; - REQUEST_AT_LEAST_SIZE(xGLXDestroyWindowReq); + REQUEST_SIZE_MATCH(xGLXDestroyWindowReq); __GLX_SWAP_INT(&req->glxwindow); return __glXDisp_DestroyWindow(cl, pc); } int __glXDispSwap_SwapBuffers(__GLXclientState * cl, GLbyte * pc) @@ -719,17 +717,17 @@ __glXDispSwap_GetDrawableAttributesSGIX(__GLXclientState * cl, GLbyte * pc) int __glXDispSwap_GetDrawableAttributes(__GLXclientState * cl, GLbyte * pc) { ClientPtr client = cl->client; xGLXGetDrawableAttributesReq *req = (xGLXGetDrawableAttributesReq *) pc; __GLX_DECLARE_SWAP_VARIABLES; - REQUEST_AT_LEAST_SIZE(xGLXGetDrawableAttributesReq); + REQUEST_SIZE_MATCH(xGLXGetDrawableAttributesReq); __GLX_SWAP_SHORT(&req->length); __GLX_SWAP_INT(&req->drawable); return __glXDisp_GetDrawableAttributes(cl, pc); } /************************************************************************/ -- 2.53.0