Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[xsrc/trunk]: xsrc/external/mit/xorg-server.old/dist/xkb merge security fixes...



details:   https://anonhg.NetBSD.org/xsrc/rev/2d0e10fa742a
branches:  trunk
changeset: 10635:2d0e10fa742a
user:      mrg <mrg%NetBSD.org@localhost>
date:      Sat Dec 05 23:36:45 2020 +0000

description:
merge security fixes for xkb, as found in these xserver gitlab
commits:

270e439739e023463e7e0719a4eede69d45f7a3f - xkb: only swap once in XkbSetMap
446ff2d3177087b8173fa779fa5b77a2a128988b - Check SetMap request length carefully
87c64fc5b0db9f62f4e361444f4b60501ebf67b9 - Fix XkbSetDeviceInfo() and SetDeviceIndicators() heap overflows
de940e06f8733d87bbb857aef85d830053442cfe - xkb: fix key type index check in _XkbSetMapChecks
f7cd1276bbd4fe3a9700096dec33b52b8440788d - Correct bounds checking in XkbSetNames()

i haven't tested these run OK, and it was a 33 out of 34 hunks
did not apply cleanly, but they merge was still largely the
same (patch failed due to whitespace changes mostly), and i am
able to build-test successfully.

diffstat:

 external/mit/xorg-server.old/dist/xkb/xkb.c |  198 +++++++++++++++++++++++++--
 1 files changed, 180 insertions(+), 18 deletions(-)

diffs (truncated from 450 to 300 lines):

diff -r 06e940bbe615 -r 2d0e10fa742a external/mit/xorg-server.old/dist/xkb/xkb.c
--- a/external/mit/xorg-server.old/dist/xkb/xkb.c       Sat Dec 05 22:44:37 2020 +0000
+++ b/external/mit/xorg-server.old/dist/xkb/xkb.c       Sat Dec 05 23:36:45 2020 +0000
@@ -151,6 +151,19 @@
 #define        CHK_REQ_KEY_RANGE(err,first,num,r)  \
        CHK_REQ_KEY_RANGE2(err,first,num,r,client->errorValue,BadValue)
 
+static Bool
+_XkbCheckRequestBounds(ClientPtr client, void *stuff, void *from, void *to) {
+    char *cstuff = (char *)stuff;
+    char *cfrom = (char *)from;
+    char *cto = (char *)to;
+
+    return cfrom < cto &&
+           cfrom >= cstuff &&
+           cfrom < cstuff + ((size_t)client->req_len << 2) &&
+           cto >= cstuff &&
+           cto <= cstuff + ((size_t)client->req_len << 2);
+}
+
 /***====================================================================***/
 
 int
@@ -1550,7 +1563,8 @@
                xkbSetMapReq *  req,
                xkbKeyTypeWireDesc **wireRtrn,
                int      *      nMapsRtrn,
