0003-dix-Disallow-GenericEvent-in-SendEvent-request.patch 2.9 KB

1234567891011121314151617181920212223242526272829303132333435363738394041424344454647484950515253545556575859606162636465666768697071
  1. From 215f894965df5fb0bb45b107d84524e700d2073c Mon Sep 17 00:00:00 2001
  2. From: Michal Srb <msrb@suse.com>
  3. Date: Wed, 24 May 2017 15:54:40 +0300
  4. Subject: [PATCH] dix: Disallow GenericEvent in SendEvent request.
  5. The SendEvent request holds xEvent which is exactly 32 bytes long, no more,
  6. no less. Both ProcSendEvent and SProcSendEvent verify that the received data
  7. exactly match the request size. However nothing stops the client from passing
  8. in event with xEvent::type = GenericEvent and any value of
  9. xGenericEvent::length.
  10. In the case of ProcSendEvent, the event will be eventually passed to
  11. WriteEventsToClient which will see that it is Generic event and copy the
  12. arbitrary length from the receive buffer (and possibly past it) and send it to
  13. the other client. This allows clients to copy unitialized heap memory out of X
  14. server or to crash it.
  15. In case of SProcSendEvent, it will attempt to swap the incoming event by
  16. calling a swapping function from the EventSwapVector array. The swapped event
  17. is written to target buffer, which in this case is local xEvent variable. The
  18. xEvent variable is 32 bytes long, but the swapping functions for GenericEvents
  19. expect that the target buffer has size matching the size of the source
  20. GenericEvent. This allows clients to cause stack buffer overflows.
  21. Signed-off-by: Michal Srb <msrb@suse.com>
  22. Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
  23. Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
  24. Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
  25. ---
  26. dix/events.c | 6 ++++++
  27. dix/swapreq.c | 7 +++++++
  28. 2 files changed, 13 insertions(+)
  29. diff --git a/dix/events.c b/dix/events.c
  30. index 3e3a01ef9..d3a33ea3f 100644
  31. --- a/dix/events.c
  32. +++ b/dix/events.c
  33. @@ -5366,6 +5366,12 @@ ProcSendEvent(ClientPtr client)
  34. client->errorValue = stuff->event.u.u.type;
  35. return BadValue;
  36. }
  37. + /* Generic events can have variable size, but SendEvent request holds
  38. + exactly 32B of event data. */
  39. + if (stuff->event.u.u.type == GenericEvent) {
  40. + client->errorValue = stuff->event.u.u.type;
  41. + return BadValue;
  42. + }
  43. if (stuff->event.u.u.type == ClientMessage &&
  44. stuff->event.u.u.detail != 8 &&
  45. stuff->event.u.u.detail != 16 && stuff->event.u.u.detail != 32) {
  46. diff --git a/dix/swapreq.c b/dix/swapreq.c
  47. index 719e9b81c..67850593b 100644
  48. --- a/dix/swapreq.c
  49. +++ b/dix/swapreq.c
  50. @@ -292,6 +292,13 @@ SProcSendEvent(ClientPtr client)
  51. swapl(&stuff->destination);
  52. swapl(&stuff->eventMask);
  53. + /* Generic events can have variable size, but SendEvent request holds
  54. + exactly 32B of event data. */
  55. + if (stuff->event.u.u.type == GenericEvent) {
  56. + client->errorValue = stuff->event.u.u.type;
  57. + return BadValue;
  58. + }
  59. +
  60. /* Swap event */
  61. proc = EventSwapVector[stuff->event.u.u.type & 0177];
  62. if (!proc || proc == NotImplemented) /* no swapping proc; invalid event type? */
  63. --
  64. 2.11.0