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 402b329c3a ("glx: Work around
wrong request lengths sent by mesa"). This was fixed in mesa commit
4324d6fdfbba1 in 2011 (mesa 7.11).

Fixes: 402b329c3a ("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
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2228>
This commit is contained in:
Peter Hutterer 2026-04-20 11:18:48 +10:00
parent 867b59b33b
commit 6d459e4daf
2 changed files with 10 additions and 23 deletions

View file

@ -1140,8 +1140,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);
}
@ -1362,9 +1361,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);
}
@ -1520,14 +1517,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));
@ -1594,8 +1585,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);
}
@ -1957,8 +1947,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);
}

View file

@ -236,7 +236,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);
@ -328,7 +328,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);
@ -441,9 +441,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);
@ -515,7 +513,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);
@ -724,7 +722,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);