[PATCH] Bug #22804: Reject out of bounds XGetImage requests
Aaron Plattner
aplattner at nvidia.com
Wed Jul 22 11:01:24 PDT 2009
Okay, how does this look?
===
The XGetImage man page states:
If the drawable is a window, the window must be viewable, and it must be the
case that if there were no inferiors or overlapping windows, the specified
rectangle of the window would be fully visible on the screen and wholly
contained within the outside edges of the window, or a BadMatch error
results. Note that the borders of the window can be included and read with
this request.
However, the server was only checking the requested region against the screen
bounds, allowing XGetImage requests to read pixels outside the bounds of a
window's ancestors. Normally, this would just read other pixels from the
screen, but if one of the ancestor windows is redirected, the window's backing
pixmap may be smaller than the window itself.
This change checks the region against the window's bounding drawable, which is
either the screen pixmap, a redirected window's backing pixmap, or the root
window for servers that don't support GetWindowPixmap.
---
dix/dispatch.c | 70 +++++++++++++++++++++++++++++++++++++------------------
1 files changed, 47 insertions(+), 23 deletions(-)
diff --git a/dix/dispatch.c b/dix/dispatch.c
index 4df775d..b7c3800 100644
--- a/dix/dispatch.c
+++ b/dix/dispatch.c
@@ -2062,9 +2062,11 @@ DoGetImage(ClientPtr client, int format, Drawable drawable,
int x, int y, int width, int height,
Mask planemask, xGetImageReply **im_return)
{
- DrawablePtr pDraw;
+ DrawablePtr pDraw, pBoundingDraw;
int nlines, linesPerBuf, rc;
- int linesDone;
+ int linesDone;
+ /* coordinates relative to the bounding drawable */
+ int relx, rely;
long widthBytesLine, length;
Mask plane = 0;
char *pBuf;
@@ -2081,35 +2083,57 @@ DoGetImage(ClientPtr client, int format, Drawable drawable,
return rc;
memset(&xgi, 0, sizeof(xGetImageReply));
+
+ relx = x;
+ rely = y;
+
if(pDraw->type == DRAWABLE_WINDOW)
{
- if( /* check for being viewable */
- !((WindowPtr) pDraw)->realized ||
- /* check for being on screen */
- pDraw->x + x < 0 ||
- pDraw->x + x + width > pDraw->pScreen->width ||
- pDraw->y + y < 0 ||
- pDraw->y + y + height > pDraw->pScreen->height ||
- /* check for being inside of border */
- x < - wBorderWidth((WindowPtr)pDraw) ||
- x + width > wBorderWidth((WindowPtr)pDraw) + (int)pDraw->width ||
- y < -wBorderWidth((WindowPtr)pDraw) ||
- y + height > wBorderWidth ((WindowPtr)pDraw) + (int)pDraw->height
- )
- return(BadMatch);
- xgi.visual = wVisual (((WindowPtr) pDraw));
+ WindowPtr pWin = (WindowPtr)pDraw;
+
+ /* "If the drawable is a window, the window must be viewable ... or a
+ * BadMatch error results" */
+ if (!pWin->viewable)
+ return BadMatch;
+
+ relx += pDraw->x;
+ rely += pDraw->y;
+
+ if (pDraw->pScreen->GetWindowPixmap) {
+ PixmapPtr pPix = (*pDraw->pScreen->GetWindowPixmap) (pWin);
+
+ pBoundingDraw = &pPix->drawable;
+#ifdef COMPOSITE
+ relx -= pPix->screen_x;
+ rely -= pPix->screen_y;
+#endif
+ } else {
+ pBoundingDraw = (DrawablePtr)WindowTable[pDraw->pScreen->myNum];
+ }
+
+ xgi.visual = wVisual (pWin);
}
else
{
- if(x < 0 ||
- x+width > (int)pDraw->width ||
- y < 0 ||
- y+height > (int)pDraw->height
- )
- return(BadMatch);
+ pBoundingDraw = pDraw;
xgi.visual = None;
}
+ /* "If the drawable is a pixmap, the given rectangle must be wholly
+ * contained within the pixmap, or a BadMatch error results. If the
+ * drawable is a window [...] it must be the case that if there were no
+ * inferiors or overlapping windows, the specified rectangle of the window
+ * would be fully visible on the screen and wholly contained within the
+ * outside edges of the window, or a BadMatch error results."
+ *
+ * We relax the window case slightly to mean that the rectangle must exist
+ * within the bounds of the window's backing pixmap. In particular, this
+ * means that a GetImage request may succeed or fail with BadMatch depending
+ * on whether any of its ancestor windows are redirected. */
+ if(relx < 0 || relx + width > (int)pBoundingDraw->width ||
+ rely < 0 || rely + height > (int)pBoundingDraw->height)
+ return BadMatch;
+
xgi.type = X_Reply;
xgi.sequenceNumber = client->sequence;
xgi.depth = pDraw->depth;
--
1.6.0.4
More information about the xorg-devel
mailing list