181 lines
7.3 KiB
Diff
181 lines
7.3 KiB
Diff
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
|
|
|