1 From 54fa1f815507cd27280f661be7a64f2f2e6c579e Mon Sep 17 00:00:00 2001
2 From: Alan Coopersmith <alan.coopersmith@oracle.com>
3 Date: Sun, 26 Jan 2014 10:54:41 -0800
4 Subject: [PATCH 08/33] Xi: unvalidated lengths in Xinput extension
7 Multiple functions in the Xinput extension handling of requests from
8 clients failed to check that the length of the request sent by the
9 client was large enough to perform all the required operations and
10 thus could read or write to memory outside the bounds of the request
13 This commit includes the creation of a new REQUEST_AT_LEAST_EXTRA_SIZE
14 macro in include/dix.h for the common case of needing to ensure a
15 request is large enough to include both the request itself and a
16 minimum amount of extra data following the request header.
18 Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
19 Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
21 Xi/chgdctl.c | 8 ++++++--
25 Xi/xichangecursor.c | 2 +-
26 Xi/xichangehierarchy.c | 35 ++++++++++++++++++++++++++++++++---
27 Xi/xigetclientpointer.c | 1 +
28 Xi/xigrabdev.c | 9 ++++++++-
29 Xi/xipassivegrab.c | 12 ++++++++++--
30 Xi/xiproperty.c | 14 ++++++--------
31 Xi/xiquerydevice.c | 1 +
32 Xi/xiquerypointer.c | 2 ++
33 Xi/xiselectev.c | 8 ++++++++
34 Xi/xisetclientpointer.c | 3 ++-
35 Xi/xisetdevfocus.c | 4 ++++
36 Xi/xiwarppointer.c | 2 ++
37 include/dix.h | 4 ++++
38 17 files changed, 94 insertions(+), 18 deletions(-)
42 @@ -78,7 +78,7 @@ SProcXChangeDeviceControl(ClientPtr clie
44 REQUEST(xChangeDeviceControlReq);
45 swaps(&stuff->length);
46 - REQUEST_AT_LEAST_SIZE(xChangeDeviceControlReq);
47 + REQUEST_AT_LEAST_EXTRA_SIZE(xChangeDeviceControlReq, sizeof(xDeviceCtl));
48 swaps(&stuff->control);
49 ctl = (xDeviceCtl *) &stuff[1];
51 @@ -115,7 +115,7 @@ ProcXChangeDeviceControl(ClientPtr clien
54 REQUEST(xChangeDeviceControlReq);
55 - REQUEST_AT_LEAST_SIZE(xChangeDeviceControlReq);
56 + REQUEST_AT_LEAST_EXTRA_SIZE(xChangeDeviceControlReq, sizeof(xDeviceCtl));
58 len = stuff->length - bytes_to_int32(sizeof(xChangeDeviceControlReq));
59 ret = dixLookupDevice(&dev, stuff->deviceid, client, DixManageAccess);
60 @@ -192,6 +192,10 @@ ProcXChangeDeviceControl(ClientPtr clien
63 e = (xDeviceEnableCtl *) &stuff[1];
64 + if ((len != bytes_to_int32(sizeof(xDeviceEnableCtl)))) {
69 if (IsXTestDevice(dev, NULL))
73 @@ -467,6 +467,8 @@ ProcXChangeFeedbackControl(ClientPtr cli
74 xStringFeedbackCtl *f = ((xStringFeedbackCtl *) &stuff[1]);
76 if (client->swapped) {
77 + if (len < bytes_to_int32(sizeof(xStringFeedbackCtl)))
79 swaps(&f->num_keysyms);
84 @@ -135,6 +135,9 @@ ProcXSendExtensionEvent(ClientPtr client
88 + if (stuff->num_events == 0)
91 /* The client's event type must be one defined by an extension. */
93 first = ((xEvent *) &stuff[1]);
97 SProcXIAllowEvents(ClientPtr client)
99 REQUEST(xXIAllowEventsReq);
100 + REQUEST_AT_LEAST_SIZE(xXIAllowEventsReq);
102 swaps(&stuff->length);
103 swaps(&stuff->deviceid);
104 @@ -55,6 +56,7 @@ SProcXIAllowEvents(ClientPtr client)
105 if (stuff->length > 3) {
106 xXI2_2AllowEventsReq *req_xi22 = (xXI2_2AllowEventsReq *) stuff;
108 + REQUEST_AT_LEAST_SIZE(xXI2_2AllowEventsReq);
109 swapl(&req_xi22->touchid);
110 swapl(&req_xi22->grab_window);
112 --- a/Xi/xichangecursor.c
113 +++ b/Xi/xichangecursor.c
114 @@ -57,11 +57,11 @@ int
115 SProcXIChangeCursor(ClientPtr client)
117 REQUEST(xXIChangeCursorReq);
118 + REQUEST_SIZE_MATCH(xXIChangeCursorReq);
119 swaps(&stuff->length);
121 swapl(&stuff->cursor);
122 swaps(&stuff->deviceid);
123 - REQUEST_SIZE_MATCH(xXIChangeCursorReq);
124 return (ProcXIChangeCursor(client));
127 --- a/Xi/xichangehierarchy.c
128 +++ b/Xi/xichangehierarchy.c
129 @@ -411,7 +411,7 @@ int
130 ProcXIChangeHierarchy(ClientPtr client)
132 xXIAnyHierarchyChangeInfo *any;
133 - int required_len = sizeof(xXIChangeHierarchyReq);
134 + size_t len; /* length of data remaining in request */
136 int flags[MAXDEVICES] = { 0 };
138 @@ -421,21 +421,46 @@ ProcXIChangeHierarchy(ClientPtr client)
139 if (!stuff->num_changes)
142 + if (stuff->length > (INT_MAX >> 2))
144 + len = (stuff->length << 2) - sizeof(xXIAnyHierarchyChangeInfo);
146 any = (xXIAnyHierarchyChangeInfo *) &stuff[1];
147 while (stuff->num_changes--) {
148 + if (len < sizeof(xXIAnyHierarchyChangeInfo)) {
153 SWAPIF(swaps(&any->type));
154 SWAPIF(swaps(&any->length));
156 - required_len += any->length;
157 - if ((stuff->length * 4) < required_len)
158 + if ((any->length > (INT_MAX >> 2)) || (len < (any->length << 2)))
161 +#define CHANGE_SIZE_MATCH(type) \
163 + if ((len < sizeof(type)) || (any->length != (sizeof(type) >> 2))) { \
172 xXIAddMasterInfo *c = (xXIAddMasterInfo *) any;
174 + /* Variable length, due to appended name string */
175 + if (len < sizeof(xXIAddMasterInfo)) {
179 SWAPIF(swaps(&c->name_len));
180 + if (c->name_len > (len - sizeof(xXIAddMasterInfo))) {
185 rc = add_master(client, c, flags);
187 @@ -446,6 +471,7 @@ ProcXIChangeHierarchy(ClientPtr client)
189 xXIRemoveMasterInfo *r = (xXIRemoveMasterInfo *) any;
191 + CHANGE_SIZE_MATCH(xXIRemoveMasterInfo);
192 rc = remove_master(client, r, flags);
195 @@ -455,6 +481,7 @@ ProcXIChangeHierarchy(ClientPtr client)
197 xXIDetachSlaveInfo *c = (xXIDetachSlaveInfo *) any;
199 + CHANGE_SIZE_MATCH(xXIDetachSlaveInfo);
200 rc = detach_slave(client, c, flags);
203 @@ -464,6 +491,7 @@ ProcXIChangeHierarchy(ClientPtr client)
205 xXIAttachSlaveInfo *c = (xXIAttachSlaveInfo *) any;
207 + CHANGE_SIZE_MATCH(xXIAttachSlaveInfo);
208 rc = attach_slave(client, c, flags);
211 @@ -471,6 +499,7 @@ ProcXIChangeHierarchy(ClientPtr client)
215 + len -= any->length * 4;
216 any = (xXIAnyHierarchyChangeInfo *) ((char *) any + any->length * 4);
219 --- a/Xi/xigetclientpointer.c
220 +++ b/Xi/xigetclientpointer.c
221 @@ -50,6 +50,7 @@ int
222 SProcXIGetClientPointer(ClientPtr client)
224 REQUEST(xXIGetClientPointerReq);
225 + REQUEST_SIZE_MATCH(xXIGetClientPointerReq);
227 swaps(&stuff->length);
231 @@ -47,6 +47,11 @@ int
232 SProcXIGrabDevice(ClientPtr client)
234 REQUEST(xXIGrabDeviceReq);
236 + * Check here for at least the length of the struct we swap, then
237 + * let ProcXIGrabDevice check the full size after we swap mask_len.
239 + REQUEST_AT_LEAST_SIZE(xXIGrabDeviceReq);
241 swaps(&stuff->length);
242 swaps(&stuff->deviceid);
243 @@ -71,7 +76,7 @@ ProcXIGrabDevice(ClientPtr client)
244 unsigned int pointer_mode;
246 REQUEST(xXIGrabDeviceReq);
247 - REQUEST_AT_LEAST_SIZE(xXIGrabDeviceReq);
248 + REQUEST_FIXED_SIZE(xXIGrabDeviceReq, ((size_t) stuff->mask_len) * 4);
250 ret = dixLookupDevice(&dev, stuff->deviceid, client, DixGrabAccess);
252 @@ -131,6 +136,7 @@ int
253 SProcXIUngrabDevice(ClientPtr client)
255 REQUEST(xXIUngrabDeviceReq);
256 + REQUEST_SIZE_MATCH(xXIUngrabDeviceReq);
258 swaps(&stuff->length);
259 swaps(&stuff->deviceid);
260 @@ -148,6 +154,7 @@ ProcXIUngrabDevice(ClientPtr client)
263 REQUEST(xXIUngrabDeviceReq);
264 + REQUEST_SIZE_MATCH(xXIUngrabDeviceReq);
266 ret = dixLookupDevice(&dev, stuff->deviceid, client, DixGetAttrAccess);
268 --- a/Xi/xipassivegrab.c
269 +++ b/Xi/xipassivegrab.c
270 @@ -53,6 +53,7 @@ SProcXIPassiveGrabDevice(ClientPtr clien
273 REQUEST(xXIPassiveGrabDeviceReq);
274 + REQUEST_AT_LEAST_SIZE(xXIPassiveGrabDeviceReq);
276 swaps(&stuff->length);
277 swaps(&stuff->deviceid);
278 @@ -63,6 +64,8 @@ SProcXIPassiveGrabDevice(ClientPtr clien
279 swaps(&stuff->mask_len);
280 swaps(&stuff->num_modifiers);
282 + REQUEST_FIXED_SIZE(xXIPassiveGrabDeviceReq,
283 + ((uint32_t) stuff->mask_len + stuff->num_modifiers) *4);
284 mods = (uint32_t *) &stuff[1] + stuff->mask_len;
286 for (i = 0; i < stuff->num_modifiers; i++, mods++) {
287 @@ -92,7 +95,8 @@ ProcXIPassiveGrabDevice(ClientPtr client
290 REQUEST(xXIPassiveGrabDeviceReq);
291 - REQUEST_AT_LEAST_SIZE(xXIPassiveGrabDeviceReq);
292 + REQUEST_FIXED_SIZE(xXIPassiveGrabDeviceReq,
293 + ((uint32_t) stuff->mask_len + stuff->num_modifiers) * 4);
295 if (stuff->deviceid == XIAllDevices)
296 dev = inputInfo.all_devices;
297 @@ -252,6 +256,7 @@ SProcXIPassiveUngrabDevice(ClientPtr cli
300 REQUEST(xXIPassiveUngrabDeviceReq);
301 + REQUEST_AT_LEAST_SIZE(xXIPassiveUngrabDeviceReq);
303 swaps(&stuff->length);
304 swapl(&stuff->grab_window);
305 @@ -259,6 +264,8 @@ SProcXIPassiveUngrabDevice(ClientPtr cli
306 swapl(&stuff->detail);
307 swaps(&stuff->num_modifiers);
309 + REQUEST_FIXED_SIZE(xXIPassiveUngrabDeviceReq,
310 + ((uint32_t) stuff->num_modifiers) << 2);
311 modifiers = (uint32_t *) &stuff[1];
313 for (i = 0; i < stuff->num_modifiers; i++, modifiers++)
314 @@ -277,7 +284,8 @@ ProcXIPassiveUngrabDevice(ClientPtr clie
317 REQUEST(xXIPassiveUngrabDeviceReq);
318 - REQUEST_AT_LEAST_SIZE(xXIPassiveUngrabDeviceReq);
319 + REQUEST_FIXED_SIZE(xXIPassiveUngrabDeviceReq,
320 + ((uint32_t) stuff->num_modifiers) << 2);
322 if (stuff->deviceid == XIAllDevices)
323 dev = inputInfo.all_devices;
324 --- a/Xi/xiproperty.c
325 +++ b/Xi/xiproperty.c
326 @@ -1013,10 +1013,9 @@ int
327 SProcXListDeviceProperties(ClientPtr client)
329 REQUEST(xListDevicePropertiesReq);
330 + REQUEST_SIZE_MATCH(xListDevicePropertiesReq);
332 swaps(&stuff->length);
334 - REQUEST_SIZE_MATCH(xListDevicePropertiesReq);
335 return (ProcXListDeviceProperties(client));
338 @@ -1037,10 +1036,10 @@ int
339 SProcXDeleteDeviceProperty(ClientPtr client)
341 REQUEST(xDeleteDevicePropertyReq);
342 + REQUEST_SIZE_MATCH(xDeleteDevicePropertyReq);
344 swaps(&stuff->length);
345 swapl(&stuff->property);
346 - REQUEST_SIZE_MATCH(xDeleteDevicePropertyReq);
347 return (ProcXDeleteDeviceProperty(client));
350 @@ -1048,13 +1047,13 @@ int
351 SProcXGetDeviceProperty(ClientPtr client)
353 REQUEST(xGetDevicePropertyReq);
354 + REQUEST_SIZE_MATCH(xGetDevicePropertyReq);
356 swaps(&stuff->length);
357 swapl(&stuff->property);
359 swapl(&stuff->longOffset);
360 swapl(&stuff->longLength);
361 - REQUEST_SIZE_MATCH(xGetDevicePropertyReq);
362 return (ProcXGetDeviceProperty(client));
365 @@ -1253,11 +1252,10 @@ int
366 SProcXIListProperties(ClientPtr client)
368 REQUEST(xXIListPropertiesReq);
369 + REQUEST_SIZE_MATCH(xXIListPropertiesReq);
371 swaps(&stuff->length);
372 swaps(&stuff->deviceid);
374 - REQUEST_SIZE_MATCH(xXIListPropertiesReq);
375 return (ProcXIListProperties(client));
378 @@ -1279,11 +1277,11 @@ int
379 SProcXIDeleteProperty(ClientPtr client)
381 REQUEST(xXIDeletePropertyReq);
382 + REQUEST_SIZE_MATCH(xXIDeletePropertyReq);
384 swaps(&stuff->length);
385 swaps(&stuff->deviceid);
386 swapl(&stuff->property);
387 - REQUEST_SIZE_MATCH(xXIDeletePropertyReq);
388 return (ProcXIDeleteProperty(client));
391 @@ -1291,6 +1289,7 @@ int
392 SProcXIGetProperty(ClientPtr client)
394 REQUEST(xXIGetPropertyReq);
395 + REQUEST_SIZE_MATCH(xXIGetPropertyReq);
397 swaps(&stuff->length);
398 swaps(&stuff->deviceid);
399 @@ -1298,7 +1297,6 @@ SProcXIGetProperty(ClientPtr client)
401 swapl(&stuff->offset);
403 - REQUEST_SIZE_MATCH(xXIGetPropertyReq);
404 return (ProcXIGetProperty(client));
407 --- a/Xi/xiquerydevice.c
408 +++ b/Xi/xiquerydevice.c
409 @@ -54,6 +54,7 @@ int
410 SProcXIQueryDevice(ClientPtr client)
412 REQUEST(xXIQueryDeviceReq);
413 + REQUEST_SIZE_MATCH(xXIQueryDeviceReq);
415 swaps(&stuff->length);
416 swaps(&stuff->deviceid);
417 --- a/Xi/xiquerypointer.c
418 +++ b/Xi/xiquerypointer.c
419 @@ -63,6 +63,8 @@ int
420 SProcXIQueryPointer(ClientPtr client)
422 REQUEST(xXIQueryPointerReq);
423 + REQUEST_SIZE_MATCH(xXIQueryPointerReq);
425 swaps(&stuff->length);
426 swaps(&stuff->deviceid);
428 --- a/Xi/xiselectev.c
429 +++ b/Xi/xiselectev.c
430 @@ -114,6 +114,7 @@ int
431 SProcXISelectEvents(ClientPtr client)
435 xXIEventMask *evmask;
437 REQUEST(xXISelectEventsReq);
438 @@ -122,10 +123,17 @@ SProcXISelectEvents(ClientPtr client)
440 swaps(&stuff->num_masks);
442 + len = stuff->length - bytes_to_int32(sizeof(xXISelectEventsReq));
443 evmask = (xXIEventMask *) &stuff[1];
444 for (i = 0; i < stuff->num_masks; i++) {
445 + if (len < bytes_to_int32(sizeof(xXIEventMask)))
447 + len -= bytes_to_int32(sizeof(xXIEventMask));
448 swaps(&evmask->deviceid);
449 swaps(&evmask->mask_len);
450 + if (len < evmask->mask_len)
452 + len -= evmask->mask_len;
454 (xXIEventMask *) (((char *) &evmask[1]) + evmask->mask_len * 4);
456 --- a/Xi/xisetclientpointer.c
457 +++ b/Xi/xisetclientpointer.c
458 @@ -51,10 +51,11 @@ int
459 SProcXISetClientPointer(ClientPtr client)
461 REQUEST(xXISetClientPointerReq);
462 + REQUEST_SIZE_MATCH(xXISetClientPointerReq);
464 swaps(&stuff->length);
466 swaps(&stuff->deviceid);
467 - REQUEST_SIZE_MATCH(xXISetClientPointerReq);
468 return (ProcXISetClientPointer(client));
471 --- a/Xi/xisetdevfocus.c
472 +++ b/Xi/xisetdevfocus.c
473 @@ -44,6 +44,8 @@ int
474 SProcXISetFocus(ClientPtr client)
476 REQUEST(xXISetFocusReq);
477 + REQUEST_AT_LEAST_SIZE(xXISetFocusReq);
479 swaps(&stuff->length);
480 swaps(&stuff->deviceid);
481 swapl(&stuff->focus);
482 @@ -56,6 +58,8 @@ int
483 SProcXIGetFocus(ClientPtr client)
485 REQUEST(xXIGetFocusReq);
486 + REQUEST_AT_LEAST_SIZE(xXIGetFocusReq);
488 swaps(&stuff->length);
489 swaps(&stuff->deviceid);
491 --- a/Xi/xiwarppointer.c
492 +++ b/Xi/xiwarppointer.c
493 @@ -56,6 +56,8 @@ int
494 SProcXIWarpPointer(ClientPtr client)
496 REQUEST(xXIWarpPointerReq);
497 + REQUEST_SIZE_MATCH(xXIWarpPointerReq);
499 swaps(&stuff->length);
500 swapl(&stuff->src_win);
501 swapl(&stuff->dst_win);
504 @@ -74,6 +74,10 @@ SOFTWARE.
505 if ((sizeof(req) >> 2) > client->req_len )\
508 +#define REQUEST_AT_LEAST_EXTRA_SIZE(req, extra) \
509 + if (((sizeof(req) + ((uint64_t) extra)) >> 2) > client->req_len ) \
512 #define REQUEST_FIXED_SIZE(req, n)\
513 if (((sizeof(req) >> 2) > client->req_len) || \
514 ((n >> 2) >= client->req_len) || \