From 901da57abfde6bf2ae33859c2dd47b45d6dd5871 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 20 Apr 2026 11:18:48 +1000 Subject: [PATCH] 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 (cherry picked from commit 6d459e4daf715bea8abdafa8fb130be2f8a1d145) Part-of: --- glx/glxcmds.c | 21 +++++---------------- glx/glxcmdsswap.c | 12 +++++------- 2 files changed, 10 insertions(+), 23 deletions(-) diff --git a/glx/glxcmds.c b/glx/glxcmds.c index 1e46d0c72..8a13e34b5 100644 --- a/glx/glxcmds.c +++ b/glx/glxcmds.c @@ -1144,8 +1144,7 @@ __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); } @@ -1366,9 +1365,7 @@ __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); } @@ -1524,14 +1521,8 @@ __glXDisp_ChangeDrawableAttributes(__GLXclientState * cl, GLbyte * pc) 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)); @@ -1598,8 +1589,7 @@ __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); } @@ -1960,8 +1950,7 @@ __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); } diff --git a/glx/glxcmdsswap.c b/glx/glxcmdsswap.c index 7d6674470..96382672a 100644 --- a/glx/glxcmdsswap.c +++ b/glx/glxcmdsswap.c @@ -235,7 +235,7 @@ __glXDispSwap_GetFBConfigsSGIX(__GLXclientState * cl, GLbyte * pc) __GLX_DECLARE_SWAP_VARIABLES; - REQUEST_AT_LEAST_SIZE(xGLXGetFBConfigsSGIXReq); + REQUEST_SIZE_MATCH(xGLXGetFBConfigsSGIXReq); __GLX_SWAP_INT(&req->screen); return __glXDisp_GetFBConfigsSGIX(cl, pc); @@ -327,7 +327,7 @@ __glXDispSwap_DestroyPixmap(__GLXclientState * cl, GLbyte * pc) __GLX_DECLARE_SWAP_VARIABLES; - REQUEST_AT_LEAST_SIZE(xGLXDestroyGLXPixmapReq); + REQUEST_SIZE_MATCH(xGLXDestroyGLXPixmapReq); __GLX_SWAP_SHORT(&req->length); __GLX_SWAP_INT(&req->glxpixmap); @@ -440,9 +440,7 @@ __glXDispSwap_ChangeDrawableAttributes(__GLXclientState * cl, GLbyte * pc) 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); @@ -514,7 +512,7 @@ __glXDispSwap_DestroyWindow(__GLXclientState * cl, GLbyte * pc) __GLX_DECLARE_SWAP_VARIABLES; - REQUEST_AT_LEAST_SIZE(xGLXDestroyWindowReq); + REQUEST_SIZE_MATCH(xGLXDestroyWindowReq); __GLX_SWAP_INT(&req->glxwindow); @@ -723,7 +721,7 @@ __glXDispSwap_GetDrawableAttributes(__GLXclientState * cl, GLbyte * pc) __GLX_DECLARE_SWAP_VARIABLES; - REQUEST_AT_LEAST_SIZE(xGLXGetDrawableAttributesReq); + REQUEST_SIZE_MATCH(xGLXGetDrawableAttributesReq); __GLX_SWAP_SHORT(&req->length); __GLX_SWAP_INT(&req->drawable);