-               CARD8 *         mapWidthRtrn)
+               CARD8 *         mapWidthRtrn,
+               Bool doswap)
 {
 unsigned               nMaps;
 register unsigned      i,n;
@@ -1588,7 +1602,7 @@
     }
     for (i=0;i<req->nTypes;i++) {
        unsigned        width;
-       if (client->swapped) {
+        if (client->swapped && doswap) {
            register int s;
            swaps(&wire->virtualMods,s);
        }
@@ -1615,7 +1629,7 @@
            mapWire= (xkbKTSetMapEntryWireDesc *)&wire[1];
            preWire= (xkbModsWireDesc *)&mapWire[wire->nMapEntries];
            for (n=0;n<wire->nMapEntries;n++) {
-               if (client->swapped) {
+                if (client->swapped && doswap) {
                    register int s;
                    swaps(&mapWire[n].virtualMods,s);
                }
@@ -1634,7 +1648,7 @@
                    return 0;
                }
                if (wire->preserve) {
-                   if (client->swapped) {
+                   if (client->swapped && doswap) {
                        register int s;
                        swaps(&preWire[n].virtualMods,s);
                    }
@@ -1673,7 +1687,8 @@
                CARD8 *                 mapWidths,
                CARD16 *                symsPerKey,
                xkbSymMapWireDesc **    wireRtrn,
-               int *                   errorRtrn)
+               int *                   errorRtrn,
+               Bool                    doswap)
 {
 register unsigned      i;
 XkbSymMapPtr           map;
@@ -1685,7 +1700,7 @@
     for (i=0;i<req->nKeySyms;i++) {
        KeySym *pSyms;
        register unsigned nG;
-       if (client->swapped) {
+       if (client->swapped && doswap) {
            swaps(&wire->nSyms,nG);
        }
        nG = XkbNumGroups(wire->groupInfo);
@@ -2322,13 +2337,99 @@
     }
     return (char *)wire;
 }
+ 
+#define _add_check_len(new) \
+    if (len > UINT32_MAX - (new) || len > req_len - (new)) goto bad; \
+    else len += new
+
+/**
+ * Check the length of the SetMap request
+ */
+static int
+_XkbSetMapCheckLength(xkbSetMapReq *req)
+{
+    size_t len = sz_xkbSetMapReq, req_len = req->length << 2;
+    xkbKeyTypeWireDesc *keytype;
+    xkbSymMapWireDesc *symmap;
+    BOOL preserve;
+    int i, map_count, nSyms;
+
+    if (req_len < len)
+        goto bad;
+    /* types */
+    if (req->present & XkbKeyTypesMask) {
+        keytype = (xkbKeyTypeWireDesc *)(req + 1);
+        for (i = 0; i < req->nTypes; i++) {
+            _add_check_len(XkbPaddedSize(sz_xkbKeyTypeWireDesc));
+            if (req->flags & XkbSetMapResizeTypes) {
+                _add_check_len(keytype->nMapEntries
+                               * sz_xkbKTSetMapEntryWireDesc);
+                preserve = keytype->preserve;
+                map_count = keytype->nMapEntries;
+                if (preserve) {
+                    _add_check_len(map_count * sz_xkbModsWireDesc);
+                }
+                keytype += 1;
+                keytype = (xkbKeyTypeWireDesc *)
+                          ((xkbKTSetMapEntryWireDesc *)keytype + map_count);
+                if (preserve)
+                    keytype = (xkbKeyTypeWireDesc *)
+                              ((xkbModsWireDesc *)keytype + map_count);
+            }
+        }
+    }
+    /* syms */
+    if (req->present & XkbKeySymsMask) {
+        symmap = (xkbSymMapWireDesc *)((char *)req + len);
+        for (i = 0; i < req->nKeySyms; i++) {
+            _add_check_len(sz_xkbSymMapWireDesc);
+            nSyms = symmap->nSyms;
+            _add_check_len(nSyms*sizeof(CARD32));
+            symmap += 1;
+            symmap = (xkbSymMapWireDesc *)((CARD32 *)symmap + nSyms);
+        }
+    }
+    /* actions */
+    if (req->present & XkbKeyActionsMask) {
+       _add_check_len(req->totalActs * sz_xkbActionWireDesc 
+                       + XkbPaddedSize(req->nKeyActs));
+    }
+    /* behaviours */
+    if (req->present & XkbKeyBehaviorsMask) {
+        _add_check_len(req->totalKeyBehaviors * sz_xkbBehaviorWireDesc);
+    }
+    /* vmods */
+    if (req->present & XkbVirtualModsMask) {
+        _add_check_len(XkbPaddedSize(Ones(req->virtualMods)));
+    }
+    /* explicit */
+    if (req->present & XkbExplicitComponentsMask) {
+        /* two bytes per non-zero explicit componen */
+        _add_check_len(XkbPaddedSize(req->totalKeyExplicit * sizeof(CARD16)));
+    }
+    /* modmap */
+    if (req->present & XkbModifierMapMask) {
+         /* two bytes per non-zero modmap component */
+        _add_check_len(XkbPaddedSize(req->totalModMapKeys * sizeof(CARD16)));
+    }
+    /* vmodmap */
+    if (req->present & XkbVirtualModMapMask) {
+        _add_check_len(req->totalVModMapKeys * sz_xkbVModMapWireDesc);
+    }
+    if (len == req_len)
+        return Success;
+bad:
+    ErrorF("[xkb] BOGUS LENGTH in SetMap: expected %ld got %ld\n",
+           len, req_len);
+    return BadLength;
+}
 
 /**
  * Check if the given request can be applied to the given device but don't
- * actually do anything..
+ * actually do anything, except swap values when client->swapped and doswap are both true.
  */
 static int
-_XkbSetMapChecks(ClientPtr client, DeviceIntPtr dev, xkbSetMapReq *req, char* values)
+_XkbSetMapChecks(ClientPtr client, DeviceIntPtr dev, xkbSetMapReq *req, char* values, Bool doswap)
 {
     XkbSrvInfoPtr       xkbi;
     XkbDescPtr          xkb;
@@ -2362,10 +2463,13 @@
 
     if ((req->present & XkbKeyTypesMask) &&
        (!CheckKeyTypes(client,xkb,req,(xkbKeyTypeWireDesc **)&values,
-                                               &nTypes,mapWidths))) {
+                                               &nTypes,mapWidths, doswap))) {
        client->errorValue = nTypes;
        return BadValue;
     }
+    else {
+        nTypes = xkb->map->num_types;
+    }
 
     /* symsPerKey/mapWidths must be filled regardless of client-side flags */
     map = &xkb->map->key_sym_map[xkb->min_key_code];
@@ -2375,7 +2479,7 @@
        for (w=g=0;g<ng;g++) {
            if (map->kt_index[g]>=(unsigned)nTypes) {
                client->errorValue = _XkbErrCode4(0x13,i,g,map->kt_index[g]);
-               return 0;
+               return BadValue;
            }
            if (mapWidths[map->kt_index[g]]>w)
                w= mapWidths[map->kt_index[g]];
@@ -2385,7 +2489,7 @@
 
     if ((req->present & XkbKeySymsMask) &&
        (!CheckKeySyms(client,xkb,req,nTypes,mapWidths,symsPerKey,
-                                       (xkbSymMapWireDesc **)&values,&error))) {
+                                       (xkbSymMapWireDesc **)&values,&error, doswap))) {
        client->errorValue = error;
        return BadValue;
     }
@@ -2554,12 +2658,17 @@
     CHK_KBD_DEVICE(dev, stuff->deviceSpec, client, DixManageAccess);
     CHK_MASK_LEGAL(0x01,stuff->present,XkbAllMapComponentsMask);
 
+    /* first verify the request length carefully */
+    rc = _XkbSetMapCheckLength(stuff);
+    if (rc != Success)
+        return rc;
+
     tmp = (char *)&stuff[1];
 
     /* Check if we can to the SetMap on the requested device. If this
        succeeds, do the same thing for all extension devices (if needed).
        If any of them fails, fail.  */
-    rc = _XkbSetMapChecks(client, dev, stuff, tmp);
+    rc = _XkbSetMapChecks(client, dev, stuff, tmp, TRUE);
 
     if (rc != Success)
         return rc;
@@ -2574,7 +2683,7 @@
                 rc = XaceHook(XACE_DEVICE_ACCESS, client, other, DixManageAccess);
                 if (rc == Success)
                 {
-                    rc = _XkbSetMapChecks(client, other, stuff, tmp);
+                    rc = _XkbSetMapChecks(client, other, stuff, tmp, FALSE);
                     if (rc != Success)
                         return rc;
                 }
@@ -3914,6 +4023,8 @@
             client->errorValue = _XkbErrCode2(0x04,stuff->firstType);
             return BadAccess;
         }
+        if (!_XkbCheckRequestBounds(client, stuff, tmp, tmp + stuff->nTypes))
+            return BadLength;
         old= tmp;
         tmp= _XkbCheckAtoms(tmp,stuff->nTypes,client->swapped,&bad);
         if (!tmp) {
@@ -3941,6 +4052,8 @@
         }
         width = (CARD8 *)tmp;
         tmp= (CARD32 *)(((char *)tmp)+XkbPaddedSize(stuff->nKTLevels));
+        if (!_XkbCheckRequestBounds(client, stuff, width, tmp))
+            return BadLength;
         type = &xkb->map->types[stuff->firstKTLevel];
         for (i=0;i<stuff->nKTLevels;i++,type++) {
             if (width[i]==0)
@@ -3950,6 +4063,8 @@
                         type->num_levels,width[i]);
                 return BadMatch;
             }
