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 <[email protected]>
> 
> Signed-off-by: Alan Coopersmith <[email protected]>
> ---
>  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
> 
> _______________________________________________
> [email protected]: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel

-- 
Matthieu Herrb
_______________________________________________
[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