Xi/sendexev.c | 24 +++++++++++++++++-------
debian/changelog | 14 ++++++++++++++
dix/events.c | 6 ++++++
dix/swapreq.c | 7 +++++++
4 files changed, 44 insertions(+), 7 deletions(-)
New commits:
commit d7bc00f578020e207a4e2f394ca798ded7651ec2
Author: Julien Cristau <jcris...@debian.org>
Date: Fri Jul 7 07:31:13 2017 +0200
Changelog update
diff --git a/debian/changelog b/debian/changelog
index 4414ff8..4aa0758 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,17 @@
+xorg-server (2:1.19.3-2) unstable; urgency=high
+
+ * CVE-2017-10972: information leak out of the X server due to an
+ uninitialized stack area when swapping:
+ - Xi: Zero target buffer in SProcXSendExtensionEvent
+ * CVE-2017-10971: stack overflow due to missing GenericEvent handling in
+ XSendEvent:
+ - dix: Disallow GenericEvent in SendEvent request
+ - Xi: Verify all events in ProcXSendExtensionEvent
+ - Xi: Do not try to swap GenericEvent
+ * With both those fixes, this closes: #867492
+
+ -- Julien Cristau <jcris...@debian.org> Fri, 07 Jul 2017 07:31:11 +0200
+
xorg-server (2:1.19.3-1) unstable; urgency=medium
* New upstream release.
commit e3f9ab63d89af38d96be06d923d969e5ba8c7b82
Author: Michal Srb <m...@suse.com>
Date: Wed May 24 15:54:42 2017 +0300
Xi: Do not try to swap GenericEvent.
The SProcXSendExtensionEvent must not attempt to swap GenericEvent because
it is assuming that the event has fixed size and gives the swapping function
xEvent-sized buffer.
A GenericEvent would be later rejected by ProcXSendExtensionEvent anyway.
Signed-off-by: Michal Srb <m...@suse.com>
Reviewed-by: Peter Hutterer <peter.hutte...@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
(cherry picked from commit ba336b24052122b136486961c82deac76bbde455)
diff --git a/Xi/sendexev.c b/Xi/sendexev.c
index 365c791..5ecc228 100644
--- a/Xi/sendexev.c
+++ b/Xi/sendexev.c
@@ -95,9 +95,17 @@ SProcXSendExtensionEvent(ClientPtr client)
eventP = (xEvent *) &stuff[1];
for (i = 0; i < stuff->num_events; i++, eventP++) {
+ if (eventP->u.u.type == GenericEvent) {
+ client->errorValue = eventP->u.u.type;
+ return BadValue;
+ }
+
proc = EventSwapVector[eventP->u.u.type & 0177];
- if (proc == NotImplemented) /* no swapping proc; invalid event
type? */
+ /* no swapping proc; invalid event type? */
+ if (proc == NotImplemented) {
+ client->errorValue = eventP->u.u.type;
return BadValue;
+ }
(*proc) (eventP, &eventT);
*eventP = eventT;
}
commit 877d94702190b92a39dea2d4b76f41df1c1f9b0c
Author: Michal Srb <m...@suse.com>
Date: Wed May 24 15:54:41 2017 +0300
Xi: Verify all events in ProcXSendExtensionEvent.
The requirement is that events have type in range
EXTENSION_EVENT_BASE..lastEvent, but it was tested
only for first event of all.
Signed-off-by: Michal Srb <m...@suse.com>
Reviewed-by: Peter Hutterer <peter.hutte...@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
(cherry picked from commit 8caed4df36b1f802b4992edcfd282cbeeec35d9d)
diff --git a/Xi/sendexev.c b/Xi/sendexev.c
index c9b7dde..365c791 100644
--- a/Xi/sendexev.c
+++ b/Xi/sendexev.c
@@ -117,7 +117,7 @@ SProcXSendExtensionEvent(ClientPtr client)
int
ProcXSendExtensionEvent(ClientPtr client)
{
- int ret;
+ int ret, i;
DeviceIntPtr dev;
xEvent *first;
XEventClass *list;
@@ -141,10 +141,12 @@ ProcXSendExtensionEvent(ClientPtr client)
/* The client's event type must be one defined by an extension. */
first = ((xEvent *) &stuff[1]);
- if (!((EXTENSION_EVENT_BASE <= first->u.u.type) &&
- (first->u.u.type < lastEvent))) {
- client->errorValue = first->u.u.type;
- return BadValue;
+ for (i = 0; i < stuff->num_events; i++) {
+ if (!((EXTENSION_EVENT_BASE <= first[i].u.u.type) &&
+ (first[i].u.u.type < lastEvent))) {
+ client->errorValue = first[i].u.u.type;
+ return BadValue;
+ }
}
list = (XEventClass *) (first + stuff->num_events);
commit 339d961d2a60531452f5ef4421d652b8d0afc4ff
Author: Michal Srb <m...@suse.com>
Date: Wed May 24 15:54:40 2017 +0300
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 <m...@suse.com>
Reviewed-by: Peter Hutterer <peter.hutte...@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
(cherry picked from commit 215f894965df5fb0bb45b107d84524e700d2073c)
diff --git a/dix/events.c b/dix/events.c
index cc26ba5..3faad53 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 61d3ce0..8cc64b6 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? */
commit 8e627350db3e7c22d2cec2e886e90b7057728933
Author: Michal Srb <m...@suse.com>
Date: Wed May 24 15:54:39 2017 +0300
Xi: Zero target buffer in SProcXSendExtensionEvent.
Make sure that the xEvent eventT is initialized with zeros, the same way as
in SProcSendEvent.
Some event swapping functions do not overwrite all 32 bytes of xEvent
structure, for example XSecurityAuthorizationRevoked. Two cooperating
clients, one swapped and the other not, can send
XSecurityAuthorizationRevoked event to each other to retrieve old stack data
from X server. This can be potentialy misused to go around ASLR or
stack-protector.
Signed-off-by: Michal Srb <m...@suse.com>
Reviewed-by: Peter Hutterer <peter.hutte...@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
(cherry picked from commit 05442de962d3dc624f79fc1a00eca3ffc5489ced)
diff --git a/Xi/sendexev.c b/Xi/sendexev.c
index 183f88d..c9b7dde 100644
--- a/Xi/sendexev.c
+++ b/Xi/sendexev.c
@@ -78,7 +78,7 @@ SProcXSendExtensionEvent(ClientPtr client)
{
CARD32 *p;
int i;
- xEvent eventT;
+ xEvent eventT = { .u.u.type = 0 };
xEvent *eventP;
EventSwapPtr proc;
