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