From 21f559038c8776acc6439faadbdcab7df4300c66 Mon Sep 17 00:00:00 2001 From: Michal Srb Date: Wed, 24 May 2017 15:54:40 +0300 Subject: 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 Reviewed-by: Peter Hutterer Signed-off-by: Peter Hutterer (cherry picked from commit 215f894965df5fb0bb45b107d84524e700d2073c) --- dix/events.c | 6 ++++++ dix/swapreq.c | 7 +++++++ 2 files changed, 13 insertions(+) (limited to 'dix') diff --git a/dix/events.c b/dix/events.c index cc26ba5db..3faad53a8 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 61d3ce0f4..8cc64b6ed 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? */ -- cgit v1.2.3