From e9415ddef2ac81d4139bd32d5e9cda9394a60051 Mon Sep 17 00:00:00 2001
From: Alan Coopersmith <alan.coopersmith@oracle.com>
Date: Sat, 13 Apr 2013 01:20:08 -0700
Subject: 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 <ivansprundel@ioactive.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
---
 src/XvMC.c | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

(limited to 'src')

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);
-- 
cgit v1.2.3