[PATCH libXrandr] Avoid out of boundary accesses on illegal responses

[Julien Cristau]

[resending with xorg-devel cc added which I forgot the first time around]

On Sun, Sep 25, 2016 at 22:50:44 +0200, Matthieu Herrb wrote:

> diff --git a/src/XrrCrtc.c b/src/XrrCrtc.c
> index 5ae35c5..6665092 100644
> --- a/src/XrrCrtc.c
> +++ b/src/XrrCrtc.c
[...]
> @@ -357,7 +378,7 @@ XRRGetCrtcTransform (Display	*dpy,
>      xRRGetCrtcTransformReq	*req;
>      int				major_version, minor_version;
>      XRRCrtcTransformAttributes	*attr;
> -    char			*extra = NULL, *e;
> +    char			*extra = NULL, *end = NULL, *e;
>      int				p;
>  
>      *attributes = NULL;
> @@ -395,9 +416,17 @@ XRRGetCrtcTransform (Display	*dpy,
>  	else
>  	{
>  	    int extraBytes = rep.length * 4 - CrtcTransformExtra;
> -	    extra = Xmalloc (extraBytes);
> +	    if (rep.length < INT_MAX / 4 &&
> +		rep.length * 4 >= CrtcTransformExtra) {
> +		extra = Xmalloc (extraBytes);
> +		end = extra + extraBytes;
> +	    } else
> +		extra = NULL;
>  	    if (!extra) {
> -		_XEatDataWords (dpy, rep.length - (CrtcTransformExtra >> 2));
> +		if (rep.length > (CrtcTransformExtra >> 2))
> +		    _XEatDataWords (dpy, rep.length - (CrtcTransformExtra >> 2));
> +		else
> +		    _XEatDataWords (dpy, rep.length);
>  		UnlockDisplay (dpy);
>  		SyncHandle ();
>  		return False;
> @@ -429,22 +458,38 @@ XRRGetCrtcTransform (Display	*dpy,
>  
>      e = extra;
>  
> +    if (e + rep.pendingNbytesFilter > end) {
> +	XFree (extra);
> +	return False;
> +    }
>      memcpy (attr->pendingFilter, e, rep.pendingNbytesFilter);
>      attr->pendingFilter[rep.pendingNbytesFilter] = '\0';
>      e += (rep.pendingNbytesFilter + 3) & ~3;
>      for (p = 0; p < rep.pendingNparamsFilter; p++) {
>  	INT32	f;
> +	if (e + 4 > end) {
> +	    XFree (extra);
> +	    return False;
> +	}
>  	memcpy (&f, e, 4);
>  	e += 4;
>  	attr->pendingParams[p] = (XFixed) f;
>      }
>      attr->pendingNparams = rep.pendingNparamsFilter;
>  
> +    if (e + rep.currentNbytesFilter > end) {
> +	XFree (extra);
> +	return False;
> +    }
>      memcpy (attr->currentFilter, e, rep.currentNbytesFilter);
>      attr->currentFilter[rep.currentNbytesFilter] = '\0';
>      e += (rep.currentNbytesFilter + 3) & ~3;
>      for (p = 0; p < rep.currentNparamsFilter; p++) {
>  	INT32	f;
> +	if (e + 4 > end) {
> +	    XFree (extra);
> +	    return False;
> +	}
>  	memcpy (&f, e, 4);
>  	e += 4;
>  	attr->currentParams[p] = (XFixed) f;

It looks like we're leaking 'attr' on these error paths?

Cheers,
Julien