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>
This commit is contained in:
Michal Srb 2017-05-24 15:54:40 +03:00 committed by Peter Hutterer
parent 05442de962
commit 215f894965
2 changed files with 13 additions and 0 deletions

View File

@ -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) {

View File

@ -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? */