Compare commits

...

2 Commits

13 changed files with 1001 additions and 1 deletions

View File

@ -0,0 +1 @@
86ae4add5719e6026a569f5559d51e8707171d5d xorg-server-1.20.11.tar.bz2

View File

@ -0,0 +1,77 @@
From 1801fe0ac3926882d47d7e1ad6c0518a2cdffd41 Mon Sep 17 00:00:00 2001
From: Povilas Kanapickas <povilas@radix.lt>
Date: Sun, 19 Dec 2021 18:11:07 +0200
Subject: [PATCH] dix: Fix use after free in input device shutdown
This fixes access to freed heap memory via dev->master. E.g. when
running BarrierNotify.ReceivesNotifyEvents/7 test from
xorg-integration-tests:
==24736==ERROR: AddressSanitizer: heap-use-after-free on address
0x619000065020 at pc 0x55c450e2b9cf bp 0x7fffc532fd20 sp 0x7fffc532fd10
READ of size 4 at 0x619000065020 thread T0
#0 0x55c450e2b9ce in GetMaster ../../../dix/devices.c:2722
#1 0x55c450e9d035 in IsFloating ../../../dix/events.c:346
#2 0x55c4513209c6 in GetDeviceUse ../../../Xi/xiquerydevice.c:525
../../../Xi/xichangehierarchy.c:95
#4 0x55c450e3455c in RemoveDevice ../../../dix/devices.c:1204
../../../hw/xfree86/common/xf86Xinput.c:1142
#6 0x55c450e17b04 in CloseDeviceList ../../../dix/devices.c:1038
#7 0x55c450e1de85 in CloseDownDevices ../../../dix/devices.c:1068
#8 0x55c450e837ef in dix_main ../../../dix/main.c:302
#9 0x55c4517a8d93 in main ../../../dix/stubmain.c:34
(/lib/x86_64-linux-gnu/libc.so.6+0x28564)
#11 0x55c450d0113d in _start (/usr/lib/xorg/Xorg+0x117713d)
0x619000065020 is located 160 bytes inside of 912-byte region
[0x619000064f80,0x619000065310)
freed by thread T0 here:
(/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10d7cf)
#1 0x55c450e19f1c in CloseDevice ../../../dix/devices.c:1014
#2 0x55c450e343a4 in RemoveDevice ../../../dix/devices.c:1186
../../../hw/xfree86/common/xf86Xinput.c:1142
#4 0x55c450e17b04 in CloseDeviceList ../../../dix/devices.c:1038
#5 0x55c450e1de85 in CloseDownDevices ../../../dix/devices.c:1068
#6 0x55c450e837ef in dix_main ../../../dix/main.c:302
#7 0x55c4517a8d93 in main ../../../dix/stubmain.c:34
(/lib/x86_64-linux-gnu/libc.so.6+0x28564)
previously allocated by thread T0 here:
(/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10ddc6)
#1 0x55c450e1c57b in AddInputDevice ../../../dix/devices.c:259
#2 0x55c450e34840 in AllocDevicePair ../../../dix/devices.c:2755
#3 0x55c45130318f in add_master ../../../Xi/xichangehierarchy.c:152
../../../Xi/xichangehierarchy.c:465
#5 0x55c4512cb9f5 in ProcIDispatch ../../../Xi/extinit.c:390
#6 0x55c450e6a92b in Dispatch ../../../dix/dispatch.c:551
#7 0x55c450e834b7 in dix_main ../../../dix/main.c:272
#8 0x55c4517a8d93 in main ../../../dix/stubmain.c:34
(/lib/x86_64-linux-gnu/libc.so.6+0x28564)
The problem is caused by dev->master being not reset when disabling the
device, which then causes dangling pointer when the master device itself
is being deleted when exiting whole server.
Note that RecalculateMasterButtons() requires dev->master to be still
valid, so we can reset it only at the end of function.
Signed-off-by: Povilas Kanapickas <povilas@radix.lt>
---
dix/devices.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/dix/devices.c b/dix/devices.c
index e62c34c55..5f9ce1678 100644
--- a/dix/devices.c
+++ b/dix/devices.c
@@ -520,6 +520,7 @@ DisableDevice(DeviceIntPtr dev, BOOL sendevent)
}
RecalculateMasterButtons(dev);
+ dev->master = NULL;
return TRUE;
}
--
2.43.0

View File

