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(-)
40 diff --git a/Xi/chgdctl.c b/Xi/chgdctl.c
41 index d078aa2..b3ee867 100644
44 @@ -78,7 +78,7 @@ SProcXChangeDeviceControl(ClientPtr client)
46 REQUEST(xChangeDeviceControlReq);
47 swaps(&stuff->length);
48 - REQUEST_AT_LEAST_SIZE(xChangeDeviceControlReq);
49 + REQUEST_AT_LEAST_EXTRA_SIZE(xChangeDeviceControlReq, sizeof(xDeviceCtl));
50 swaps(&stuff->control);
51 ctl = (xDeviceCtl *) &stuff[1];
53 @@ -115,7 +115,7 @@ ProcXChangeDeviceControl(ClientPtr client)
56 REQUEST(xChangeDeviceControlReq);
57 - REQUEST_AT_LEAST_SIZE(xChangeDeviceControlReq);
58 + REQUEST_AT_LEAST_EXTRA_SIZE(xChangeDeviceControlReq, sizeof(xDeviceCtl));
60 len = stuff->length - bytes_to_int32(sizeof(xChangeDeviceControlReq));
61 ret = dixLookupDevice(&dev, stuff->deviceid, client, DixManageAccess);
62 @@ -192,6 +192,10 @@ ProcXChangeDeviceControl(ClientPtr client)
65 e = (xDeviceEnableCtl *) &stuff[1];
66 + if ((len != bytes_to_int32(sizeof(xDeviceEnableCtl)))) {
71 if (IsXTestDevice(dev, NULL))
73 diff --git a/Xi/chgfctl.c b/Xi/chgfctl.c
74 index 6dcf60c..224c2ba 100644
77 @@ -467,6 +467,8 @@ ProcXChangeFeedbackControl(ClientPtr client)
78 xStringFeedbackCtl *f = ((xStringFeedbackCtl *) &stuff[1]);
80 if (client->swapped) {
81 + if (len < bytes_to_int32(sizeof(xStringFeedbackCtl)))
83 swaps(&f->num_keysyms);
86 diff --git a/Xi/sendexev.c b/Xi/sendexev.c
87 index 3c21386..183f88d 100644
90 @@ -135,6 +135,9 @@ ProcXSendExtensionEvent(ClientPtr client)
94 + if (stuff->num_events == 0)
97 /* The client's event type must be one defined by an extension. */
99 first = ((xEvent *) &stuff[1]);
100 diff --git a/Xi/xiallowev.c b/Xi/xiallowev.c
101 index ebef233..ca263ef 100644
104 @@ -48,6 +48,7 @@ int
105 SProcXIAllowEvents(ClientPtr client)
107 REQUEST(xXIAllowEventsReq);
108 + REQUEST_AT_LEAST_SIZE(xXIAllowEventsReq);
110 swaps(&stuff->length);
111 swaps(&stuff->deviceid);
112 @@ -55,6 +56,7 @@ SProcXIAllowEvents(ClientPtr client)
113 if (stuff->length > 3) {
114 xXI2_2AllowEventsReq *req_xi22 = (xXI2_2AllowEventsReq *) stuff;
116 + REQUEST_AT_LEAST_SIZE(xXI2_2AllowEventsReq);
117 swapl(&req_xi22->touchid);
118 swapl(&req_xi22->grab_window);
120 diff --git a/Xi/xichangecursor.c b/Xi/xichangecursor.c
121 index 7a1bb7a..8e6255b 100644
122 --- a/Xi/xichangecursor.c
123 +++ b/Xi/xichangecursor.c
124 @@ -57,11 +57,11 @@ int
125 SProcXIChangeCursor(ClientPtr client)
127 REQUEST(xXIChangeCursorReq);
128 + REQUEST_SIZE_MATCH(xXIChangeCursorReq);
129 swaps(&stuff->length);
131 swapl(&stuff->cursor);
132 swaps(&stuff->deviceid);
133 - REQUEST_SIZE_MATCH(xXIChangeCursorReq);
134 return (ProcXIChangeCursor(client));
137 diff --git a/Xi/xichangehierarchy.c b/Xi/xichangehierarchy.c
138 index 9e36354..2732445 100644
139 --- a/Xi/xichangehierarchy.c
140 +++ b/Xi/xichangehierarchy.c
141 @@ -411,7 +411,7 @@ int
142 ProcXIChangeHierarchy(ClientPtr client)
144 xXIAnyHierarchyChangeInfo *any;
145 - int required_len = sizeof(xXIChangeHierarchyReq);
146 + size_t len; /* length of data remaining in request */
148 int flags[MAXDEVICES] = { 0 };
150 @@ -421,21 +421,46 @@ ProcXIChangeHierarchy(ClientPtr client)
151 if (!stuff->num_changes)
154 + if (stuff->length > (INT_MAX >> 2))
156 + len = (stuff->length << 2) - sizeof(xXIAnyHierarchyChangeInfo);
158 any = (xXIAnyHierarchyChangeInfo *) &stuff[1];
159 while (stuff->num_changes--) {
160 + if (len < sizeof(xXIAnyHierarchyChangeInfo)) {
165 SWAPIF(swaps(&any->type));
166 SWAPIF(swaps(&any->length));
168 - required_len += any->length;
169 - if ((stuff->length * 4) < required_len)
170 + if ((any->length > (INT_MAX >> 2)) || (len < (any->length << 2)))
173 +#define CHANGE_SIZE_MATCH(type) \
175 + if ((len < sizeof(type)) || (any->length != (sizeof(type) >> 2))) { \
184 xXIAddMasterInfo *c = (xXIAddMasterInfo *) any;
186 + /* Variable length, due to appended name string */
187 + if (len < sizeof(xXIAddMasterInfo)) {
191 SWAPIF(swaps(&c->name_len));
192 + if (c->name_len > (len - sizeof(xXIAddMasterInfo))) {
197 rc = add_master(client, c, flags);
199 @@ -446,6 +471,7 @@ ProcXIChangeHierarchy(ClientPtr client)
201 xXIRemoveMasterInfo *r = (xXIRemoveMasterInfo *) any;
203 + CHANGE_SIZE_MATCH(xXIRemoveMasterInfo);
204 rc = remove_master(client, r, flags);
207 @@ -455,6 +481,7 @@ ProcXIChangeHierarchy(ClientPtr client)
209 xXIDetachSlaveInfo *c = (xXIDetachSlaveInfo *) any;
211 + CHANGE_SIZE_MATCH(xXIDetachSlaveInfo);
212 rc = detach_slave(client, c, flags);
215 @@ -464,6 +491,7 @@ ProcXIChangeHierarchy(ClientPtr client)
217 xXIAttachSlaveInfo *c = (xXIAttachSlaveInfo *) any;
219 + CHANGE_SIZE_MATCH(xXIAttachSlaveInfo);
220 rc = attach_slave(client, c, flags);
223 @@ -471,6 +499,7 @@ ProcXIChangeHierarchy(ClientPtr client)
227 + len -= any->length * 4;
228 any = (xXIAnyHierarchyChangeInfo *) ((char *) any + any->length * 4);
231 diff --git a/Xi/xigetclientpointer.c b/Xi/xigetclientpointer.c
232 index 3c90d58..306dd39 100644
233 --- a/Xi/xigetclientpointer.c
234 +++ b/Xi/xigetclientpointer.c
235 @@ -50,6 +50,7 @@ int
236 SProcXIGetClientPointer(ClientPtr client)
238 REQUEST(xXIGetClientPointerReq);
239 + REQUEST_SIZE_MATCH(xXIGetClientPointerReq);
241 swaps(&stuff->length);
243 diff --git a/Xi/xigrabdev.c b/Xi/xigrabdev.c
244 index 63d95bc..e2a2ae3 100644
247 @@ -47,6 +47,11 @@ int
248 SProcXIGrabDevice(ClientPtr client)
250 REQUEST(xXIGrabDeviceReq);
252 + * Check here for at least the length of the struct we swap, then
253 + * let ProcXIGrabDevice check the full size after we swap mask_len.
255 + REQUEST_AT_LEAST_SIZE(xXIGrabDeviceReq);
257 swaps(&stuff->length);
258 swaps(&stuff->deviceid);
259 @@ -71,7 +76,7 @@ ProcXIGrabDevice(ClientPtr client)
260 unsigned int pointer_mode;
262 REQUEST(xXIGrabDeviceReq);
263 - REQUEST_AT_LEAST_SIZE(xXIGrabDeviceReq);
264 + REQUEST_FIXED_SIZE(xXIGrabDeviceReq, ((size_t) stuff->mask_len) * 4);
266 ret = dixLookupDevice(&dev, stuff->deviceid, client, DixGrabAccess);
268 @@ -131,6 +136,7 @@ int
269 SProcXIUngrabDevice(ClientPtr client)
271 REQUEST(xXIUngrabDeviceReq);
272 + REQUEST_SIZE_MATCH(xXIUngrabDeviceReq);
274 swaps(&stuff->length);
275 swaps(&stuff->deviceid);
276 @@ -148,6 +154,7 @@ ProcXIUngrabDevice(ClientPtr client)
279 REQUEST(xXIUngrabDeviceReq);
280 + REQUEST_SIZE_MATCH(xXIUngrabDeviceReq);
282 ret = dixLookupDevice(&dev, stuff->deviceid, client, DixGetAttrAccess);
284 diff --git a/Xi/xipassivegrab.c b/Xi/xipassivegrab.c
285 index 700622d..9241ffd 100644
286 --- a/Xi/xipassivegrab.c
287 +++ b/Xi/xipassivegrab.c
288 @@ -53,6 +53,7 @@ SProcXIPassiveGrabDevice(ClientPtr client)
291 REQUEST(xXIPassiveGrabDeviceReq);
292 + REQUEST_AT_LEAST_SIZE(xXIPassiveGrabDeviceReq);
294 swaps(&stuff->length);
295 swaps(&stuff->deviceid);
296 @@ -63,6 +64,8 @@ SProcXIPassiveGrabDevice(ClientPtr client)
297 swaps(&stuff->mask_len);
298 swaps(&stuff->num_modifiers);
300 + REQUEST_FIXED_SIZE(xXIPassiveGrabDeviceReq,
301 + ((uint32_t) stuff->mask_len + stuff->num_modifiers) *4);
302 mods = (uint32_t *) &stuff[1] + stuff->mask_len;
304 for (i = 0; i < stuff->num_modifiers; i++, mods++) {
305 @@ -92,7 +95,8 @@ ProcXIPassiveGrabDevice(ClientPtr client)
308 REQUEST(xXIPassiveGrabDeviceReq);
309 - REQUEST_AT_LEAST_SIZE(xXIPassiveGrabDeviceReq);
310 + REQUEST_FIXED_SIZE(xXIPassiveGrabDeviceReq,
311 + ((uint32_t) stuff->mask_len + stuff->num_modifiers) * 4);
313 if (stuff->deviceid == XIAllDevices)
314 dev = inputInfo.all_devices;
315 @@ -252,6 +256,7 @@ SProcXIPassiveUngrabDevice(ClientPtr client)
318 REQUEST(xXIPassiveUngrabDeviceReq);
319 + REQUEST_AT_LEAST_SIZE(xXIPassiveUngrabDeviceReq);
321 swaps(&stuff->length);
322 swapl(&stuff->grab_window);
323 @@ -259,6 +264,8 @@ SProcXIPassiveUngrabDevice(ClientPtr client)
324 swapl(&stuff->detail);
325 swaps(&stuff->num_modifiers);
327 + REQUEST_FIXED_SIZE(xXIPassiveUngrabDeviceReq,
328 + ((uint32_t) stuff->num_modifiers) << 2);
329 modifiers = (uint32_t *) &stuff[1];
331 for (i = 0; i < stuff->num_modifiers; i++, modifiers++)
332 @@ -277,7 +284,8 @@ ProcXIPassiveUngrabDevice(ClientPtr client)
335 REQUEST(xXIPassiveUngrabDeviceReq);
336 - REQUEST_AT_LEAST_SIZE(xXIPassiveUngrabDeviceReq);
337 + REQUEST_FIXED_SIZE(xXIPassiveUngrabDeviceReq,
338 + ((uint32_t) stuff->num_modifiers) << 2);
340 if (stuff->deviceid == XIAllDevices)
341 dev = inputInfo.all_devices;
342 diff --git a/Xi/xiproperty.c b/Xi/xiproperty.c
343 index 463607d..8e8e4b0 100644
344 --- a/Xi/xiproperty.c
345 +++ b/Xi/xiproperty.c
346 @@ -1013,10 +1013,9 @@ int
347 SProcXListDeviceProperties(ClientPtr client)
349 REQUEST(xListDevicePropertiesReq);
350 + REQUEST_SIZE_MATCH(xListDevicePropertiesReq);
352 swaps(&stuff->length);
354 - REQUEST_SIZE_MATCH(xListDevicePropertiesReq);
355 return (ProcXListDeviceProperties(client));
358 @@ -1037,10 +1036,10 @@ int
359 SProcXDeleteDeviceProperty(ClientPtr client)
361 REQUEST(xDeleteDevicePropertyReq);
362 + REQUEST_SIZE_MATCH(xDeleteDevicePropertyReq);
364 swaps(&stuff->length);
365 swapl(&stuff->property);
366 - REQUEST_SIZE_MATCH(xDeleteDevicePropertyReq);
367 return (ProcXDeleteDeviceProperty(client));
370 @@ -1048,13 +1047,13 @@ int
371 SProcXGetDeviceProperty(ClientPtr client)
373 REQUEST(xGetDevicePropertyReq);
374 + REQUEST_SIZE_MATCH(xGetDevicePropertyReq);
376 swaps(&stuff->length);
377 swapl(&stuff->property);
379 swapl(&stuff->longOffset);
380 swapl(&stuff->longLength);
381 - REQUEST_SIZE_MATCH(xGetDevicePropertyReq);
382 return (ProcXGetDeviceProperty(client));
385 @@ -1253,11 +1252,10 @@ int
386 SProcXIListProperties(ClientPtr client)
388 REQUEST(xXIListPropertiesReq);
389 + REQUEST_SIZE_MATCH(xXIListPropertiesReq);
391 swaps(&stuff->length);
392 swaps(&stuff->deviceid);
394 - REQUEST_SIZE_MATCH(xXIListPropertiesReq);
395 return (ProcXIListProperties(client));
398 @@ -1279,11 +1277,11 @@ int
399 SProcXIDeleteProperty(ClientPtr client)
401 REQUEST(xXIDeletePropertyReq);
402 + REQUEST_SIZE_MATCH(xXIDeletePropertyReq);
404 swaps(&stuff->length);
405 swaps(&stuff->deviceid);
406 swapl(&stuff->property);
407 - REQUEST_SIZE_MATCH(xXIDeletePropertyReq);
408 return (ProcXIDeleteProperty(client));
411 @@ -1291,6 +1289,7 @@ int
412 SProcXIGetProperty(ClientPtr client)
414 REQUEST(xXIGetPropertyReq);
415 + REQUEST_SIZE_MATCH(xXIGetPropertyReq);
417 swaps(&stuff->length);
418 swaps(&stuff->deviceid);
419 @@ -1298,7 +1297,6 @@ SProcXIGetProperty(ClientPtr client)
421 swapl(&stuff->offset);
423 - REQUEST_SIZE_MATCH(xXIGetPropertyReq);
424 return (ProcXIGetProperty(client));
427 diff --git a/Xi/xiquerydevice.c b/Xi/xiquerydevice.c
428 index 4e544f0..67a9a4f 100644
429 --- a/Xi/xiquerydevice.c
430 +++ b/Xi/xiquerydevice.c
431 @@ -54,6 +54,7 @@ int
432 SProcXIQueryDevice(ClientPtr client)
434 REQUEST(xXIQueryDeviceReq);
435 + REQUEST_SIZE_MATCH(xXIQueryDeviceReq);
437 swaps(&stuff->length);
438 swaps(&stuff->deviceid);
439 diff --git a/Xi/xiquerypointer.c b/Xi/xiquerypointer.c
440 index e9bdd42..7ec0c85 100644
441 --- a/Xi/xiquerypointer.c
442 +++ b/Xi/xiquerypointer.c
443 @@ -63,6 +63,8 @@ int
444 SProcXIQueryPointer(ClientPtr client)
446 REQUEST(xXIQueryPointerReq);
447 + REQUEST_SIZE_MATCH(xXIQueryPointerReq);
449 swaps(&stuff->length);
450 swaps(&stuff->deviceid);
452 diff --git a/Xi/xiselectev.c b/Xi/xiselectev.c
453 index 45a996e..168579f 100644
454 --- a/Xi/xiselectev.c
455 +++ b/Xi/xiselectev.c
456 @@ -114,6 +114,7 @@ int
457 SProcXISelectEvents(ClientPtr client)
461 xXIEventMask *evmask;
463 REQUEST(xXISelectEventsReq);
464 @@ -122,10 +123,17 @@ SProcXISelectEvents(ClientPtr client)
466 swaps(&stuff->num_masks);
468 + len = stuff->length - bytes_to_int32(sizeof(xXISelectEventsReq));
469 evmask = (xXIEventMask *) &stuff[1];
470 for (i = 0; i < stuff->num_masks; i++) {
471 + if (len < bytes_to_int32(sizeof(xXIEventMask)))
473 + len -= bytes_to_int32(sizeof(xXIEventMask));
474 swaps(&evmask->deviceid);
475 swaps(&evmask->mask_len);
476 + if (len < evmask->mask_len)
478 + len -= evmask->mask_len;
480 (xXIEventMask *) (((char *) &evmask[1]) + evmask->mask_len * 4);
482 diff --git a/Xi/xisetclientpointer.c b/Xi/xisetclientpointer.c
483 index 38ff51e..24d4a53 100644
484 --- a/Xi/xisetclientpointer.c
485 +++ b/Xi/xisetclientpointer.c
486 @@ -51,10 +51,11 @@ int
487 SProcXISetClientPointer(ClientPtr client)
489 REQUEST(xXISetClientPointerReq);
490 + REQUEST_SIZE_MATCH(xXISetClientPointerReq);
492 swaps(&stuff->length);
494 swaps(&stuff->deviceid);
495 - REQUEST_SIZE_MATCH(xXISetClientPointerReq);
496 return (ProcXISetClientPointer(client));
499 diff --git a/Xi/xisetdevfocus.c b/Xi/xisetdevfocus.c
500 index 372ec24..96a9a16 100644
501 --- a/Xi/xisetdevfocus.c
502 +++ b/Xi/xisetdevfocus.c
503 @@ -44,6 +44,8 @@ int
504 SProcXISetFocus(ClientPtr client)
506 REQUEST(xXISetFocusReq);
507 + REQUEST_AT_LEAST_SIZE(xXISetFocusReq);
509 swaps(&stuff->length);
510 swaps(&stuff->deviceid);
511 swapl(&stuff->focus);
512 @@ -56,6 +58,8 @@ int
513 SProcXIGetFocus(ClientPtr client)
515 REQUEST(xXIGetFocusReq);
516 + REQUEST_AT_LEAST_SIZE(xXIGetFocusReq);
518 swaps(&stuff->length);
519 swaps(&stuff->deviceid);
521 diff --git a/Xi/xiwarppointer.c b/Xi/xiwarppointer.c
522 index 3f051f7..780758a 100644
523 --- a/Xi/xiwarppointer.c
524 +++ b/Xi/xiwarppointer.c
525 @@ -56,6 +56,8 @@ int
526 SProcXIWarpPointer(ClientPtr client)
528 REQUEST(xXIWarpPointerReq);
529 + REQUEST_SIZE_MATCH(xXIWarpPointerReq);
531 swaps(&stuff->length);
532 swapl(&stuff->src_win);
533 swapl(&stuff->dst_win);
534 diff --git a/include/dix.h b/include/dix.h
535 index e0c6ed8..21176a8 100644
538 @@ -74,6 +74,10 @@ SOFTWARE.
539 if ((sizeof(req) >> 2) > client->req_len )\
542 +#define REQUEST_AT_LEAST_EXTRA_SIZE(req, extra) \
543 + if (((sizeof(req) + ((uint64_t) extra)) >> 2) > client->req_len ) \
546 #define REQUEST_FIXED_SIZE(req, n)\
547 if (((sizeof(req) >> 2) > client->req_len) || \
548 ((n >> 2) >= client->req_len) || \