From e9415ddef2ac81d4139bd32d5e9cda9394a60051 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 13 Apr 2013 01:20:08 -0700 Subject: [PATCH 5/6] Multiple unvalidated assumptions in XvMCGetDRInfo() [CVE-2013-1999] The individual string sizes is assumed to not be more than the amount of data read from the network, and could cause buffer overflow if they are. The strings returned from the X server are assumed to be null terminated, and could cause callers to read past the end of the buffer if they are not. Also be sure to set the returned pointers to NULL, so callers don't try accessing bad pointers on failure cases. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith --- src/XvMC.c | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/XvMC.c b/src/XvMC.c index d8bc59d..cb42487 100644 --- a/src/XvMC.c +++ b/src/XvMC.c @@ -499,7 +499,6 @@ Status XvMCGetDRInfo(Display *dpy, XvPortID port, XExtDisplayInfo *info = xvmc_find_display(dpy); xvmcGetDRInfoReply rep; xvmcGetDRInfoReq *req; - char *tmpBuf = NULL; CARD32 magic; #ifdef HAVE_SHMAT @@ -510,6 +509,9 @@ Status XvMCGetDRInfo(Display *dpy, XvPortID port, here.tz_dsttime = 0; #endif + *name = NULL; + *busID = NULL; + XvMCCheckExtension (dpy, info, BadImplementation); LockDisplay (dpy); @@ -568,31 +570,31 @@ Status XvMCGetDRInfo(Display *dpy, XvPortID port, #endif if (rep.length > 0) { - - int realSize = rep.length << 2; - - tmpBuf = (char *) Xmalloc(realSize); - if (tmpBuf) { - *name = (char *) Xmalloc(rep.nameLen); - if (*name) { - *busID = (char *) Xmalloc(rep.busIDLen); - if (! *busID) { - XFree(*name); - XFree(tmpBuf); - } - } else { - XFree(tmpBuf); + unsigned long realSize = 0; + char *tmpBuf = NULL; + + if (rep.length < (INT_MAX >> 2)) { + realSize = rep.length << 2; + if (realSize >= (rep.nameLen + rep.busIDLen)) { + tmpBuf = Xmalloc(realSize); + *name = Xmalloc(rep.nameLen); + *busID = Xmalloc(rep.busIDLen); } } if (*name && *busID && tmpBuf) { - _XRead(dpy, tmpBuf, realSize); strncpy(*name,tmpBuf,rep.nameLen); + name[rep.nameLen - 1] = '\0'; strncpy(*busID,tmpBuf+rep.nameLen,rep.busIDLen); + busID[rep.busIDLen - 1] = '\0'; XFree(tmpBuf); - } else { + XFree(*name); + *name = NULL; + XFree(*busID); + *name = NULL; + XFree(tmpBuf); _XEatDataWords(dpy, rep.length); UnlockDisplay (dpy); -- 1.8.2.3