aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKees Cook <kees@outflux.net>2013-06-09 11:13:42 -0700
committerUlrich Sibiller <uli42@gmx.de>2016-10-19 21:40:28 +0200
commitea8239650656846d810a93e7659e182e412f07b3 (patch)
treefe420308c8107eeccfe75162f92d568129185866
parent39c6e5aa859c633fcb48e299643bb0189f333a0d (diff)
downloadnx-libs-ea8239650656846d810a93e7659e182e412f07b3.tar.gz
nx-libs-ea8239650656846d810a93e7659e182e412f07b3.tar.bz2
nx-libs-ea8239650656846d810a93e7659e182e412f07b3.zip
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>
-rw-r--r--nx-X11/lib/X11/ModMap.c10
-rw-r--r--nx-X11/lib/X11/XlibInt.c8
2 files changed, 15 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;
diff --git a/nx-X11/lib/X11/XlibInt.c b/nx-X11/lib/X11/XlibInt.c
index 564d318fc..deab8b3ad 100644
--- a/nx-X11/lib/X11/XlibInt.c
+++ b/nx-X11/lib/X11/XlibInt.c
@@ -3980,6 +3980,14 @@ void *_XGetRequest(Display *dpy, CARD8 type, size_t len)
if (dpy->bufptr + len > dpy->bufmax)
_XFlush(dpy);
+ /* Request still too large, so do not allow it to overflow. */
+ if (dpy->bufptr + len > dpy->bufmax) {
+ fprintf(stderr,
+ "Xlib: request %d length %zd would exceed buffer size.\n",
+ type, len);
+ /* Changes failure condition from overflow to NULL dereference. */
+ return NULL;
+ }
if (len % 4)
fprintf(stderr,