Hi,
I have prepared an update for xorg-server, addressing CVE-2022-2319 and
CVE-2022-2320. I have tested it on my development machine without any issues so
far, and I'm not aware of any upstream regressions.
fwiw looks like these also affect src:xwayland (which is not in any Debian
release), I'll double check and add it to the security-tracker if so, so that it
can be tracked.
Let me know if I should upload xorg-server to security-master.
Cheers,
Emilio
diff -u xorg-server-1.20.11/debian/changelog
xorg-server-1.20.11/debian/changelog
--- xorg-server-1.20.11/debian/changelog
+++ xorg-server-1.20.11/debian/changelog
@@ -1,3 +1,10 @@
+xorg-server (2:1.20.11-1+deb11u2) bullseye-security; urgency=medium
+
+ * xkb: add request length validation for XkbSetGeometry (CVE-2022-2319)
+ * xkb: swap XkbSetDeviceInfo and XkbSetDeviceInfoCheck (CVE-2022-2320)
+
+ -- Emilio Pozuelo Monfort <po...@debian.org> Fri, 05 Aug 2022 10:00:36 +0200
+
xorg-server (2:1.20.11-1+deb11u1) bullseye-security; urgency=high
* Team upload.
diff -u xorg-server-1.20.11/debian/patches/series
xorg-server-1.20.11/debian/patches/series
--- xorg-server-1.20.11/debian/patches/series
+++ xorg-server-1.20.11/debian/patches/series
@@ -7,3 +7,6 @@
05_Revert-Unload-submodules.diff
06_use-intel-only-on-pre-gen4.diff
07_use-modesetting-driver-by-default-on-GeForce.diff
+08_xkb-switch-to-array-index-loops-to-moving-pointers.patch
+09_xkb-add-request-length-validation-for-XkbSetGeometry.patch
+10_xkb-swap-XkbSetDeviceInfo-and-XkbSetDeviceInfoCheck.patch
only in patch2:
unchanged:
---
xorg-server-1.20.11.orig/debian/patches/08_xkb-switch-to-array-index-loops-to-moving-pointers.patch
+++
xorg-server-1.20.11/debian/patches/08_xkb-switch-to-array-index-loops-to-moving-pointers.patch
@@ -0,0 +1,75 @@
+From f1070c01d616c5f21f939d5ebc533738779451ac Mon Sep 17 00:00:00 2001
+From: Peter Hutterer <peter.hutte...@who-t.net>
+Date: Tue, 5 Jul 2022 12:40:47 +1000
+Subject: [PATCH] xkb: switch to array index loops to moving pointers
+
+Most similar loops here use a pointer that advances with each loop
+iteration, let's do the same here for consistency.
+
+No functional changes.
+
+Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
+Reviewed-by: Olivier Fourdan <ofour...@redhat.com>
+---
+ xkb/xkb.c | 20 ++++++++++----------
+ 1 file changed, 10 insertions(+), 10 deletions(-)
+
+diff --git a/xkb/xkb.c b/xkb/xkb.c
+index a29262c24..64e52611e 100644
+--- a/xkb/xkb.c
++++ b/xkb/xkb.c
+@@ -5368,16 +5368,16 @@ _CheckSetSections(XkbGeometryPtr geom,
+ row->left = rWire->left;
+ row->vertical = rWire->vertical;
+ kWire = (xkbKeyWireDesc *) &rWire[1];
+- for (k = 0; k < rWire->nKeys; k++) {
++ for (k = 0; k < rWire->nKeys; k++, kWire++) {
+ XkbKeyPtr key;
+
+ key = XkbAddGeomKey(row);
+ if (!key)
+ return BadAlloc;
+- memcpy(key->name.name, kWire[k].name, XkbKeyNameLength);
+- key->gap = kWire[k].gap;
+- key->shape_ndx = kWire[k].shapeNdx;
+- key->color_ndx = kWire[k].colorNdx;
++ memcpy(key->name.name, kWire->name, XkbKeyNameLength);
++ key->gap = kWire->gap;
++ key->shape_ndx = kWire->shapeNdx;
++ key->color_ndx = kWire->colorNdx;
+ if (key->shape_ndx >= geom->num_shapes) {
+ client->errorValue = _XkbErrCode3(0x10, key->shape_ndx,
+ geom->num_shapes);
+@@ -5389,7 +5389,7 @@ _CheckSetSections(XkbGeometryPtr geom,
+ return BadMatch;
+ }
+ }
+- rWire = (xkbRowWireDesc *) &kWire[rWire->nKeys];
++ rWire = (xkbRowWireDesc *)kWire;
+ }
+ wire = (char *) rWire;
+ if (sWire->nDoodads > 0) {
+@@ -5454,16 +5454,16 @@ _CheckSetShapes(XkbGeometryPtr geom,
+ return BadAlloc;
+ ol->corner_radius = olWire->cornerRadius;
+ ptWire = (xkbPointWireDesc *) &olWire[1];
+- for (p = 0, pt = ol->points; p < olWire->nPoints; p++, pt++) {
+- pt->x = ptWire[p].x;
+- pt->y = ptWire[p].y;
++ for (p = 0, pt = ol->points; p < olWire->nPoints; p++, pt++,
ptWire++) {
++ pt->x = ptWire->x;
++ pt->y = ptWire->y;
+ if (client->swapped) {
+ swaps(&pt->x);
+ swaps(&pt->y);
+ }
+ }
+ ol->num_points = olWire->nPoints;
+- olWire = (xkbOutlineWireDesc *) (&ptWire[olWire->nPoints]);
++ olWire = (xkbOutlineWireDesc *)ptWire;
+ }
+ if (shapeWire->primaryNdx != XkbNoShape)
+ shape->primary = &shape->outlines[shapeWire->primaryNdx];
+--
+2.30.2
+
only in patch2:
unchanged:
---
xorg-server-1.20.11.orig/debian/patches/09_xkb-add-request-length-validation-for-XkbSetGeometry.patch
+++
xorg-server-1.20.11/debian/patches/09_xkb-add-request-length-validation-for-XkbSetGeometry.patch
@@ -0,0 +1,181 @@
+From 6907b6ea2b4ce949cb07271f5b678d5966d9df42 Mon Sep 17 00:00:00 2001
+From: Peter Hutterer <peter.hutte...@who-t.net>
+Date: Tue, 5 Jul 2022 11:11:06 +1000
+Subject: [PATCH] xkb: add request length validation for XkbSetGeometry
+
+No validation of the various fields on that report were done, so a
+malicious client could send a short request that claims it had N
+sections, or rows, or keys, and the server would process the request for
+N sections, running out of bounds of the actual request data.
+
+Fix this by adding size checks to ensure our data is valid.
+
+ZDI-CAN 16062, CVE-2022-2319.
+
+This vulnerability was discovered by:
+Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
+
+Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
+---
+ xkb/xkb.c | 43 ++++++++++++++++++++++++++++++++++++++-----
+ 1 file changed, 38 insertions(+), 5 deletions(-)
+
+diff --git a/xkb/xkb.c b/xkb/xkb.c
+index 34b2c290b..4692895db 100644
+--- a/xkb/xkb.c
++++ b/xkb/xkb.c
+@@ -5156,7 +5156,7 @@ _GetCountedString(char **wire_inout, ClientPtr client,
char **str)
+ }
+
+ static Status
+-_CheckSetDoodad(char **wire_inout,
++_CheckSetDoodad(char **wire_inout, xkbSetGeometryReq *req,
+ XkbGeometryPtr geom, XkbSectionPtr section, ClientPtr client)
+ {
+ char *wire;
+@@ -5167,6 +5167,9 @@ _CheckSetDoodad(char **wire_inout,
+ Status status;
+
+ dWire = (xkbDoodadWireDesc *) (*wire_inout);
++ if (!_XkbCheckRequestBounds(client, req, dWire, dWire + 1))
++ return BadLength;
++
+ any = dWire->any;
+ wire = (char *) &dWire[1];
+ if (client->swapped) {
+@@ -5269,7 +5272,7 @@ _CheckSetDoodad(char **wire_inout,
+ }
+
+ static Status
+-_CheckSetOverlay(char **wire_inout,
++_CheckSetOverlay(char **wire_inout, xkbSetGeometryReq *req,
+ XkbGeometryPtr geom, XkbSectionPtr section, ClientPtr client)
+ {
+ register int r;
+@@ -5280,6 +5283,9 @@ _CheckSetOverlay(char **wire_inout,
+
+ wire = *wire_inout;
+ olWire = (xkbOverlayWireDesc *) wire;
++ if (!_XkbCheckRequestBounds(client, req, olWire, olWire + 1))
++ return BadLength;
++
+ if (client->swapped) {
+ swapl(&olWire->name);
+ }
+@@ -5291,6 +5297,9 @@ _CheckSetOverlay(char **wire_inout,
+ xkbOverlayKeyWireDesc *kWire;
+ XkbOverlayRowPtr row;
+
++ if (!_XkbCheckRequestBounds(client, req, rWire, rWire + 1))
++ return BadLength;
++
+ if (rWire->rowUnder > section->num_rows) {
+ client->errorValue = _XkbErrCode4(0x20, r, section->num_rows,
+ rWire->rowUnder);
+@@ -5299,6 +5308,9 @@ _CheckSetOverlay(char **wire_inout,
+ row = XkbAddGeomOverlayRow(ol, rWire->rowUnder, rWire->nKeys);
+ kWire = (xkbOverlayKeyWireDesc *) &rWire[1];
+ for (k = 0; k < rWire->nKeys; k++, kWire++) {
++ if (!_XkbCheckRequestBounds(client, req, kWire, kWire + 1))
++ return BadLength;
++
+ if (XkbAddGeomOverlayKey(ol, row,
+ (char *) kWire->over,
+ (char *) kWire->under) == NULL) {
+@@ -5332,6 +5344,9 @@ _CheckSetSections(XkbGeometryPtr geom,
+ register int r;
+ xkbRowWireDesc *rWire;
+
++ if (!_XkbCheckRequestBounds(client, req, sWire, sWire + 1))
++ return BadLength;
++
+ if (client->swapped) {
+ swapl(&sWire->name);
+ swaps(&sWire->top);
+@@ -5357,6 +5372,9 @@ _CheckSetSections(XkbGeometryPtr geom,
+ XkbRowPtr row;
+ xkbKeyWireDesc *kWire;
+
++ if (!_XkbCheckRequestBounds(client, req, rWire, rWire + 1))
++ return BadLength;
++
+ if (client->swapped) {
+ swaps(&rWire->top);
+ swaps(&rWire->left);
+@@ -5371,6 +5389,9 @@ _CheckSetSections(XkbGeometryPtr geom,
+ for (k = 0; k < rWire->nKeys; k++, kWire++) {
+ XkbKeyPtr key;
+
++ if (!_XkbCheckRequestBounds(client, req, kWire, kWire + 1))
++ return BadLength;
++
+ key = XkbAddGeomKey(row);
+ if (!key)
+ return BadAlloc;
+@@ -5396,7 +5417,7 @@ _CheckSetSections(XkbGeometryPtr geom,
+ register int d;
+
+ for (d = 0; d < sWire->nDoodads; d++) {
+- status = _CheckSetDoodad(&wire, geom, section, client);
++ status = _CheckSetDoodad(&wire, req, geom, section, client);
+ if (status != Success)
+ return status;
+ }
+@@ -5405,7 +5426,7 @@ _CheckSetSections(XkbGeometryPtr geom,
+ register int o;
+
+ for (o = 0; o < sWire->nOverlays; o++) {
+- status = _CheckSetOverlay(&wire, geom, section, client);
++ status = _CheckSetOverlay(&wire, req, geom, section, client);
+ if (status != Success)
+ return status;
+ }
+@@ -5439,6 +5460,9 @@ _CheckSetShapes(XkbGeometryPtr geom,
+ xkbOutlineWireDesc *olWire;
+ XkbOutlinePtr ol;
+
++ if (!_XkbCheckRequestBounds(client, req, shapeWire, shapeWire +
1))
++ return BadLength;
++
+ shape =
+ XkbAddGeomShape(geom, shapeWire->name, shapeWire->nOutlines);
+ if (!shape)
+@@ -5449,12 +5473,18 @@ _CheckSetShapes(XkbGeometryPtr geom,
+ XkbPointPtr pt;
+ xkbPointWireDesc *ptWire;
+
++ if (!_XkbCheckRequestBounds(client, req, olWire, olWire + 1))
++ return BadLength;
++
+ ol = XkbAddGeomOutline(shape, olWire->nPoints);
+ if (!ol)
+ return BadAlloc;
+ ol->corner_radius = olWire->cornerRadius;
+ ptWire = (xkbPointWireDesc *) &olWire[1];
+ for (p = 0, pt = ol->points; p < olWire->nPoints; p++, pt++,
ptWire++) {
++ if (!_XkbCheckRequestBounds(client, req, ptWire, ptWire +
1))
++ return BadLength;
++
+ pt->x = ptWire->x;
+ pt->y = ptWire->y;
+ if (client->swapped) {
+@@ -5560,12 +5590,15 @@ _CheckSetGeom(XkbGeometryPtr geom, xkbSetGeometryReq *
req, ClientPtr client)
+ return status;
+
+ for (i = 0; i < req->nDoodads; i++) {
+- status = _CheckSetDoodad(&wire, geom, NULL, client);
++ status = _CheckSetDoodad(&wire, req, geom, NULL, client);
+ if (status != Success)
+ return status;
+ }
+
+ for (i = 0; i < req->nKeyAliases; i++) {
++ if (!_XkbCheckRequestBounds(client, req, wire, wire +
XkbKeyNameLength))
++ return BadLength;
++
+ if (XkbAddGeomKeyAlias(geom, &wire[XkbKeyNameLength], wire) == NULL)
+ return BadAlloc;
+ wire += 2 * XkbKeyNameLength;
+--
+2.30.2
+
only in patch2:
unchanged:
---
xorg-server-1.20.11.orig/debian/patches/10_xkb-swap-XkbSetDeviceInfo-and-XkbSetDeviceInfoCheck.patch
+++
xorg-server-1.20.11/debian/patches/10_xkb-swap-XkbSetDeviceInfo-and-XkbSetDeviceInfoCheck.patch
@@ -0,0 +1,178 @@
+From dd8caf39e9e15d8f302e54045dd08d8ebf1025dc Mon Sep 17 00:00:00 2001
+From: Peter Hutterer <peter.hutte...@who-t.net>
+Date: Tue, 5 Jul 2022 09:50:41 +1000
+Subject: [PATCH] xkb: swap XkbSetDeviceInfo and XkbSetDeviceInfoCheck
+
+XKB often uses a FooCheck and Foo function pair, the former is supposed
+to check all values in the request and error out on BadLength,
+BadValue, etc. The latter is then called once we're confident the values
+are good (they may still fail on an individual device, but that's a
+different topic).
+
+In the case of XkbSetDeviceInfo, those functions were incorrectly
+named, with XkbSetDeviceInfo ending up as the checker function and
+XkbSetDeviceInfoCheck as the setter function. As a result, the setter
+function was called before the checker function, accessing request
+data and modifying device state before we ensured that the data is
+valid.
+
+In particular, the setter function relied on values being already
+byte-swapped. This in turn could lead to potential OOB memory access.
+
+Fix this by correctly naming the functions and moving the length checks
+over to the checker function. These were added in 87c64fc5b0 to the
+wrong function, probably due to the incorrect naming.
+
+Fixes ZDI-CAN 16070, CVE-2022-2320.
+
+This vulnerability was discovered by:
+Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
+
+Introduced in c06e27b2f6fd9f7b9f827623a48876a225264132
+
+Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
+---
+ xkb/xkb.c | 46 +++++++++++++++++++++++++---------------------
+ 1 file changed, 25 insertions(+), 21 deletions(-)
+
+diff --git a/xkb/xkb.c b/xkb/xkb.c
+index 64e52611e..34b2c290b 100644
+--- a/xkb/xkb.c
++++ b/xkb/xkb.c
+@@ -6550,7 +6550,8 @@ ProcXkbGetDeviceInfo(ClientPtr client)
+ static char *
+ CheckSetDeviceIndicators(char *wire,
+ DeviceIntPtr dev,
+- int num, int *status_rtrn, ClientPtr client)
++ int num, int *status_rtrn, ClientPtr client,
++ xkbSetDeviceInfoReq * stuff)
+ {
+ xkbDeviceLedsWireDesc *ledWire;
+ int i;
+@@ -6558,6 +6559,11 @@ CheckSetDeviceIndicators(char *wire,
+
+ ledWire = (xkbDeviceLedsWireDesc *) wire;
+ for (i = 0; i < num; i++) {
++ if (!_XkbCheckRequestBounds(client, stuff, ledWire, ledWire + 1)) {
++ *status_rtrn = BadLength;
++ return (char *) ledWire;
++ }
++
+ if (client->swapped) {
+ swaps(&ledWire->ledClass);
+ swaps(&ledWire->ledID);
+@@ -6585,6 +6591,11 @@ CheckSetDeviceIndicators(char *wire,
+ atomWire = (CARD32 *) &ledWire[1];
+ if (nNames > 0) {
+ for (n = 0; n < nNames; n++) {
++ if (!_XkbCheckRequestBounds(client, stuff, atomWire,
atomWire + 1)) {
++ *status_rtrn = BadLength;
++ return (char *) atomWire;
++ }
++
+ if (client->swapped) {
+ swapl(atomWire);
+ }
+@@ -6596,6 +6607,10 @@ CheckSetDeviceIndicators(char *wire,
+ mapWire = (xkbIndicatorMapWireDesc *) atomWire;
+ if (nMaps > 0) {
+ for (n = 0; n < nMaps; n++) {
++ if (!_XkbCheckRequestBounds(client, stuff, mapWire,
mapWire + 1)) {
++ *status_rtrn = BadLength;
++ return (char *) mapWire;
++ }
+ if (client->swapped) {
+ swaps(&mapWire->virtualMods);
+ swapl(&mapWire->ctrls);
+@@ -6647,11 +6662,6 @@ SetDeviceIndicators(char *wire,
+ xkbIndicatorMapWireDesc *mapWire;
+ XkbSrvLedInfoPtr sli;
+
+- if (!_XkbCheckRequestBounds(client, stuff, ledWire, ledWire + 1)) {
+- *status_rtrn = BadLength;
+- return (char *) ledWire;
+- }
+-
+ namec = mapc = statec = 0;
+ sli = XkbFindSrvLedInfo(dev, ledWire->ledClass, ledWire->ledID,
+ XkbXI_IndicatorMapsMask);
+@@ -6670,10 +6680,6 @@ SetDeviceIndicators(char *wire,
+ memset((char *) sli->names, 0, XkbNumIndicators * sizeof(Atom));
+ for (n = 0, bit = 1; n < XkbNumIndicators; n++, bit <<= 1) {
+ if (ledWire->namesPresent & bit) {
+- if (!_XkbCheckRequestBounds(client, stuff, atomWire,
atomWire + 1)) {
+- *status_rtrn = BadLength;
+- return (char *) atomWire;
+- }
+ sli->names[n] = (Atom) *atomWire;
+ if (sli->names[n] == None)
+ ledWire->namesPresent &= ~bit;
+@@ -6691,10 +6697,6 @@ SetDeviceIndicators(char *wire,
+ if (ledWire->mapsPresent) {
+ for (n = 0, bit = 1; n < XkbNumIndicators; n++, bit <<= 1) {
+ if (ledWire->mapsPresent & bit) {
+- if (!_XkbCheckRequestBounds(client, stuff, mapWire,
mapWire + 1)) {
+- *status_rtrn = BadLength;
+- return (char *) mapWire;
+- }
+ sli->maps[n].flags = mapWire->flags;
+ sli->maps[n].which_groups = mapWire->whichGroups;
+ sli->maps[n].groups = mapWire->groups;
+@@ -6730,13 +6732,17 @@ SetDeviceIndicators(char *wire,
+ }
+
+ static int
+-_XkbSetDeviceInfo(ClientPtr client, DeviceIntPtr dev,
++_XkbSetDeviceInfoCheck(ClientPtr client, DeviceIntPtr dev,
+ xkbSetDeviceInfoReq * stuff)
+ {
+ char *wire;
+
+ wire = (char *) &stuff[1];
+ if (stuff->change & XkbXI_ButtonActionsMask) {
++ int sz = stuff->nBtns * SIZEOF(xkbActionWireDesc);
++ if (!_XkbCheckRequestBounds(client, stuff, wire, (char *) wire + sz))
++ return BadLength;
++
+ if (!dev->button) {
+ client->errorValue = _XkbErrCode2(XkbErr_BadClass, ButtonClass);
+ return XkbKeyboardErrorCode;
+@@ -6747,13 +6753,13 @@ _XkbSetDeviceInfo(ClientPtr client, DeviceIntPtr dev,
+ dev->button->numButtons);
+ return BadMatch;
+ }
+- wire += (stuff->nBtns * SIZEOF(xkbActionWireDesc));
++ wire += sz;
+ }
+ if (stuff->change & XkbXI_IndicatorsMask) {
+ int status = Success;
+
+ wire = CheckSetDeviceIndicators(wire, dev, stuff->nDeviceLedFBs,
+- &status, client);
++ &status, client, stuff);
+ if (status != Success)
+ return status;
+ }
+@@ -6764,8 +6770,8 @@ _XkbSetDeviceInfo(ClientPtr client, DeviceIntPtr dev,
+ }
+
+ static int
+-_XkbSetDeviceInfoCheck(ClientPtr client, DeviceIntPtr dev,
+- xkbSetDeviceInfoReq * stuff)
++_XkbSetDeviceInfo(ClientPtr client, DeviceIntPtr dev,
++ xkbSetDeviceInfoReq * stuff)
+ {
+ char *wire;
+ xkbExtensionDeviceNotify ed;
+@@ -6789,8 +6795,6 @@ _XkbSetDeviceInfoCheck(ClientPtr client, DeviceIntPtr
dev,
+ if (stuff->firstBtn + stuff->nBtns > nBtns)
+ return BadValue;
+ sz = stuff->nBtns * SIZEOF(xkbActionWireDesc);
+- if (!_XkbCheckRequestBounds(client, stuff, wire, (char *) wire + sz))
+- return BadLength;
+ memcpy((char *) &acts[stuff->firstBtn], (char *) wire, sz);
+ wire += sz;
+ ed.reason |= XkbXI_ButtonActionsMask;
+--
+2.30.2
+
