Skip to content

Commit 795266d

Browse files
CBL-Mariner-BotSumynwa
CBL-Mariner-Bot
and
Sumynwa
authored
[AUTO-CHERRYPICK] Patch xorg-x11-server for CVE-2024-0229, CVE-2024-0409 & CVE-2024-21886 - branch main (#10496)
Co-authored-by: Sumedh Alok Sharma <sumsharma@microsoft.com>
1 parent 65a3f01 commit 795266d

4 files changed

Lines changed: 527 additions & 14 deletions

File tree

‎SPECS/xorg-x11-server/CVE-2024-0229.patch‎

Lines changed: 331 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,331 @@
1+
From ece23be888a93b741aa1209d1dbf64636109d6a5 Mon Sep 17 00:00:00 2001
2+
From: Peter Hutterer <peter.hutterer@who-t.net>
3+
Date: Mon, 18 Dec 2023 14:27:50 +1000
4+
Subject: [PATCH] dix: Allocate sufficient xEvents for our DeviceStateNotify
5+
6+
If a device has both a button class and a key class and numButtons is
7+
zero, we can get an OOB write due to event under-allocation.
8+
9+
This function seems to assume a device has either keys or buttons, not
10+
both. It has two virtually identical code paths, both of which assume
11+
they're applying to the first event in the sequence.
12+
13+
A device with both a key and button class triggered a logic bug - only
14+
one xEvent was allocated but the deviceStateNotify pointer was pushed on
15+
once per type. So effectively this logic code:
16+
17+
int count = 1;
18+
if (button && nbuttons > 32) count++;
19+
if (key && nbuttons > 0) count++;
20+
if (key && nkeys > 32) count++; // this is basically always true
21+
// count is at 2 for our keys + zero button device
22+
23+
ev = alloc(count * sizeof(xEvent));
24+
FixDeviceStateNotify(ev);
25+
if (button)
26+
FixDeviceStateNotify(ev++);
27+
if (key)
28+
FixDeviceStateNotify(ev++); // santa drops into the wrong chimney here
29+
30+
If the device has more than 3 valuators, the OOB is pushed back - we're
31+
off by one so it will happen when the last deviceValuator event is
32+
written instead.
33+
34+
Fix this by allocating the maximum number of events we may allocate.
35+
Note that the current behavior is not protocol-correct anyway, this
36+
patch fixes only the allocation issue.
37+
38+
Note that this issue does not trigger if the device has at least one
39+
button. While the server does not prevent a button class with zero
40+
buttons, it is very unlikely.
41+
42+
CVE-2024-0229, ZDI-CAN-22678
43+
44+
This vulnerability was discovered by:
45+
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
46+
---
47+
dix/enterleave.c | 6 +++---
48+
1 file changed, 3 insertions(+), 3 deletions(-)
49+
50+
diff --git a/dix/enterleave.c b/dix/enterleave.c
51+
index ded8679d76..17964b00a4 100644
52+
--- a/dix/enterleave.c
53+
+++ b/dix/enterleave.c
54+
@@ -675,7 +675,8 @@ static void
55+
DeliverStateNotifyEvent(DeviceIntPtr dev, WindowPtr win)
56+
{
57+
int evcount = 1;
58+
- deviceStateNotify *ev, *sev;
59+
+ deviceStateNotify sev[6 + (MAX_VALUATORS + 2)/3];
60+
+ deviceStateNotify *ev;
61+
deviceKeyStateNotify *kev;
62+
deviceButtonStateNotify *bev;
63+
64+
@@ -714,7 +715,7 @@ DeliverStateNotifyEvent(DeviceIntPtr dev, WindowPtr win)
65+
}
66+
}
67+
68+
- sev = ev = xallocarray(evcount, sizeof(xEvent));
69+
+ ev = sev;
70+
FixDeviceStateNotify(dev, ev, NULL, NULL, NULL, first);
71+
72+
if (b != NULL) {
73+
@@ -770,7 +771,6 @@ DeliverStateNotifyEvent(DeviceIntPtr dev, WindowPtr win)
74+
75+
DeliverEventsToWindow(dev, win, (xEvent *) sev, evcount,
76+
DeviceStateNotifyMask, NullGrab);
77+
- free(sev);
78+
}
79+
80+
void
81+
--
82+
From 219c54b8a3337456ce5270ded6a67bcde53553d5 Mon Sep 17 00:00:00 2001
83+
From: Peter Hutterer <peter.hutterer@who-t.net>
84+
Date: Mon, 18 Dec 2023 12:26:20 +1000
85+
Subject: [PATCH] dix: fix DeviceStateNotify event calculation
86+
87+
The previous code only made sense if one considers buttons and keys to
88+
be mutually exclusive on a device. That is not necessarily true, causing
89+
a number of issues.
90+
91+
This function allocates and fills in the number of xEvents we need to
92+
send the device state down the wire. This is split across multiple
93+
32-byte devices including one deviceStateNotify event and optional
94+
deviceKeyStateNotify, deviceButtonStateNotify and (possibly multiple)
95+
deviceValuator events.
96+
97+
The previous behavior would instead compose a sequence
98+
of [state, buttonstate, state, keystate, valuator...]. This is not
99+
protocol correct, and on top of that made the code extremely convoluted.
100+
101+
Fix this by streamlining: add both button and key into the deviceStateNotify
102+
and then append the key state and button state, followed by the
103+
valuators. Finally, the deviceValuator events contain up to 6 valuators
104+
per event but we only ever sent through 3 at a time. Let's double that
105+
troughput.
106+
107+
CVE-2024-0229, ZDI-CAN-22678
108+
109+
This vulnerability was discovered by:
110+
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
111+
---
112+
dix/enterleave.c | 121 ++++++++++++++++++++---------------------------
113+
1 file changed, 52 insertions(+), 69 deletions(-)
114+
115+
diff --git a/dix/enterleave.c b/dix/enterleave.c
116+
index 17964b00a4..7b7ba1098b 100644
117+
--- a/dix/enterleave.c
118+
+++ b/dix/enterleave.c
119+
@@ -615,9 +615,15 @@ FixDeviceValuator(DeviceIntPtr dev, deviceValuator * ev, ValuatorClassPtr v,
120+
121+
ev->type = DeviceValuator;
122+
ev->deviceid = dev->id;
123+
- ev->num_valuators = nval < 3 ? nval : 3;
124+
+ ev->num_valuators = nval < 6 ? nval : 6;
125+
ev->first_valuator = first;
126+
switch (ev->num_valuators) {
127+
+ case 6:
128+
+ ev->valuator2 = v->axisVal[first + 5];
129+
+ case 5:
130+
+ ev->valuator2 = v->axisVal[first + 4];
131+
+ case 4:
132+
+ ev->valuator2 = v->axisVal[first + 3];
133+
case 3:
134+
ev->valuator2 = v->axisVal[first + 2];
135+
case 2:
136+
@@ -626,7 +632,6 @@ FixDeviceValuator(DeviceIntPtr dev, deviceValuator * ev, ValuatorClassPtr v,
137+
ev->valuator0 = v->axisVal[first];
138+
break;
139+
}
140+
- first += ev->num_valuators;
141+
}
142+
143+
static void
144+
@@ -646,7 +651,7 @@ FixDeviceStateNotify(DeviceIntPtr dev, deviceStateNotify * ev, KeyClassPtr k,
145+
ev->num_buttons = b->numButtons;
146+
memcpy((char *) ev->buttons, (char *) b->down, 4);
147+
}
148+
- else if (k) {
149+
+ if (k) {
150+
ev->classes_reported |= (1 << KeyClass);
151+
ev->num_keys = k->xkbInfo->desc->max_key_code -
152+
k->xkbInfo->desc->min_key_code;
153+
@@ -670,15 +675,26 @@ FixDeviceStateNotify(DeviceIntPtr dev, deviceStateNotify * ev, KeyClassPtr k,
154+
}
155+
}
156+
157+
-
158+
+/**
159+
+ * The device state notify event is split across multiple 32-byte events.
160+
+ * The first one contains the first 32 button state bits, the first 32
161+
+ * key state bits, and the first 3 valuator values.
162+
+ *
163+
+ * If a device has more than that, the server sends out:
164+
+ * - one deviceButtonStateNotify for buttons 32 and above
165+
+ * - one deviceKeyStateNotify for keys 32 and above
166+
+ * - one deviceValuator event per 6 valuators above valuator 4
167+
+ *
168+
+ * All events but the last one have the deviceid binary ORed with MORE_EVENTS,
169+
+ */
170+
static void
171+
DeliverStateNotifyEvent(DeviceIntPtr dev, WindowPtr win)
172+
{
173+
+ /* deviceStateNotify, deviceKeyStateNotify, deviceButtonStateNotify
174+
+ * and one deviceValuator for each 6 valuators */
175+
+ deviceStateNotify sev[3 + (MAX_VALUATORS + 6)/6];
176+
int evcount = 1;
177+
- deviceStateNotify sev[6 + (MAX_VALUATORS + 2)/3];
178+
- deviceStateNotify *ev;
179+
- deviceKeyStateNotify *kev;
180+
- deviceButtonStateNotify *bev;
181+
+ deviceStateNotify *ev = sev;
182+
183+
KeyClassPtr k;
184+
ButtonClassPtr b;
185+
@@ -691,82 +707,49 @@ DeliverStateNotifyEvent(DeviceIntPtr dev, WindowPtr win)
186+
187+
if ((b = dev->button) != NULL) {
188+
nbuttons = b->numButtons;
189+
- if (nbuttons > 32)
190+
+ if (nbuttons > 32) /* first 32 are encoded in deviceStateNotify */
191+
evcount++;
192+
}
193+
if ((k = dev->key) != NULL) {
194+
nkeys = k->xkbInfo->desc->max_key_code - k->xkbInfo->desc->min_key_code;
195+
- if (nkeys > 32)
196+
+ if (nkeys > 32) /* first 32 are encoded in deviceStateNotify */
197+
evcount++;
198+
- if (nbuttons > 0) {
199+
- evcount++;
200+
- }
201+
}
202+
if ((v = dev->valuator) != NULL) {
203+
nval = v->numAxes;
204+
-
205+
- if (nval > 3)
206+
- evcount++;
207+
- if (nval > 6) {
208+
- if (!(k && b))
209+
- evcount++;
210+
- if (nval > 9)
211+
- evcount += ((nval - 7) / 3);
212+
- }
213+
+ /* first three are encoded in deviceStateNotify, then
214+
+ * it's 6 per deviceValuator event */
215+
+ evcount += ((nval - 3) + 6)/6;
216+
}
217+
218+
- ev = sev;
219+
- FixDeviceStateNotify(dev, ev, NULL, NULL, NULL, first);
220+
-
221+
- if (b != NULL) {
222+
- FixDeviceStateNotify(dev, ev++, NULL, b, v, first);
223+
- first += 3;
224+
- nval -= 3;
225+
- if (nbuttons > 32) {
226+
- (ev - 1)->deviceid |= MORE_EVENTS;
227+
- bev = (deviceButtonStateNotify *) ev++;
228+
- bev->type = DeviceButtonStateNotify;
229+
- bev->deviceid = dev->id;
230+
- memcpy((char *) &bev->buttons[4], (char *) &b->down[4],
231+
- DOWN_LENGTH - 4);
232+
- }
233+
- if (nval > 0) {
234+
- (ev - 1)->deviceid |= MORE_EVENTS;
235+
- FixDeviceValuator(dev, (deviceValuator *) ev++, v, first);
236+
- first += 3;
237+
- nval -= 3;
238+
- }
239+
+ BUG_RETURN(evcount <= ARRAY_SIZE(sev));
240+
+
241+
+ FixDeviceStateNotify(dev, ev, k, b, v, first);
242+
+
243+
+ if (b != NULL && nbuttons > 32) {
244+
+ deviceButtonStateNotify *bev = (deviceButtonStateNotify *) ++ev;
245+
+ (ev - 1)->deviceid |= MORE_EVENTS;
246+
+ bev->type = DeviceButtonStateNotify;
247+
+ bev->deviceid = dev->id;
248+
+ memcpy((char *) &bev->buttons[4], (char *) &b->down[4],
249+
+ DOWN_LENGTH - 4);
250+
}
251+
252+
- if (k != NULL) {
253+
- FixDeviceStateNotify(dev, ev++, k, NULL, v, first);
254+
- first += 3;
255+
- nval -= 3;
256+
- if (nkeys > 32) {
257+
- (ev - 1)->deviceid |= MORE_EVENTS;
258+
- kev = (deviceKeyStateNotify *) ev++;
259+
- kev->type = DeviceKeyStateNotify;
260+
- kev->deviceid = dev->id;
261+
- memmove((char *) &kev->keys[0], (char *) &k->down[4], 28);
262+
- }
263+
- if (nval > 0) {
264+
- (ev - 1)->deviceid |= MORE_EVENTS;
265+
- FixDeviceValuator(dev, (deviceValuator *) ev++, v, first);
266+
- first += 3;
267+
- nval -= 3;
268+
- }
269+
+ if (k != NULL && nkeys > 32) {
270+
+ deviceKeyStateNotify *kev = (deviceKeyStateNotify *) ++ev;
271+
+ (ev - 1)->deviceid |= MORE_EVENTS;
272+
+ kev->type = DeviceKeyStateNotify;
273+
+ kev->deviceid = dev->id;
274+
+ memmove((char *) &kev->keys[0], (char *) &k->down[4], 28);
275+
}
276+
277+
+ first = 3;
278+
+ nval -= 3;
279+
while (nval > 0) {
280+
- FixDeviceStateNotify(dev, ev++, NULL, NULL, v, first);
281+
- first += 3;
282+
- nval -= 3;
283+
- if (nval > 0) {
284+
- (ev - 1)->deviceid |= MORE_EVENTS;
285+
- FixDeviceValuator(dev, (deviceValuator *) ev++, v, first);
286+
- first += 3;
287+
- nval -= 3;
288+
- }
289+
+ ev->deviceid |= MORE_EVENTS;
290+
+ FixDeviceValuator(dev, (deviceValuator *) ++ev, v, first);
291+
+ first += 6;
292+
+ nval -= 6;
293+
}
294+
295+
DeliverEventsToWindow(dev, win, (xEvent *) sev, evcount,
296+
--
297+
From df3c65706eb169d5938df0052059f3e0d5981b74 Mon Sep 17 00:00:00 2001
298+
From: Peter Hutterer <peter.hutterer@who-t.net>
299+
Date: Thu, 21 Dec 2023 13:48:10 +1000
300+
Subject: [PATCH] Xi: when creating a new ButtonClass, set the number of
301+
buttons
302+
303+
There's a racy sequence where a master device may copy the button class
304+
from the slave, without ever initializing numButtons. This leads to a
305+
device with zero buttons but a button class which is invalid.
306+
307+
Let's copy the numButtons value from the source - by definition if we
308+
don't have a button class yet we do not have any other slave devices
309+
with more than this number of buttons anyway.
310+
311+
CVE-2024-0229, ZDI-CAN-22678
312+
313+
This vulnerability was discovered by:
314+
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
315+
---
316+
Xi/exevents.c | 1 +
317+
1 file changed, 1 insertion(+)
318+
319+
diff --git a/Xi/exevents.c b/Xi/exevents.c
320+
index 54ea11a938..e161714682 100644
321+
--- a/Xi/exevents.c
322+
+++ b/Xi/exevents.c
323+
@@ -605,6 +605,7 @@ DeepCopyPointerClasses(DeviceIntPtr from, DeviceIntPtr to)
324+
to->button = calloc(1, sizeof(ButtonClassRec));
325+
if (!to->button)
326+
FatalError("[Xi] no memory for class shift.\n");
327+
+ to->button->numButtons = from->button->numButtons;
328+
}
329+
else
330+
classes->button = NULL;
331+
--

‎SPECS/xorg-x11-server/CVE-2024-0409.patch‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
From 2ef0f1116c65d5cb06d7b6d83f8a1aea702c94f7 Mon Sep 17 00:00:00 2001
2+
From: Olivier Fourdan <ofourdan@redhat.com>
3+
Date: Wed, 6 Dec 2023 11:51:56 +0100
4+
Subject: [PATCH] ephyr,xwayland: Use the proper private key for cursor
5+
6+
The cursor in DIX is actually split in two parts, the cursor itself and
7+
the cursor bits, each with their own devPrivates.
8+
9+
The cursor itself includes the cursor bits, meaning that the cursor bits
10+
devPrivates in within structure of the cursor.
11+
12+
Both Xephyr and Xwayland were using the private key for the cursor bits
13+
to store the data for the cursor, and when using XSELINUX which comes
14+
with its own special devPrivates, the data stored in that cursor bits'
15+
devPrivates would interfere with the XSELINUX devPrivates data and the
16+
SELINUX security ID would point to some other unrelated data, causing a
17+
crash in the XSELINUX code when trying to (re)use the security ID.
18+
19+
CVE-2024-0409
20+
21+
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
22+
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
23+
---
24+
hw/kdrive/ephyr/ephyrcursor.c | 2 +-
25+
hw/xwayland/xwayland-cursor.c | 2 +-
26+
2 files changed, 2 insertions(+), 2 deletions(-)
27+
28+
diff --git a/hw/kdrive/ephyr/ephyrcursor.c b/hw/kdrive/ephyr/ephyrcursor.c
29+
index f991899c50..3f192d034a 100644
30+
--- a/hw/kdrive/ephyr/ephyrcursor.c
31+
+++ b/hw/kdrive/ephyr/ephyrcursor.c
32+
@@ -246,7 +246,7 @@ miPointerSpriteFuncRec EphyrPointerSpriteFuncs = {
33+
Bool
34+
ephyrCursorInit(ScreenPtr screen)
35+
{
36+
- if (!dixRegisterPrivateKey(&ephyrCursorPrivateKey, PRIVATE_CURSOR_BITS,
37+
+ if (!dixRegisterPrivateKey(&ephyrCursorPrivateKey, PRIVATE_CURSOR,
38+
sizeof(ephyrCursorRec)))
39+
return FALSE;
40+
41+
diff --git a/hw/xwayland/xwayland-cursor.c b/hw/xwayland/xwayland-cursor.c
42+
index e3c1aaa50c..bd94b0cfbb 100644
43+
--- a/hw/xwayland/xwayland-cursor.c
44+
+++ b/hw/xwayland/xwayland-cursor.c
45+
@@ -431,7 +431,7 @@ static miPointerScreenFuncRec xwl_pointer_screen_funcs = {
46+
Bool
47+
xwl_screen_init_cursor(struct xwl_screen *xwl_screen)
48+
{
49+
- if (!dixRegisterPrivateKey(&xwl_cursor_private_key, PRIVATE_CURSOR_BITS, 0))
50+
+ if (!dixRegisterPrivateKey(&xwl_cursor_private_key, PRIVATE_CURSOR, 0))
51+
return FALSE;
52+
53+
return miPointerInitialize(xwl_screen->screen,
54+
--
55+
GitLab
56+

0 commit comments

Comments
 (0)