[PATCH] libX11: check size of GetReqExtra after XFlush

[Kees Cook]

Two users of GetReqExtra pass arbitrarily sized allocations from the
caller (ModMap and Host). Adjust the GetReqExtra macro to double-check
the requested length and invalidate "req" when this happens. Users of
potentially >16K lengths in GetReqExtra now check "req" and fail if
something has gone wrong.

https://bugs.launchpad.net/ubuntu/+source/x11-xserver-utils/+bug/792628

Signed-off-by: Kees Cook <kees.cook at canonical.com>
---
 include/X11/Xlibint.h |   28 ++++++++++++++++++----------
 src/Host.c            |    8 ++++++++
 src/ModMap.c          |    4 ++++
 3 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/include/X11/Xlibint.h b/include/X11/Xlibint.h
index 2ce356d..81f8cfd 100644
--- a/include/X11/Xlibint.h
+++ b/include/X11/Xlibint.h
@@ -461,21 +461,29 @@ extern LockInfoPtr _Xglobal_lock;
         WORD64ALIGN\
 	if ((dpy->bufptr + SIZEOF(x##name##Req) + n) > dpy->bufmax)\
 		_XFlush(dpy);\
-	req = (x##name##Req *)(dpy->last_req = dpy->bufptr);\
-	req->reqType = X_##name;\
-	req->length = (SIZEOF(x##name##Req) + n)>>2;\
-	dpy->bufptr += SIZEOF(x##name##Req) + n;\
-	dpy->request++
+	if ((dpy->bufptr + SIZEOF(x##name##Req) + n) <= dpy->bufmax) {\
+	    req = (x##name##Req *)(dpy->last_req = dpy->bufptr);\
+    	req->reqType = X_##name;\
+    	req->length = (SIZEOF(x##name##Req) + n)>>2;\
+    	dpy->bufptr += SIZEOF(x##name##Req) + n;\
+    	dpy->request++;\
+    } else {\
+        req = NULL;\
+    }
 #else
 #define GetReqExtra(name, n, req) \
         WORD64ALIGN\
 	if ((dpy->bufptr + SIZEOF(x/**/name/**/Req) + n) > dpy->bufmax)\
 		_XFlush(dpy);\
-	req = (x/**/name/**/Req *)(dpy->last_req = dpy->bufptr);\
-	req->reqType = X_/**/name;\
-	req->length = (SIZEOF(x/**/name/**/Req) + n)>>2;\
-	dpy->bufptr += SIZEOF(x/**/name/**/Req) + n;\
-	dpy->request++
+	if ((dpy->bufptr + SIZEOF(x/**/name/**/Req) + n) <= dpy->bufmax) {\
+    	req = (x/**/name/**/Req *)(dpy->last_req = dpy->bufptr);\
+    	req->reqType = X_/**/name;\
+    	req->length = (SIZEOF(x/**/name/**/Req) + n)>>2;\
+    	dpy->bufptr += SIZEOF(x/**/name/**/Req) + n;\
+    	dpy->request++;\
+    } else {\
+        req = NULL;\
+    }
 #endif
 
 
diff --git a/src/Host.c b/src/Host.c
index da9923a..3f87731 100644
--- a/src/Host.c
+++ b/src/Host.c
@@ -83,6 +83,10 @@ XAddHost (
 
     LockDisplay(dpy);
     GetReqExtra (ChangeHosts, length, req);
+    if (!req) {
+        UnlockDisplay(dpy);
+        return 0;
+    }
     req->mode = HostInsert;
     req->hostFamily = host->family;
     req->hostLength = addrlen;
@@ -118,6 +122,10 @@ XRemoveHost (
 
     LockDisplay(dpy);
     GetReqExtra (ChangeHosts, length, req);
+    if (!req) {
+        UnlockDisplay(dpy);
+        return 0;
+    }
     req->mode = HostDelete;
     req->hostFamily = host->family;
     req->hostLength = addrlen;
diff --git a/src/ModMap.c b/src/ModMap.c
index c99bfdd..5deb894 100644
--- a/src/ModMap.c
+++ b/src/ModMap.c
@@ -75,6 +75,10 @@ XSetModifierMapping(
 
     LockDisplay(dpy);
     GetReqExtra(SetModifierMapping, mapSize, req);
+    if (!req) {
+        UnlockDisplay(dpy);
+        return 2;
+    }
 
     req->numKeyPerModifier = modifier_map->max_keypermod;
 
-- 
1.7.4.1


-- 
Kees Cook
Ubuntu Security Team