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 <[email protected]>
---
 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
_______________________________________________
[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