| 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 |
| |