On 06/ 6/13 10:40 AM, Kees Cook wrote:
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) now check "req" and fail if something has gone wrong.
An alternative would be to convert them to use Data instead of GetReqExtra()
as the requests like PutImage do that truly send arbitrarily large data to
the server, but I'm not sure it's worth it in these cases.
diff --git a/src/ModMap.c b/src/ModMap.c
index 5c5b426..f66e559 100644
--- a/src/ModMap.c
+++ b/src/ModMap.c
@@ -80,6 +80,10 @@ XSetModifierMapping(
LockDisplay(dpy);
GetReqExtra(SetModifierMapping, mapSize, req);
+ if (!req) {
+ UnlockDisplay(dpy);
+ return 2;
Style nit, please return MappingFailed instead of the raw value. (I see
the comment above the function uses the raw values instead of the symbolic
names used in the man pages and server side code, which is unfortunate.)
+ }
req->numKeyPerModifier = modifier_map->max_keypermod;
Since the protocol defines that as a single byte, we should probably reject
at the top of the function any modifier_map->max_keypermod > 255, as otherwise
we send a packet to the X server with more data than expected, which it will
reject - and that limit would also keep the mapSize within the normal Xlib
buffer size. That doesn't need to be this patch though.
--
-Alan Coopersmith- [email protected]
Oracle Solaris Engineering - http://blogs.oracle.com/alanc
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