+            if (!_XkbCheckRequestBounds(client, stuff, tmp, tmp + width[i]))
+                return BadLength;
             tmp= _XkbCheckAtoms(tmp,width[i],client->swapped,&bad);
             if (!tmp) {
                 client->errorValue= bad;
@@ -3962,6 +4077,9 @@
             client->errorValue= 0x08;
             return BadMatch;
         }
+        if (!_XkbCheckRequestBounds(client, stuff, tmp,
+                                    tmp + Ones(stuff->indicators)))
+            return BadLength;
         tmp= _XkbCheckMaskedAtoms(tmp,XkbNumIndicators,stuff->indicators,
                 client->swapped,&bad);
         if (!tmp) {
@@ -3974,6 +4092,9 @@
             client->errorValue= 0x09;
             return BadMatch;
         }
+        if (!_XkbCheckRequestBounds(client, stuff, tmp,
+                                    tmp + Ones(stuff->virtualMods)))
+            return BadLength;
         tmp= _XkbCheckMaskedAtoms(tmp,XkbNumVirtualMods,
                 (CARD32)stuff->virtualMods,
                 client->swapped,&bad);
@@ -3987,6 +4108,9 @@
             client->errorValue= 0x0a;
             return BadMatch;
         }
+        if (!_XkbCheckRequestBounds(client, stuff, tmp,
+                                    tmp + Ones(stuff->groupNames)))
+            return BadLength;
         tmp= _XkbCheckMaskedAtoms(tmp,XkbNumKbdGroups,
                 (CARD32)stuff->groupNames,
                 client->swapped,&bad);
@@ -4007,9 +4131,14 @@



Home | Main Index | Thread Index | Old Index