rpms/xorg-x11-server
7
0
Fork 0
Code Issues Pull Requests Releases Activity

Fix CVE-2022-2319/ZDI-CAN-16062, CVE-2022-2320/ZDI-CAN-16070

Browse Source 
Resolves: rhbz#2108157, rhbz#2108162
This commit is contained in:
Olivier Fourdan 2022-07-29 10:25:05 +02:00
parent 7374372354
commit e1f8d8cc41
4 changed files with 449 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

77
0001-xkb-switch-to-array-index-loops-to-moving-pointers.patch Normal file
View File

@ -0,0 +1,77 @@
From c9b379ec5a1a34692af06056925bd0fc5f809713 Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutterer@who-t.net>
Date: Tue, 5 Jul 2022 12:40:47 +1000
Subject: [PATCH xserver 1/3] 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.hutterer@who-t.net>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
(cherry picked from commit f1070c01d616c5f21f939d5ebc533738779451ac)
---
xkb/xkb.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/xkb/xkb.c b/xkb/xkb.c
index d056c698c..684394d77 100644
--- a/xkb/xkb.c
+++ b/xkb/xkb.c
@@ -5372,16 +5372,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);
@@ -5393,7 +5393,7 @@ _CheckSetSections(XkbGeometryPtr geom,
return BadMatch;
}
}
- rWire = (xkbRowWireDesc *) &kWire[rWire->nKeys];
+ rWire = (xkbRowWireDesc *)kWire;
}
wire = (char *) rWire;
if (sWire->nDoodads > 0) {
@@ -5458,16 +5458,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.36.1

180
0002-xkb-swap-XkbSetDeviceInfo-and-XkbSetDeviceInfoCheck.patch Normal file
View File

@ -0,0 +1,180 @@
From 45a0af83129eb7dc244c5118360afc1972a686c7 Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutterer@who-t.net>
Date: Tue, 5 Jul 2022 09:50:41 +1000
Subject: [PATCH xserver 2/3] 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.hutterer@who-t.net>
(cherry picked from commit dd8caf39e9e15d8f302e54045dd08d8ebf1025dc)
---
xkb/xkb.c | 46 +++++++++++++++++++++++++---------------------
1 file changed, 25 insertions(+), 21 deletions(-)
diff --git a/xkb/xkb.c b/xkb/xkb.c
index 684394d77..36464a770 100644
--- a/xkb/xkb.c
+++ b/xkb/xkb.c
@@ -6554,7 +6554,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;
@@ -6562,6 +6563,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);
@@ -6589,6 +6595,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);
}
@@ -6600,6 +6611,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);
@@ -6651,11 +6666,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);
@@ -6674,10 +6684,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;
@@ -6695,10 +6701,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;
@@ -6734,13 +6736,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;
@@ -6751,13 +6757,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;
}
@@ -6768,8 +6774,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;
@@ -6793,8 +6799,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.36.1

183
0003-xkb-add-request-length-validation-for-XkbSetGeometry.patch Normal file
View File

@ -0,0 +1,183 @@
From bd134231e282d9eb126b6fdaa40bb383180fa72b Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutterer@who-t.net>
Date: Tue, 5 Jul 2022 11:11:06 +1000
Subject: [PATCH xserver 3/3] 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.hutterer@who-t.net>
(cherry picked from commit 6907b6ea2b4ce949cb07271f5b678d5966d9df42)
---
xkb/xkb.c | 43 ++++++++++++++++++++++++++++++++++++++-----
1 file changed, 38 insertions(+), 5 deletions(-)
diff --git a/xkb/xkb.c b/xkb/xkb.c
index 36464a770..27d19793e 100644
--- a/xkb/xkb.c
+++ b/xkb/xkb.c
@@ -5160,7 +5160,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;
@@ -5171,6 +5171,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) {
@@ -5273,7 +5276,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;
@@ -5284,6 +5287,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);
}
@@ -5295,6 +5301,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);
@@ -5303,6 +5312,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) {
@@ -5336,6 +5348,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);
@@ -5361,6 +5376,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);
@@ -5375,6 +5393,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;
@@ -5400,7 +5421,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;
}
@@ -5409,7 +5430,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;
}
@@ -5443,6 +5464,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)
@@ -5453,12 +5477,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) {
@@ -5564,12 +5594,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.36.1

11
xorg-x11-server.spec
View File

@ -42,7 +42,7 @@
Summary: X.Org X11 X server
Name: xorg-x11-server
Version: 1.20.11
Release: 10%{?gitdate:.%{gitdate}}%{?dist}
Release: 11%{?gitdate:.%{gitdate}}%{?dist}
URL: http://www.x.org
License: MIT
@ -124,7 +124,10 @@ Patch10010: 0002-xfixes-Fix-out-of-bounds-access-in-ProcXFixesCreateP.patch
Patch10011: 0003-Xext-Fix-out-of-bounds-access-in-SProcScreenSaverSus.patch
# CVE-2021-4008
Patch10012: 0004-render-Fix-out-of-bounds-access-in-SProcRenderCompos.patch
# CVE-2022-2319/ZDI-CAN-16062, CVE-2022-2320/ZDI-CAN-16070
Patch10013: 0001-xkb-switch-to-array-index-loops-to-moving-pointers.patch
Patch10014: 0002-xkb-swap-XkbSetDeviceInfo-and-XkbSetDeviceInfoCheck.patch
Patch10015: 0003-xkb-add-request-length-validation-for-XkbSetGeometry.patch
BuildRequires: make
BuildRequires: systemtap-sdt-devel
@ -535,6 +538,10 @@ find %{inst_srcdir}/hw/xfree86 -name \*.c -delete
%changelog
* Fri Jul 29 2022 Olivier Fourdan <ofourdan@redhat.com> - 1.20.11-11
- CVE fix for: CVE-2022-2319/ZDI-CAN-16062, CVE-2022-2320/ZDI-CAN-16070
Resolves: rhbz#2108157, rhbz#2108162
* Thu Feb 10 2022 Olivier Fourdan <ofourdan@redhat.com> - 1.20.11-10
- Fix a regression with hybrid gfx and NVIDIA proprietary driver (#2052605)