From c37ceda76bf9ec6f5166122e864663e10f106546 Mon Sep 17 00:00:00 2001
From: Alan Coopersmith <alan.coopersmith@oracle.com>
Date: Sun, 14 Apr 2013 10:50:50 -0700
Subject: [PATCH] Xephyr: integer overflow in ephyrHostGLXGetStringFromServer()

reply.length & reply.size are CARD32s and need to be bounds checked before
multiplying or adding to come up with the total size to allocate, to avoid
integer overflow leading to underallocation and writing data from the
network past the end of the allocated buffer.

Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
---
 hw/kdrive/ephyr/ephyrhostglx.c | 47 +++++++++++++++++++++-------------
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/hw/kdrive/ephyr/ephyrhostglx.c b/hw/kdrive/ephyr/ephyrhostglx.c
index 5c6c40f0b..90f450c60 100644
--- a/hw/kdrive/ephyr/ephyrhostglx.c
+++ b/hw/kdrive/ephyr/ephyrhostglx.c
@@ -137,7 +137,7 @@ ephyrHostGLXQueryVersion(int *a_major, int *a_minor)
 }
 
 /**
- * GLX protocol structure for the ficticious "GXLGenericGetString" request.
+ * GLX protocol structure for the ficticious "GLXGenericGetString" request.
  * 
  * This is a non-existant protocol packet.  It just so happens that all of
  * the real protocol packets used to request a string from the server have
@@ -169,7 +169,8 @@ ephyrHostGLXGetStringFromServer(int a_screen_number,
     int default_screen = DefaultScreen(dpy);
     xGLXGenericGetStringReq *req = NULL;
     xGLXSingleReply reply;
-    int length = 0, numbytes = 0, major_opcode = 0, get_string_op = 0;
+    unsigned long length = 0, numbytes = 0;
+    int major_opcode = 0, get_string_op = 0;
 
     EPHYR_RETURN_VAL_IF_FAIL(dpy && a_string, FALSE);
 
@@ -209,36 +210,46 @@ ephyrHostGLXGetStringFromServer(int a_screen_number,
 
     _XReply(dpy, (xReply *) &reply, 0, False);
 
-    length = reply.length * 4;
-    if (!length) {
-        numbytes = 0;
-    }
-    else {
+#if UINT32_MAX >= (ULONG_MAX / 4)
+    if (reply.length >= (ULONG_MAX / 4))
+        goto eat_out;
+#endif
+    if (reply.length > 0) {
+        length = (unsigned long) reply.length * 4;
         numbytes = reply.size;
+        if (numbytes > length) {
+            EPHYR_LOG_ERROR("string length %d longer than reply length %d\n",
+                            numbytes, length);
+            goto eat_out;
+        }
     }
     EPHYR_LOG("going to get a string of size:%d\n", numbytes);
 
-    *a_string = (char *) Xmalloc(numbytes + 1);
-    if (!a_string) {
+    if (numbytes < INT_MAX)
+        *a_string = Xcalloc(numbytes + 1, 1);
+    else
+        *a_string = NULL;
+    if (*a_string == NULL) {
         EPHYR_LOG_ERROR("allocation failed\n");
-        goto out;
+        goto eat_out;
     }
 
-    memset(*a_string, 0, numbytes + 1);
     if (_XRead(dpy, *a_string, numbytes)) {
-        UnlockDisplay(dpy);
-        SyncHandle();
         EPHYR_LOG_ERROR("read failed\n");
-        goto out;
+        length = 0; /* if read failed, no idea how much to eat */
     }
-    length -= numbytes;
+    else {
+        length -= numbytes;
+        EPHYR_LOG("strname:%#x, strvalue:'%s', strlen:%d\n",
+                  a_string_name, *a_string, numbytes);
+        is_ok = TRUE;
+    }
+
+ eat_out:
     _XEatData(dpy, length);
     UnlockDisplay(dpy);
     SyncHandle();
-    EPHYR_LOG("strname:%#x, strvalue:'%s', strlen:%d\n",
-              a_string_name, *a_string, numbytes);
 
-    is_ok = TRUE;
  out:
     EPHYR_LOG("leave\n");
     return is_ok;