[PATCH v2:libXxf86vm] Discard correct length for old-format replies in XF86VidModeGetGamma

[Matthieu Herrb]

On Tue, Jan 06, 2015 at 12:03:37AM -0800, Alan Coopersmith wrote:
> Regression introduced in libXxf86vm 1.1.3 / commit 284a88e21fc05a63466
> Unlikely to be hit in practice since it requires out-of-range privsize
> or malloc failure while talking to a server using the XFree86 3.x version
> of the protocol.
> 
> Found by Oracle Parfait 1.5.1:
> 
> Error: Uninitialised memory (CWE 456)
>    Possible access to uninitialised memory '&rep.length'
>         at line 279 of open-src/lib/libXxf86vm/unpacked-src/src/XF86VMode.c in function 'XF86VidModeGetModeLine'.
>           &rep.length allocated at line 218.
>           &rep.length uninitialised when majorVersion < 2 at line
>    233.

Not tested, but it looks right to me. So

Reviewed-by: Matthieu Herrb <matthieu at herrb.eu>
> 
> Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
> ---
>  src/XF86VMode.c |   14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/src/XF86VMode.c b/src/XF86VMode.c
> index c7169c7..d13da14 100644
> --- a/src/XF86VMode.c
> +++ b/src/XF86VMode.c
> @@ -204,10 +204,9 @@ XF86VidModeGetModeLine(Display* dpy, int screen, int* dotclock,
>  		       XF86VidModeModeLine* modeline)
>  {
>      XExtDisplayInfo *info = find_display (dpy);
> -    xXF86VidModeGetModeLineReply rep;
> -    xXF86OldVidModeGetModeLineReply oldrep;
>      xXF86VidModeGetModeLineReq *req;
>      int majorVersion, minorVersion;
> +    CARD32 remaining_len;
>      Bool result = True;
>  
>      XF86VidModeCheckExtension (dpy, info, False);
> @@ -220,12 +219,16 @@ XF86VidModeGetModeLine(Display* dpy, int screen, int* dotclock,
>      req->screen = screen;
>  
>      if (majorVersion < 2) {
> +	xXF86OldVidModeGetModeLineReply oldrep;
> +
>  	if (!_XReply(dpy, (xReply *)&oldrep,
>              (SIZEOF(xXF86OldVidModeGetModeLineReply) - SIZEOF(xReply)) >> 2, xFalse)) {
>  	    UnlockDisplay(dpy);
>  	    SyncHandle();
>  	    return False;
>  	}
> +	remaining_len = oldrep.length -
> +	    ((SIZEOF(xXF86OldVidModeGetModeLineReply) - SIZEOF(xReply)) >> 2);
>  	*dotclock = oldrep.dotclock;
>  	modeline->hdisplay   = oldrep.hdisplay;
>  	modeline->hsyncstart = oldrep.hsyncstart;
> @@ -239,12 +242,16 @@ XF86VidModeGetModeLine(Display* dpy, int screen, int* dotclock,
>  	modeline->flags      = oldrep.flags;
>  	modeline->privsize   = oldrep.privsize;
>      } else {
> +	xXF86VidModeGetModeLineReply rep;
> +
>  	if (!_XReply(dpy, (xReply *)&rep,
>              (SIZEOF(xXF86VidModeGetModeLineReply) - SIZEOF(xReply)) >> 2, xFalse)) {
>  	    UnlockDisplay(dpy);
>  	    SyncHandle();
>  	    return False;
>  	}
> +	remaining_len = rep.length -
> +	    ((SIZEOF(xXF86VidModeGetModeLineReply) - SIZEOF(xReply)) >> 2);
>  	*dotclock = rep.dotclock;
>  	modeline->hdisplay   = rep.hdisplay;
>  	modeline->hsyncstart = rep.hsyncstart;
> @@ -265,8 +272,7 @@ XF86VidModeGetModeLine(Display* dpy, int screen, int* dotclock,
>  	else
>  	    modeline->private = NULL;
>  	if (modeline->private == NULL) {
> -	    _XEatDataWords(dpy, rep.length -
> -		((SIZEOF(xXF86VidModeGetModeLineReply) - SIZEOF(xReply)) >> 2));
> +	    _XEatDataWords(dpy, remaining_len);
>  	    result = False;
>  	} else
>  	    _XRead(dpy, (char*)modeline->private, modeline->privsize * sizeof(INT32));
> -- 
> 1.7.9.2
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel

-- 
Matthieu Herrb