@ -0,0 +1,51 @@
From 9e2ecb2af8302dedc49cb6a63ebe063c58a9e7e3 Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutterer@who-t.net>
Date: Thu, 14 Dec 2023 11:29:49 +1000
Subject: [PATCH 1/9] dix: allocate enough space for logical button maps
Both DeviceFocusEvent and the XIQueryPointer reply contain a bit for
each logical button currently down. Since buttons can be arbitrarily mapped
to anything up to 255 make sure we have enough bits for the maximum mapping.
CVE-2023-6816, ZDI-CAN-22664, ZDI-CAN-22665
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
---
Xi/xiquerypointer.c | 3 +--
dix/enterleave.c | 5 +++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Xi/xiquerypointer.c b/Xi/xiquerypointer.c
index 5b77b1a44..2b05ac5f3 100644
--- a/Xi/xiquerypointer.c
+++ b/Xi/xiquerypointer.c
@@ -149,8 +149,7 @@ ProcXIQueryPointer(ClientPtr client)
if (pDev->button) {
int i;
- rep.buttons_len =
- bytes_to_int32(bits_to_bytes(pDev->button->numButtons));
+ rep.buttons_len = bytes_to_int32(bits_to_bytes(256)); /* button map up to 255 */
rep.length += rep.buttons_len;
buttons = calloc(rep.buttons_len, 4);
if (!buttons)
diff --git a/dix/enterleave.c b/dix/enterleave.c
index 867ec7436..ded8679d7 100644
--- a/dix/enterleave.c
+++ b/dix/enterleave.c
@@ -784,8 +784,9 @@ DeviceFocusEvent(DeviceIntPtr dev, int type, int mode, int detail,
mouse = IsFloating(dev) ? dev : GetMaster(dev, MASTER_POINTER);
- /* XI 2 event */
- btlen = (mouse->button) ? bits_to_bytes(mouse->button->numButtons) : 0;
+ /* XI 2 event contains the logical button map - maps are CARD8
+ * so we need 256 bits for the possibly maximum mapping */
+ btlen = (mouse->button) ? bits_to_bytes(256) : 0;
btlen = bytes_to_int32(btlen);
len = sizeof(xXIFocusInEvent) + btlen * 4;
--
2.43.0

View File

@ -0,0 +1,153 @@
From 454b3a826edb5fc6d0fea3a9cfd1a5e8fc568747 Mon Sep 17 00:00:00 2001
From: Adam Jackson <ajax@redhat.com>
Date: Mon, 22 Jul 2019 13:51:06 -0400
Subject: [PATCH] hw: Rename boolean config value field from bool to boolean
"bool" conflicts with C++ (meh) and stdbool.h (ngh alright fine). This
is a driver-visible change and will likely break the build for mach64,
but it can be fixed by simply using xf86ReturnOptValBool like every
other driver.
Signed-off-by: Adam Jackson <ajax@redhat.com>
---
hw/xfree86/common/xf86Opt.h | 2 +-
hw/xfree86/common/xf86Option.c | 10 +++++-----
hw/xwin/winconfig.c | 22 +++++++++++-----------
hw/xwin/winconfig.h | 2 +-
4 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/hw/xfree86/common/xf86Opt.h b/hw/xfree86/common/xf86Opt.h
index 3be2a0fc7..3046fbd41 100644
--- a/hw/xfree86/common/xf86Opt.h
+++ b/hw/xfree86/common/xf86Opt.h
@@ -41,7 +41,7 @@ typedef union {
unsigned long num;
const char *str;
double realnum;
- Bool bool;
+ Bool boolean;
OptFrequency freq;
} ValueUnion;
diff --git a/hw/xfree86/common/xf86Option.c b/hw/xfree86/common/xf86Option.c
index 06973bca3..ca538cc57 100644
--- a/hw/xfree86/common/xf86Option.c
+++ b/hw/xfree86/common/xf86Option.c
@@ -213,7 +213,7 @@ LookupBoolOption(XF86OptionPtr optlist, const char *name, int deflt,
o.name = name;
o.type = OPTV_BOOLEAN;
if (ParseOptionValue(-1, optlist, &o, markUsed))
- deflt = o.value.bool;
+ deflt = o.value.boolean;
return deflt;
}
@@ -474,7 +474,7 @@ xf86ShowUnusedOptions(int scrnIndex, XF86OptionPtr opt)
static Bool
GetBoolValue(OptionInfoPtr p, const char *s)
{
- return xf86getBoolValue(&p->value.bool, s);
+ return xf86getBoolValue(&p->value.boolean, s);
}
static Bool
@@ -678,7 +678,7 @@ ParseOptionValue(int scrnIndex, XF86OptionPtr options, OptionInfoPtr p,
if (markUsed)
xf86MarkOptionUsedByName(options, newn);
if (GetBoolValue(&opt, s)) {
- p->value.bool = !opt.value.bool;
+ p->value.boolean = !opt.value.boolean;
p->found = TRUE;
}
else {
@@ -869,7 +869,7 @@ xf86GetOptValBool(const OptionInfoRec * table, int token, Bool *value)
p = xf86TokenToOptinfo(table, token);
if (p && p->found) {
- *value = p->value.bool;
+ *value = p->value.boolean;
return TRUE;
}
else
@@ -883,7 +883,7 @@ xf86ReturnOptValBool(const OptionInfoRec * table, int token, Bool def)
p = xf86TokenToOptinfo(table, token);
if (p && p->found) {
- return p->value.bool;
+ return p->value.boolean;
}
else
return def;
diff --git a/hw/xwin/winconfig.c b/hw/xwin/winconfig.c
index 31894d2fb..646d69006 100644
--- a/hw/xwin/winconfig.c
+++ b/hw/xwin/winconfig.c
@@ -623,7 +623,7 @@ winSetBoolOption(void *optlist, const char *name, int deflt)
o.name = name;
o.type = OPTV_BOOLEAN;
if (ParseOptionValue(-1, optlist, &o))
- deflt = o.value.bool;
+ deflt = o.value.boolean;
return deflt;
}
@@ -918,7 +918,7 @@ ParseOptionValue(int scrnIndex, void *options, OptionInfoPtr p)
}
if ((s = winFindOptionValue(options, newn)) != NULL) {
if (GetBoolValue(&opt, s)) {
- p->value.bool = !opt.value.bool;
+ p->value.boolean = !opt.value.boolean;
p->found = TRUE;
}
else {
@@ -968,25 +968,25 @@ static Bool
GetBoolValue(OptionInfoPtr p, const char *s)
{
if (*s == 0) {
- p->value.bool = TRUE;
+ p->value.boolean = TRUE;
}
else {
if (winNameCompare(s, "1") == 0)
- p->value.bool = TRUE;
+ p->value.boolean = TRUE;
else if (winNameCompare(s, "on") == 0)
- p->value.bool = TRUE;
+ p->value.boolean = TRUE;
else if (winNameCompare(s, "true") == 0)
- p->value.bool = TRUE;
+ p->value.boolean = TRUE;
else if (winNameCompare(s, "yes") == 0)
- p->value.bool = TRUE;
+ p->value.boolean = TRUE;
else if (winNameCompare(s, "0") == 0)
- p->value.bool = FALSE;
+ p->value.boolean = FALSE;
else if (winNameCompare(s, "off") == 0)
- p->value.bool = FALSE;
+ p->value.boolean = FALSE;
else if (winNameCompare(s, "false") == 0)
- p->value.bool = FALSE;
+ p->value.boolean = FALSE;
else if (winNameCompare(s, "no") == 0)
- p->value.bool = FALSE;
+ p->value.boolean = FALSE;
}
return TRUE;
}
diff --git a/hw/xwin/winconfig.h b/hw/xwin/winconfig.h
index f079368c7..bd1f59650 100644
--- a/hw/xwin/winconfig.h
+++ b/hw/xwin/winconfig.h
@@ -199,7 +199,7 @@ typedef union {
unsigned long num;
char *str;
double realnum;
- Bool bool;
+ Bool boolean;
OptFrequency freq;
} ValueUnion;
--
2.43.0

View File

@ -0,0 +1,84 @@
From ece23be888a93b741aa1209d1dbf64636109d6a5 Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutterer@who-t.net>
Date: Mon, 18 Dec 2023 14:27:50 +1000
Subject: [PATCH 2/9] dix: Allocate sufficient xEvents for our
DeviceStateNotify
If a device has both a button class and a key class and numButtons is
zero, we can get an OOB write due to event under-allocation.
This function seems to assume a device has either keys or buttons, not
both. It has two virtually identical code paths, both of which assume
they're applying to the first event in the sequence.
A device with both a key and button class triggered a logic bug - only
one xEvent was allocated but the deviceStateNotify pointer was pushed on
once per type. So effectively this logic code:
int count = 1;
if (button && nbuttons > 32) count++;
if (key && nbuttons > 0) count++;
if (key && nkeys > 32) count++; // this is basically always true
// count is at 2 for our keys + zero button device
ev = alloc(count * sizeof(xEvent));
FixDeviceStateNotify(ev);
if (button)
FixDeviceStateNotify(ev++);
if (key)
FixDeviceStateNotify(ev++); // santa drops into the wrong chimney here
If the device has more than 3 valuators, the OOB is pushed back - we're
off by one so it will happen when the last deviceValuator event is
written instead.
Fix this by allocating the maximum number of events we may allocate.
Note that the current behavior is not protocol-correct anyway, this
patch fixes only the allocation issue.
Note that this issue does not trigger if the device has at least one
button. While the server does not prevent a button class with zero
buttons, it is very unlikely.
CVE-2024-0229, ZDI-CAN-22678
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
---
dix/enterleave.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/dix/enterleave.c b/dix/enterleave.c
index ded8679d7..17964b00a 100644
--- a/dix/enterleave.c
+++ b/dix/enterleave.c
@@ -675,7 +675,8 @@ static void
DeliverStateNotifyEvent(DeviceIntPtr dev, WindowPtr win)
{
int evcount = 1;
- deviceStateNotify *ev, *sev;
+ deviceStateNotify sev[6 + (MAX_VALUATORS + 2)/3];
+ deviceStateNotify *ev;
deviceKeyStateNotify *kev;
deviceButtonStateNotify *bev;
@@ -714,7 +715,7 @@ DeliverStateNotifyEvent(DeviceIntPtr dev, WindowPtr win)
}
}
- sev = ev = xallocarray(evcount, sizeof(xEvent));
+ ev = sev;
FixDeviceStateNotify(dev, ev, NULL, NULL, NULL, first);
if (b != NULL) {
@@ -770,7 +771,6 @@ DeliverStateNotifyEvent(DeviceIntPtr dev, WindowPtr win)
DeliverEventsToWindow(dev, win, (xEvent *) sev, evcount,
DeviceStateNotifyMask, NullGrab);
- free(sev);
}
void
--
2.43.0

View File

@ -0,0 +1,217 @@
From 219c54b8a3337456ce5270ded6a67bcde53553d5 Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutterer@who-t.net>
Date: Mon, 18 Dec 2023 12:26:20 +1000
Subject: [PATCH 3/9] dix: fix DeviceStateNotify event calculation
The previous code only made sense if one considers buttons and keys to
be mutually exclusive on a device. That is not necessarily true, causing
a number of issues.
This function allocates and fills in the number of xEvents we need to
send the device state down the wire. This is split across multiple
32-byte devices including one deviceStateNotify event and optional
deviceKeyStateNotify, deviceButtonStateNotify and (possibly multiple)
deviceValuator events.
The previous behavior would instead compose a sequence
of [state, buttonstate, state, keystate, valuator...]. This is not
protocol correct, and on top of that made the code extremely convoluted.
Fix this by streamlining: add both button and key into the deviceStateNotify
and then append the key state and button state, followed by the
valuators. Finally, the deviceValuator events contain up to 6 valuators
per event but we only ever sent through 3 at a time. Let's double that
troughput.
CVE-2024-0229, ZDI-CAN-22678
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
---
dix/enterleave.c | 121 ++++++++++++++++++++---------------------------
1 file changed, 52 insertions(+), 69 deletions(-)
diff --git a/dix/enterleave.c b/dix/enterleave.c
index 17964b00a..7b7ba1098 100644
--- a/dix/enterleave.c
+++ b/dix/enterleave.c
@@ -615,9 +615,15 @@ FixDeviceValuator(DeviceIntPtr dev, deviceValuator * ev, ValuatorClassPtr v,
ev->type = DeviceValuator;
ev->deviceid = dev->id;
- ev->num_valuators = nval < 3 ? nval : 3;
+ ev->num_valuators = nval < 6 ? nval : 6;
ev->first_valuator = first;
switch (ev->num_valuators) {
+ case 6:
+ ev->valuator2 = v->axisVal[first + 5];
+ case 5:
+ ev->valuator2 = v->axisVal[first + 4];
+ case 4:
+ ev->valuator2 = v->axisVal[first + 3];
case 3:
ev->valuator2 = v->axisVal[first + 2];
case 2:
@@ -626,7 +632,6 @@ FixDeviceValuator(DeviceIntPtr dev, deviceValuator * ev, ValuatorClassPtr v,
ev->valuator0 = v->axisVal[first];
break;
}
- first += ev->num_valuators;
}
static void
@@ -646,7 +651,7 @@ FixDeviceStateNotify(DeviceIntPtr dev, deviceStateNotify * ev, KeyClassPtr k,
ev->num_buttons = b->numButtons;
memcpy((char *) ev->buttons, (char *) b->down, 4);
}
- else if (k) {
+ if (k) {
ev->classes_reported |= (1 << KeyClass);
ev->num_keys = k->xkbInfo->desc->max_key_code -
k->xkbInfo->desc->min_key_code;
@@ -670,15 +675,26 @@ FixDeviceStateNotify(DeviceIntPtr dev, deviceStateNotify * ev, KeyClassPtr k,
}
}
-
+/**
+ * The device state notify event is split across multiple 32-byte events.
+ * The first one contains the first 32 button state bits, the first 32
+ * key state bits, and the first 3 valuator values.
+ *
+ * If a device has more than that, the server sends out:
+ * - one deviceButtonStateNotify for buttons 32 and above
+ * - one deviceKeyStateNotify for keys 32 and above
+ * - one deviceValuator event per 6 valuators above valuator 4
+ *
+ * All events but the last one have the deviceid binary ORed with MORE_EVENTS,
+ */
static void
DeliverStateNotifyEvent(DeviceIntPtr dev, WindowPtr win)
{
+ /* deviceStateNotify, deviceKeyStateNotify, deviceButtonStateNotify
+ * and one deviceValuator for each 6 valuators */
+ deviceStateNotify sev[3 + (MAX_VALUATORS + 6)/6];
int evcount = 1;
- deviceStateNotify sev[6 + (MAX_VALUATORS + 2)/3];
- deviceStateNotify *ev;
- deviceKeyStateNotify *kev;
- deviceButtonStateNotify *bev;
+ deviceStateNotify *ev = sev;
KeyClassPtr k;
ButtonClassPtr b;
@@ -691,82 +707,49 @@ DeliverStateNotifyEvent(DeviceIntPtr dev, WindowPtr win)
if ((b = dev->button) != NULL) {
nbuttons = b->numButtons;
- if (nbuttons > 32)
+ if (nbuttons > 32) /* first 32 are encoded in deviceStateNotify */
evcount++;
}
if ((k = dev->key) != NULL) {
nkeys = k->xkbInfo->desc->max_key_code - k->xkbInfo->desc->min_key_code;
- if (nkeys > 32)
+ if (nkeys > 32) /* first 32 are encoded in deviceStateNotify */
evcount++;
- if (nbuttons > 0) {
- evcount++;
- }
}
if ((v = dev->valuator) != NULL) {
nval = v->numAxes;
-
- if (nval > 3)
- evcount++;
- if (nval > 6) {
- if (!(k && b))
- evcount++;
- if (nval > 9)
- evcount += ((nval - 7) / 3);
- }
+ /* first three are encoded in deviceStateNotify, then
+ * it's 6 per deviceValuator event */
+ evcount += ((nval - 3) + 6)/6;
}
- ev = sev;
- FixDeviceStateNotify(dev, ev, NULL, NULL, NULL, first);
-
- if (b != NULL) {
- FixDeviceStateNotify(dev, ev++, NULL, b, v, first);
- first += 3;
- nval -= 3;
- if (nbuttons > 32) {
- (ev - 1)->deviceid |= MORE_EVENTS;
- bev = (deviceButtonStateNotify *) ev++;
- bev->type = DeviceButtonStateNotify;
- bev->deviceid = dev->id;
- memcpy((char *) &bev->buttons[4], (char *) &b->down[4],
- DOWN_LENGTH - 4);
- }
- if (nval > 0) {
- (ev - 1)->deviceid |= MORE_EVENTS;
- FixDeviceValuator(dev, (deviceValuator *) ev++, v, first);
- first += 3;
- nval -= 3;
- }
+ BUG_RETURN(evcount <= ARRAY_SIZE(sev));
+
+ FixDeviceStateNotify(dev, ev, k, b, v, first);
+
+ if (b != NULL && nbuttons > 32) {
+ deviceButtonStateNotify *bev = (deviceButtonStateNotify *) ++ev;
+ (ev - 1)->deviceid |= MORE_EVENTS;
+ bev->type = DeviceButtonStateNotify;
+ bev->deviceid = dev->id;
+ memcpy((char *) &bev->buttons[4], (char *) &b->down[4],
+ DOWN_LENGTH - 4);
}
- if (k != NULL) {
- FixDeviceStateNotify(dev, ev++, k, NULL, v, first);
- first += 3;
- nval -= 3;
- if (nkeys > 32) {
- (ev - 1)->deviceid |= MORE_EVENTS;
- kev = (deviceKeyStateNotify *) ev++;
- kev->type = DeviceKeyStateNotify;
- kev->deviceid = dev->id;
- memmove((char *) &kev->keys[0], (char *) &k->down[4], 28);
- }
- if (nval > 0) {
- (ev - 1)->deviceid |= MORE_EVENTS;
- FixDeviceValuator(dev, (deviceValuator *) ev++, v, first);
- first += 3;
- nval -= 3;
- }
+ if (k != NULL && nkeys > 32) {
+ deviceKeyStateNotify *kev = (deviceKeyStateNotify *) ++ev;
+ (ev - 1)->deviceid |= MORE_EVENTS;
+ kev->type = DeviceKeyStateNotify;
+ kev->deviceid = dev->id;
+ memmove((char *) &kev->keys[0], (char *) &k->down[4], 28);
}
+ first = 3;
+ nval -= 3;
while (nval > 0) {
- FixDeviceStateNotify(dev, ev++, NULL, NULL, v, first);
- first += 3;
- nval -= 3;
- if (nval > 0) {
- (ev - 1)->deviceid |= MORE_EVENTS;
- FixDeviceValuator(dev, (deviceValuator *) ev++, v, first);
- first += 3;
- nval -= 3;
- }
+ ev->deviceid |= MORE_EVENTS;
+ FixDeviceValuator(dev, (deviceValuator *) ++ev, v, first);
+ first += 6;
+ nval -= 6;
}
DeliverEventsToWindow(dev, win, (xEvent *) sev, evcount,
--
2.43.0

View File

@ -0,0 +1,37 @@
From df3c65706eb169d5938df0052059f3e0d5981b74 Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutterer@who-t.net>
Date: Thu, 21 Dec 2023 13:48:10 +1000
Subject: [PATCH 4/9] Xi: when creating a new ButtonClass, set the number of
buttons
There's a racy sequence where a master device may copy the button class
from the slave, without ever initializing numButtons. This leads to a
device with zero buttons but a button class which is invalid.
Let's copy the numButtons value from the source - by definition if we
don't have a button class yet we do not have any other slave devices
with more than this number of buttons anyway.
CVE-2024-0229, ZDI-CAN-22678
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
---
Xi/exevents.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/Xi/exevents.c b/Xi/exevents.c
index 54ea11a93..e16171468 100644
--- a/Xi/exevents.c
+++ b/Xi/exevents.c
@@ -605,6 +605,7 @@ DeepCopyPointerClasses(DeviceIntPtr from, DeviceIntPtr to)
to->button = calloc(1, sizeof(ButtonClassRec));
if (!to->button)
FatalError("[Xi] no memory for class shift.\n");
+ to->button->numButtons = from->button->numButtons;
}
else
classes->button = NULL;
--
2.43.0

View File

@ -0,0 +1,109 @@
From 4a5e9b1895627d40d26045bd0b7ef3dce503cbd1 Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutterer@who-t.net>
Date: Thu, 4 Jan 2024 10:01:24 +1000
Subject: [PATCH 5/9] Xi: flush hierarchy events after adding/removing master
devices
The `XISendDeviceHierarchyEvent()` function allocates space to store up
to `MAXDEVICES` (256) `xXIHierarchyInfo` structures in `info`.
If a device with a given ID was removed and a new device with the same
ID added both in the same operation, the single device ID will lead to
two info structures being written to `info`.
Since this case can occur for every device ID at once, a total of two
times `MAXDEVICES` info structures might be written to the allocation.
To avoid it, once one add/remove master is processed, send out the
device hierarchy event for the current state and continue. That event
thus only ever has exactly one of either added/removed in it (and
optionally slave attached/detached).
CVE-2024-21885, ZDI-CAN-22744
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
---
Xi/xichangehierarchy.c | 27 ++++++++++++++++++++++-----
1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/Xi/xichangehierarchy.c b/Xi/xichangehierarchy.c
index d2d985848..72d00451e 100644
--- a/Xi/xichangehierarchy.c
+++ b/Xi/xichangehierarchy.c
@@ -416,6 +416,11 @@ ProcXIChangeHierarchy(ClientPtr client)
size_t len; /* length of data remaining in request */
int rc = Success;
int flags[MAXDEVICES] = { 0 };
+ enum {
+ NO_CHANGE,
+ FLUSH,
+ CHANGED,
+ } changes = NO_CHANGE;
REQUEST(xXIChangeHierarchyReq);
REQUEST_AT_LEAST_SIZE(xXIChangeHierarchyReq);
@@ -465,8 +470,9 @@ ProcXIChangeHierarchy(ClientPtr client)
rc = add_master(client, c, flags);
if (rc != Success)
goto unwind;
- }
+ changes = FLUSH;
break;
+ }
case XIRemoveMaster:
{
xXIRemoveMasterInfo *r = (xXIRemoveMasterInfo *) any;
@@ -475,8 +481,9 @@ ProcXIChangeHierarchy(ClientPtr client)
rc = remove_master(client, r, flags);
if (rc != Success)
goto unwind;
- }
+ changes = FLUSH;
break;
+ }
case XIDetachSlave:
{
xXIDetachSlaveInfo *c = (xXIDetachSlaveInfo *) any;
@@ -485,8 +492,9 @@ ProcXIChangeHierarchy(ClientPtr client)
rc = detach_slave(client, c, flags);
if (rc != Success)
goto unwind;
- }
+ changes = CHANGED;
break;
+ }
case XIAttachSlave:
{
xXIAttachSlaveInfo *c = (xXIAttachSlaveInfo *) any;
@@ -495,16 +503,25 @@ ProcXIChangeHierarchy(ClientPtr client)
rc = attach_slave(client, c, flags);
if (rc != Success)
goto unwind;
+ changes = CHANGED;
+ break;
}
+ default:
break;
}
+ if (changes == FLUSH) {
+ XISendDeviceHierarchyEvent(flags);
+ memset(flags, 0, sizeof(flags));
+ changes = NO_CHANGE;
+ }
+
len -= any->length * 4;
any = (xXIAnyHierarchyChangeInfo *) ((char *) any + any->length * 4);
}
unwind:
-
- XISendDeviceHierarchyEvent(flags);
+ if (changes != NO_CHANGE)
+ XISendDeviceHierarchyEvent(flags);
return rc;
}
--
2.43.0

View File

@ -0,0 +1,70 @@
From bc1fdbe46559dd947674375946bbef54dd0ce36b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Exp=C3=B3sito?= <jexposit@redhat.com>
Date: Fri, 22 Dec 2023 18:28:31 +0100
Subject: [PATCH 6/9] Xi: do not keep linked list pointer during recursion
The `DisableDevice()` function is called whenever an enabled device
is disabled and it moves the device from the `inputInfo.devices` linked
list to the `inputInfo.off_devices` linked list.
However, its link/unlink operation has an issue during the recursive
call to `DisableDevice()` due to the `prev` pointer pointing to a
removed device.
This issue leads to a length mismatch between the total number of
devices and the number of device in the list, leading to a heap
overflow and, possibly, to local privilege escalation.
Simplify the code that checked whether the device passed to
`DisableDevice()` was in `inputInfo.devices` or not and find the
previous device after the recursion.
CVE-2024-21886, ZDI-CAN-22840
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
---
dix/devices.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/dix/devices.c b/dix/devices.c
index dca98c8d1..389d28a23 100644
--- a/dix/devices.c
+++ b/dix/devices.c
@@ -453,14 +453,20 @@ DisableDevice(DeviceIntPtr dev, BOOL sendevent)
{
DeviceIntPtr *prev, other;
BOOL enabled;
+ BOOL dev_in_devices_list = FALSE;
int flags[MAXDEVICES] = { 0 };
if (!dev->enabled)
return TRUE;
- for (prev = &inputInfo.devices;
- *prev && (*prev != dev); prev = &(*prev)->next);
- if (*prev != dev)
+ for (other = inputInfo.devices; other; other = other->next) {
+ if (other == dev) {
+ dev_in_devices_list = TRUE;
+ break;
+ }
+ }
+
+ if (!dev_in_devices_list)
return FALSE;
TouchEndPhysicallyActiveTouches(dev);
@@ -511,6 +517,9 @@ DisableDevice(DeviceIntPtr dev, BOOL sendevent)
LeaveWindow(dev);
SetFocusOut(dev);
+ for (prev = &inputInfo.devices;
+ *prev && (*prev != dev); prev = &(*prev)->next);
+
*prev = dev->next;
dev->next = inputInfo.off_devices;
inputInfo.off_devices = dev;
--
2.43.0

View File

@ -0,0 +1,53 @@
From 26769aa71fcbe0a8403b7fb13b7c9010cc07c3a8 Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutterer@who-t.net>
Date: Fri, 5 Jan 2024 09:40:27 +1000
Subject: [PATCH 7/9] dix: when disabling a master, float disabled slaved
devices too
Disabling a master device floats all slave devices but we didn't do this
to already-disabled slave devices. As a result those devices kept their
reference to the master device resulting in access to already freed
memory if the master device was removed before the corresponding slave
device.
And to match this behavior, also forcibly reset that pointer during
CloseDownDevices().
Related to CVE-2024-21886, ZDI-CAN-22840
---
dix/devices.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/dix/devices.c b/dix/devices.c
index 389d28a23..84a6406d1 100644
--- a/dix/devices.c
+++ b/dix/devices.c
@@ -483,6 +483,13 @@ DisableDevice(DeviceIntPtr dev, BOOL sendevent)
flags[other->id] |= XISlaveDetached;
}
}
+
+ for (other = inputInfo.off_devices; other; other = other->next) {
+ if (!IsMaster(other) && GetMaster(other, MASTER_ATTACHED) == dev) {
+ AttachDevice(NULL, other, NULL);
+ flags[other->id] |= XISlaveDetached;
+ }
+ }
}
else {
for (other = inputInfo.devices; other; other = other->next) {
@@ -1088,6 +1095,11 @@ CloseDownDevices(void)
dev->master = NULL;
}
+ for (dev = inputInfo.off_devices; dev; dev = dev->next) {
+ if (!IsMaster(dev) && !IsFloating(dev))
+ dev->master = NULL;
+ }
+
CloseDeviceList(&inputInfo.devices);
CloseDeviceList(&inputInfo.off_devices);
--
2.43.0

View File

@ -0,0 +1,60 @@
From e5e8586a12a3ec915673edffa10dc8fe5e15dac3 Mon Sep 17 00:00:00 2001
From: Olivier Fourdan <ofourdan@redhat.com>
Date: Wed, 6 Dec 2023 12:09:41 +0100
Subject: [PATCH 8/9] glx: Call XACE hooks on the GLX buffer
The XSELINUX code will label resources at creation by checking the
access mode. When the access mode is DixCreateAccess, it will call the
function to label the new resource SELinuxLabelResource().
However, GLX buffers do not go through the XACE hooks when created,
hence leaving the resource actually unlabeled.
When, later, the client tries to create another resource using that
drawable (like a GC for example), the XSELINUX code would try to use
the security ID of that object which has never been labeled, get a NULL
pointer and crash when checking whether the requested permissions are
granted for subject security ID.
To avoid the issue, make sure to call the XACE hooks when creating the
GLX buffers.
Credit goes to Donn Seeley <donn@xmission.com> for providing the patch.
CVE-2024-0408
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
---
glx/glxcmds.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/glx/glxcmds.c b/glx/glxcmds.c
index fc26a2e34..1e46d0c72 100644
--- a/glx/glxcmds.c
+++ b/glx/glxcmds.c
@@ -48,6 +48,7 @@
#include "indirect_util.h"
#include "protocol-versions.h"
#include "glxvndabi.h"
+#include "xace.h"
static char GLXServerVendorName[] = "SGI";
@@ -1392,6 +1393,13 @@ DoCreatePbuffer(ClientPtr client, int screenNum, XID fbconfigId,
if (!pPixmap)
return BadAlloc;
+ err = XaceHook(XACE_RESOURCE_ACCESS, client, glxDrawableId, RT_PIXMAP,
+ pPixmap, RT_NONE, NULL, DixCreateAccess);
+ if (err != Success) {
+ (*pGlxScreen->pScreen->DestroyPixmap) (pPixmap);
+ return err;
+ }
+
/* Assign the pixmap the same id as the pbuffer and add it as a
* resource so it and the DRI2 drawable will be reclaimed when the
* pbuffer is destroyed. */
--
2.43.0

View File

@ -0,0 +1,56 @@
From 2ef0f1116c65d5cb06d7b6d83f8a1aea702c94f7 Mon Sep 17 00:00:00 2001
From: Olivier Fourdan <ofourdan@redhat.com>
Date: Wed, 6 Dec 2023 11:51:56 +0100
Subject: [PATCH 9/9] ephyr,xwayland: Use the proper private key for cursor
The cursor in DIX is actually split in two parts, the cursor itself and
the cursor bits, each with their own devPrivates.
The cursor itself includes the cursor bits, meaning that the cursor bits
devPrivates in within structure of the cursor.
Both Xephyr and Xwayland were using the private key for the cursor bits
to store the data for the cursor, and when using XSELINUX which comes
with its own special devPrivates, the data stored in that cursor bits'
devPrivates would interfere with the XSELINUX devPrivates data and the
SELINUX security ID would point to some other unrelated data, causing a
crash in the XSELINUX code when trying to (re)use the security ID.
CVE-2024-0409
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
---
hw/kdrive/ephyr/ephyrcursor.c | 2 +-
hw/xwayland/xwayland-cursor.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/kdrive/ephyr/ephyrcursor.c b/hw/kdrive/ephyr/ephyrcursor.c
index f991899c5..3f192d034 100644
--- a/hw/kdrive/ephyr/ephyrcursor.c
+++ b/hw/kdrive/ephyr/ephyrcursor.c
@@ -246,7 +246,7 @@ miPointerSpriteFuncRec EphyrPointerSpriteFuncs = {
Bool
ephyrCursorInit(ScreenPtr screen)
{
- if (!dixRegisterPrivateKey(&ephyrCursorPrivateKey, PRIVATE_CURSOR_BITS,
+ if (!dixRegisterPrivateKey(&ephyrCursorPrivateKey, PRIVATE_CURSOR,
sizeof(ephyrCursorRec)))
return FALSE;
diff --git a/hw/xwayland/xwayland-cursor.c b/hw/xwayland/xwayland-cursor.c
index e3c1aaa50..bd94b0cfb 100644
--- a/hw/xwayland/xwayland-cursor.c
+++ b/hw/xwayland/xwayland-cursor.c
@@ -431,7 +431,7 @@ static miPointerScreenFuncRec xwl_pointer_screen_funcs = {
Bool
xwl_screen_init_cursor(struct xwl_screen *xwl_screen)
{
- if (!dixRegisterPrivateKey(&xwl_cursor_private_key, PRIVATE_CURSOR_BITS, 0))
+ if (!dixRegisterPrivateKey(&xwl_cursor_private_key, PRIVATE_CURSOR, 0))
return FALSE;
return miPointerInitialize(xwl_screen->screen,
--
2.43.0

View File

@ -42,7 +42,7 @@
Summary: X.Org X11 X server
Name: xorg-x11-server
Version: 1.20.11
Release: 22%{?gitdate:.%{gitdate}}%{?dist}
Release: 24%{?gitdate:.%{gitdate}}%{?dist}
URL: http://www.x.org
License: MIT
@ -165,6 +165,25 @@ Patch10029: 0002-mi-reset-the-PointerWindows-reference-on-screen-swit.patch
Patch10030: 0001-Xi-allocate-enough-XkbActions-for-our-buttons.patch
# CVE-2023-6478
Patch10031: 0001-randr-avoid-integer-truncation-in-length-check-of-Pr.patch
# CVE-2023-6816
Patch10032: 0001-dix-allocate-enough-space-for-logical-button-maps.patch
# CVE-2024-0229
Patch10033: 0002-dix-Allocate-sufficient-xEvents-for-our-DeviceStateN.patch
Patch10034: 0003-dix-fix-DeviceStateNotify-event-calculation.patch
Patch10035: 0004-Xi-when-creating-a-new-ButtonClass-set-the-number-of.patch
# CVE-2024-21885
Patch10036: 0005-Xi-flush-hierarchy-events-after-adding-removing-mast.patch
# CVE-2024-21886
Patch10037: 0006-Xi-do-not-keep-linked-list-pointer-during-recursion.patch
Patch10038: 0007-dix-when-disabling-a-master-float-disabled-slaved-de.patch
# CVE-2024-0408
Patch10039: 0008-glx-Call-XACE-hooks-on-the-GLX-buffer.patch
# CVE-2024-0409
Patch10040: 0009-ephyr-xwayland-Use-the-proper-private-key-for-cursor.patch
# Fix compilation error
Patch10041: 0001-hw-Rename-boolean-config-value-field-from-bool-to-bo.patch
# Related to CVE-2024-21886
Patch10042: 0001-dix-Fix-use-after-free-in-input-device-shutdown.patch
BuildRequires: make
BuildRequires: systemtap-sdt-devel
@ -574,6 +593,19 @@ find %{inst_srcdir}/hw/xfree86 -name \*.c -delete
%changelog
* Thu Jan 18 2024 José Expósito <jexposit@redhat.com> - 1.20.4-24
- Fix use after free related to CVE-2024-21886
* Tue Jan 16 2024 José Expósito <jexposit@redhat.com> - 1.20.11-23
- CVE fix for: CVE-2023-6816, CVE-2024-0229, CVE-2024-21885, CVE-2024-21886,
CVE-2024-0408 and CVE-2024-0409
Resolves: https://issues.redhat.com/browse/RHEL-21203
Resolves: https://issues.redhat.com/browse/RHEL-20531
Resolves: https://issues.redhat.com/browse/RHEL-20380
Resolves: https://issues.redhat.com/browse/RHEL-20386
Resolves: https://issues.redhat.com/browse/RHEL-21193
Resolves: https://issues.redhat.com/browse/RHEL-21200
* Thu Dec 14 2023 José Expósito <jexposit@redhat.com> - 1.20.11-22
- CVE fix for: CVE-2023-6377, CVE-2023-6478
Resolves: https://issues.redhat.com/browse/RHEL-18322