Reviewed-by: Jeremy Huddleston <[email protected]>

On Oct 27, 2011, at 22:07, Aaron Plattner wrote:

> Commit 587c3a2d1961834558193e8e14e8e381a077a253 fixed DoGetImage to
> check windows against their backing drawables, rather than against the
> screen dimensions, to prevent reading outside the bounds of redirected
> windows' backing pixmaps (see bug #22804).  Unfortunately, while
> making that change I also removed the check that the rectangle is
> contained within the bounds of the source window, which is a violation
> of the specification:
> 
>    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 Match error results). Note that the
>    borders of the window can be included and read with this request.
> 
>    (emphasis mine)
> 
> Reinstate the window dimension check, to return BadMatch if the
> GetImage request falls outside the bounds of the window.
> 
> Fixes X Test Suite test XGetImage-15:
> 
>    400|0 15 1 11:05:41|IC Start
>    200|0 15 11:05:41|TP Start
>    520|0 15 00005146 1 1|VSW5TESTSUITE PURPOSE 15
>    520|0 15 00005146 1 2|Assertion XGetImage-15.(A)
>    520|0 15 00005146 1 3|When the drawable is a window and the window is 
> viewable
>    520|0 15 00005146 1 4|and it is not the case that given there were no 
> inferiors or
>    520|0 15 00005146 1 5|overlapping windows the specified rectangle of the 
> window
>    520|0 15 00005146 1 6|would be fully visible on the screen and wholly 
> contained
>    520|0 15 00005146 1 7|within the outside edges of the window, then a 
> BadMatch
>    520|0 15 00005146 1 8|error occurs.
>    520|0 15 00005146 1 9|METH: Create window which is not fully visible on 
> the screen.
>    520|0 15 00005146 1 10|METH: Call XMapWindow to make sure the window is 
> viewable.
>    520|0 15 00005146 1 11|METH: Call XGetImage with rectangle extending 
> beyond edge of screen.
>    520|0 15 00005146 1 12|METH: Verify XGetImage return value is null.
>    520|0 15 00005146 1 13|METH: Verify that BadMatch error occurred.
>    520|0 15 00005146 1 14|METH: Create window which is fully visible on the 
> screen.
>    520|0 15 00005146 1 15|METH: Call XMapWindow to make sure the window is 
> viewable.
>    520|0 15 00005146 1 16|METH: Call XGetImage with rectangle extending 
> beyond edge of window.
>    520|0 15 00005146 1 17|METH: Verify XGetImage return value is null.
>    520|0 15 00005146 1 18|METH: Verify that BadMatch error occurred.
>    520|0 15 00005146 1 19|REPORT: Got Success, Expecting BadMatch
>    520|0 15 00005146 1 20|REPORT: Null image not returned.
>    220|0 15 1 11:05:41|FAIL
> 
> Signed-off-by: Aaron Plattner <[email protected]>
> ---
> This change has been sitting on my queue for a while.  It should be pretty 
> easy
> to review.
> 
> dix/dispatch.c |    8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/dix/dispatch.c b/dix/dispatch.c
> index 43cb4d1..2b6cb82 100644
> --- a/dix/dispatch.c
> +++ b/dix/dispatch.c
> @@ -2029,6 +2029,14 @@ DoGetImage(ClientPtr client, int format, Drawable 
> drawable,
>       if (!pWin->viewable)
>           return BadMatch;
> 
> +     /* If the drawable is a window, the rectangle must be contained within
> +      * its bounds (including the border). */
> +     if (x < -wBorderWidth(pWin) ||
> +         x + width > wBorderWidth(pWin) + (int)pDraw->width ||
> +         y < -wBorderWidth(pWin) ||
> +         y + height > wBorderWidth(pWin) + (int)pDraw->height)
> +         return BadMatch;
> +
>       relx += pDraw->x;
>       rely += pDraw->y;
> 
> -- 
> 1.7.5.4
> 
> _______________________________________________
> [email protected]: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to