rev |
line source |
shann@25634
|
1 From dd8caf39e9e15d8f302e54045dd08d8ebf1025dc Mon Sep 17 00:00:00 2001
|
shann@25634
|
2 From: Peter Hutterer <peter.hutterer@who-t.net>
|
shann@25634
|
3 Date: Tue, 5 Jul 2022 09:50:41 +1000
|
shann@25634
|
4 Subject: [PATCH] xkb: swap XkbSetDeviceInfo and XkbSetDeviceInfoCheck
|
shann@25634
|
5
|
shann@25634
|
6 XKB often uses a FooCheck and Foo function pair, the former is supposed
|
shann@25634
|
7 to check all values in the request and error out on BadLength,
|
shann@25634
|
8 BadValue, etc. The latter is then called once we're confident the values
|
shann@25634
|
9 are good (they may still fail on an individual device, but that's a
|
shann@25634
|
10 different topic).
|
shann@25634
|
11
|
shann@25634
|
12 In the case of XkbSetDeviceInfo, those functions were incorrectly
|
shann@25634
|
13 named, with XkbSetDeviceInfo ending up as the checker function and
|
shann@25634
|
14 XkbSetDeviceInfoCheck as the setter function. As a result, the setter
|
shann@25634
|
15 function was called before the checker function, accessing request
|
shann@25634
|
16 data and modifying device state before we ensured that the data is
|
shann@25634
|
17 valid.
|
shann@25634
|
18
|
shann@25634
|
19 In particular, the setter function relied on values being already
|
shann@25634
|
20 byte-swapped. This in turn could lead to potential OOB memory access.
|
shann@25634
|
21
|
shann@25634
|
22 Fix this by correctly naming the functions and moving the length checks
|
shann@25634
|
23 over to the checker function. These were added in 87c64fc5b0 to the
|
shann@25634
|
24 wrong function, probably due to the incorrect naming.
|
shann@25634
|
25
|
shann@25634
|
26 Fixes ZDI-CAN 16070, CVE-2022-2320.
|
shann@25634
|
27
|
shann@25634
|
28 This vulnerability was discovered by:
|
shann@25634
|
29 Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
|
shann@25634
|
30
|
shann@25634
|
31 Introduced in c06e27b2f6fd9f7b9f827623a48876a225264132
|
shann@25634
|
32
|
shann@25634
|
33 Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
|
shann@25634
|
34 ---
|
shann@25634
|
35 xkb/xkb.c | 46 +++++++++++++++++++++++++---------------------
|
shann@25634
|
36 1 file changed, 25 insertions(+), 21 deletions(-)
|
shann@25634
|
37
|
shann@25634
|
38 diff --git a/xkb/xkb.c b/xkb/xkb.c
|
shann@25634
|
39 index 64e52611e..34b2c290b 100644
|
shann@25634
|
40 --- a/xkb/xkb.c
|
shann@25634
|
41 +++ b/xkb/xkb.c
|
shann@25634
|
42 @@ -6550,7 +6550,8 @@ ProcXkbGetDeviceInfo(ClientPtr client)
|
shann@25634
|
43 static char *
|
shann@25634
|
44 CheckSetDeviceIndicators(char *wire,
|
shann@25634
|
45 DeviceIntPtr dev,
|
shann@25634
|
46 - int num, int *status_rtrn, ClientPtr client)
|
shann@25634
|
47 + int num, int *status_rtrn, ClientPtr client,
|
shann@25634
|
48 + xkbSetDeviceInfoReq * stuff)
|
shann@25634
|
49 {
|
shann@25634
|
50 xkbDeviceLedsWireDesc *ledWire;
|
shann@25634
|
51 int i;
|
shann@25634
|
52 @@ -6558,6 +6559,11 @@ CheckSetDeviceIndicators(char *wire,
|
shann@25634
|
53
|
shann@25634
|
54 ledWire = (xkbDeviceLedsWireDesc *) wire;
|
shann@25634
|
55 for (i = 0; i < num; i++) {
|
shann@25634
|
56 + if (!_XkbCheckRequestBounds(client, stuff, ledWire, ledWire + 1)) {
|
shann@25634
|
57 + *status_rtrn = BadLength;
|
shann@25634
|
58 + return (char *) ledWire;
|
shann@25634
|
59 + }
|
shann@25634
|
60 +
|
shann@25634
|
61 if (client->swapped) {
|
shann@25634
|
62 swaps(&ledWire->ledClass);
|
shann@25634
|
63 swaps(&ledWire->ledID);
|
shann@25634
|
64 @@ -6585,6 +6591,11 @@ CheckSetDeviceIndicators(char *wire,
|
shann@25634
|
65 atomWire = (CARD32 *) &ledWire[1];
|
shann@25634
|
66 if (nNames > 0) {
|
shann@25634
|
67 for (n = 0; n < nNames; n++) {
|
shann@25634
|
68 + if (!_XkbCheckRequestBounds(client, stuff, atomWire, atomWire + 1)) {
|
shann@25634
|
69 + *status_rtrn = BadLength;
|
shann@25634
|
70 + return (char *) atomWire;
|
shann@25634
|
71 + }
|
shann@25634
|
72 +
|
shann@25634
|
73 if (client->swapped) {
|
shann@25634
|
74 swapl(atomWire);
|
shann@25634
|
75 }
|
shann@25634
|
76 @@ -6596,6 +6607,10 @@ CheckSetDeviceIndicators(char *wire,
|
shann@25634
|
77 mapWire = (xkbIndicatorMapWireDesc *) atomWire;
|
shann@25634
|
78 if (nMaps > 0) {
|
shann@25634
|
79 for (n = 0; n < nMaps; n++) {
|
shann@25634
|
80 + if (!_XkbCheckRequestBounds(client, stuff, mapWire, mapWire + 1)) {
|
shann@25634
|
81 + *status_rtrn = BadLength;
|
shann@25634
|
82 + return (char *) mapWire;
|
shann@25634
|
83 + }
|
shann@25634
|
84 if (client->swapped) {
|
shann@25634
|
85 swaps(&mapWire->virtualMods);
|
shann@25634
|
86 swapl(&mapWire->ctrls);
|
shann@25634
|
87 @@ -6647,11 +6662,6 @@ SetDeviceIndicators(char *wire,
|
shann@25634
|
88 xkbIndicatorMapWireDesc *mapWire;
|
shann@25634
|
89 XkbSrvLedInfoPtr sli;
|
shann@25634
|
90
|
shann@25634
|
91 - if (!_XkbCheckRequestBounds(client, stuff, ledWire, ledWire + 1)) {
|
shann@25634
|
92 - *status_rtrn = BadLength;
|
shann@25634
|
93 - return (char *) ledWire;
|
shann@25634
|
94 - }
|
shann@25634
|
95 -
|
shann@25634
|
96 namec = mapc = statec = 0;
|
shann@25634
|
97 sli = XkbFindSrvLedInfo(dev, ledWire->ledClass, ledWire->ledID,
|
shann@25634
|
98 XkbXI_IndicatorMapsMask);
|
shann@25634
|
99 @@ -6670,10 +6680,6 @@ SetDeviceIndicators(char *wire,
|
shann@25634
|
100 memset((char *) sli->names, 0, XkbNumIndicators * sizeof(Atom));
|
shann@25634
|
101 for (n = 0, bit = 1; n < XkbNumIndicators; n++, bit <<= 1) {
|
shann@25634
|
102 if (ledWire->namesPresent & bit) {
|
shann@25634
|
103 - if (!_XkbCheckRequestBounds(client, stuff, atomWire, atomWire + 1)) {
|
shann@25634
|
104 - *status_rtrn = BadLength;
|
shann@25634
|
105 - return (char *) atomWire;
|
shann@25634
|
106 - }
|
shann@25634
|
107 sli->names[n] = (Atom) *atomWire;
|
shann@25634
|
108 if (sli->names[n] == None)
|
shann@25634
|
109 ledWire->namesPresent &= ~bit;
|
shann@25634
|
110 @@ -6691,10 +6697,6 @@ SetDeviceIndicators(char *wire,
|
shann@25634
|
111 if (ledWire->mapsPresent) {
|
shann@25634
|
112 for (n = 0, bit = 1; n < XkbNumIndicators; n++, bit <<= 1) {
|
shann@25634
|
113 if (ledWire->mapsPresent & bit) {
|
shann@25634
|
114 - if (!_XkbCheckRequestBounds(client, stuff, mapWire, mapWire + 1)) {
|
shann@25634
|
115 - *status_rtrn = BadLength;
|
shann@25634
|
116 - return (char *) mapWire;
|
shann@25634
|
117 - }
|
shann@25634
|
118 sli->maps[n].flags = mapWire->flags;
|
shann@25634
|
119 sli->maps[n].which_groups = mapWire->whichGroups;
|
shann@25634
|
120 sli->maps[n].groups = mapWire->groups;
|
shann@25634
|
121 @@ -6730,13 +6732,17 @@ SetDeviceIndicators(char *wire,
|
shann@25634
|
122 }
|
shann@25634
|
123
|
shann@25634
|
124 static int
|
shann@25634
|
125 -_XkbSetDeviceInfo(ClientPtr client, DeviceIntPtr dev,
|
shann@25634
|
126 +_XkbSetDeviceInfoCheck(ClientPtr client, DeviceIntPtr dev,
|
shann@25634
|
127 xkbSetDeviceInfoReq * stuff)
|
shann@25634
|
128 {
|
shann@25634
|
129 char *wire;
|
shann@25634
|
130
|
shann@25634
|
131 wire = (char *) &stuff[1];
|
shann@25634
|
132 if (stuff->change & XkbXI_ButtonActionsMask) {
|
shann@25634
|
133 + int sz = stuff->nBtns * SIZEOF(xkbActionWireDesc);
|
shann@25634
|
134 + if (!_XkbCheckRequestBounds(client, stuff, wire, (char *) wire + sz))
|
shann@25634
|
135 + return BadLength;
|
shann@25634
|
136 +
|
shann@25634
|
137 if (!dev->button) {
|
shann@25634
|
138 client->errorValue = _XkbErrCode2(XkbErr_BadClass, ButtonClass);
|
shann@25634
|
139 return XkbKeyboardErrorCode;
|
shann@25634
|
140 @@ -6747,13 +6753,13 @@ _XkbSetDeviceInfo(ClientPtr client, DeviceIntPtr dev,
|
shann@25634
|
141 dev->button->numButtons);
|
shann@25634
|
142 return BadMatch;
|
shann@25634
|
143 }
|
shann@25634
|
144 - wire += (stuff->nBtns * SIZEOF(xkbActionWireDesc));
|
shann@25634
|
145 + wire += sz;
|
shann@25634
|
146 }
|
shann@25634
|
147 if (stuff->change & XkbXI_IndicatorsMask) {
|
shann@25634
|
148 int status = Success;
|
shann@25634
|
149
|
shann@25634
|
150 wire = CheckSetDeviceIndicators(wire, dev, stuff->nDeviceLedFBs,
|
shann@25634
|
151 - &status, client);
|
shann@25634
|
152 + &status, client, stuff);
|
shann@25634
|
153 if (status != Success)
|
shann@25634
|
154 return status;
|
shann@25634
|
155 }
|
shann@25634
|
156 @@ -6764,8 +6770,8 @@ _XkbSetDeviceInfo(ClientPtr client, DeviceIntPtr dev,
|
shann@25634
|
157 }
|
shann@25634
|
158
|
shann@25634
|
159 static int
|
shann@25634
|
160 -_XkbSetDeviceInfoCheck(ClientPtr client, DeviceIntPtr dev,
|
shann@25634
|
161 - xkbSetDeviceInfoReq * stuff)
|
shann@25634
|
162 +_XkbSetDeviceInfo(ClientPtr client, DeviceIntPtr dev,
|
shann@25634
|
163 + xkbSetDeviceInfoReq * stuff)
|
shann@25634
|
164 {
|
shann@25634
|
165 char *wire;
|
shann@25634
|
166 xkbExtensionDeviceNotify ed;
|
shann@25634
|
167 @@ -6789,8 +6795,6 @@ _XkbSetDeviceInfoCheck(ClientPtr client, DeviceIntPtr dev,
|
shann@25634
|
168 if (stuff->firstBtn + stuff->nBtns > nBtns)
|
shann@25634
|
169 return BadValue;
|
shann@25634
|
170 sz = stuff->nBtns * SIZEOF(xkbActionWireDesc);
|
shann@25634
|
171 - if (!_XkbCheckRequestBounds(client, stuff, wire, (char *) wire + sz))
|
shann@25634
|
172 - return BadLength;
|
shann@25634
|
173 memcpy((char *) &acts[stuff->firstBtn], (char *) wire, sz);
|
shann@25634
|
174 wire += sz;
|
shann@25634
|
175 ed.reason |= XkbXI_ButtonActionsMask;
|
shann@25634
|
176 --
|
shann@25634
|
177 GitLab
|
shann@25634
|
178
|