libX11: check size of GetReqExtra after XFlush

Two users of GetReqExtra pass arbitrarily sized allocations from the
caller (ModMap and Host). Adjust _XGetRequest() (called by the GetReqExtra
macro) to double-check the requested length and invalidate "req" when
this happens. Users of GetReqExtra passing lengths greater than the Xlib
buffer size (normally 16K) must check "req" and fail gracefully instead
of crashing.

Any callers of GetReqExtra that do not check "req" for NULL
will experience this change, in the pathological case, as a NULL
dereference instead of a buffer overflow. This is an improvement, but
the documentation for GetReqExtra has been updated to reflect the need
to check the value of "req" after the call.

Bug that manifested the problem:
https://bugs.launchpad.net/ubuntu/+source/x11-xserver-utils/+bug/792628

Signed-off-by: Kees Cook <kees@outflux.net>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Backported-to-NX-by: Ulrich Sibiller <uli42@gmx.de>

1 files changed, 7 insertions, 3 deletions

diff --git a/nx-X11/lib/X11/ModMap.c b/nx-X11/lib/X11/ModMap.c
index 04cd676eb..3c6971c77 100644
--- a/nx-X11/lib/X11/ModMap.c
+++ b/nx-X11/lib/X11/ModMap.c
@@ -65,9 +65,9 @@ XGetModifierMapping(register Display *dpy)
 
 /*
  *	Returns:
- *	0	Success
- *	1	Busy - one or more old or new modifiers are down
- *	2	Failed - one or more new modifiers unacceptable
+ *	MappingSuccess (0)	Success
+ *	MappingBusy (1) 	Busy - one or more old or new modifiers are down
+ *	MappingFailed (2)	Failed - one or more new modifiers unacceptable
  */
 int
 XSetModifierMapping(
@@ -80,6 +80,10 @@ XSetModifierMapping(
 
     LockDisplay(dpy);
     GetReqExtra(SetModifierMapping, mapSize, req);
+    if (!req) {
+       UnlockDisplay(dpy);
+       return MappingFailed;
+    }
 
     req->numKeyPerModifier = modifier_map->max_keypermod;