followup for libX11 XKB security fixes
Julien Cristau
jcristau at debian.org
Thu May 23 10:26:50 PDT 2013
Hi,
I noticed some inconsistencies in the XKB security changes in Xlib.
Resending to the public list now that the embargo is lifted.
Here's another change I think is necessary:
diff --git a/src/xkb/XKBGetMap.c b/src/xkb/XKBGetMap.c
index 0875dfd..c73e655 100644
--- a/src/xkb/XKBGetMap.c
+++ b/src/xkb/XKBGetMap.c
@@ -426,7 +426,7 @@ XkbServerMapPtr srv;
if ( rep->totalVModMapKeys>0 ) {
if (((int) rep->firstVModMapKey + rep->nVModMapKeys)
- > xkb->max_key_code)
+ > xkb->max_key_code + 1)
return BadLength;
if (((xkb->server==NULL)||(xkb->server->vmodmap==NULL))&&
(XkbAllocServerMap(xkb,XkbVirtualModMapMask,0)!=Success)) {
diff --git a/src/xkb/XKBNames.c b/src/xkb/XKBNames.c
index 6faef02..6df7406 100644
--- a/src/xkb/XKBNames.c
+++ b/src/xkb/XKBNames.c
@@ -180,7 +180,7 @@ _XkbReadGetNamesReply( Display * dpy,
nKeys= xkb->max_key_code+1;
names->keys= _XkbTypedCalloc(nKeys,XkbKeyNameRec);
}
- else if ( ((int)rep->firstKey + rep->nKeys) > xkb->max_key_code)
+ if ( ((int)rep->firstKey + rep->nKeys) > xkb->max_key_code + 1)
goto BAILOUT;
if (names->keys!=NULL) {
if (!_XkbCopyFromReadBuffer(&buf,
This makes the checks against max_key_code consistent, and means the
GetNames reply gets checked even when names->keys was just allocated.
Running 'xset q' here under xtrace shows GetNames returning firstKey=8
nKeys=248, which would trip the original check with maxKeyCode=255.
GetMap seems to return firstVModMapKey=0 nVModMapKeys=0
totalVModMapKeys=0 though, so I don't know how to trigger the first
check.
Cheers,
Julien
More information about the xorg-devel
mailing list