xorg-server: Security Advisory - xorg-server - CVE-2015-0255

Updated x11-server packages fix security vulnerability:

Olivier Fourdan from Red Hat has discovered a protocol handling issue in
the way the X server code base handles the XkbSetGeometry request, where
the server trusts the client to send valid string lengths. A malicious
client with string lengths exceeding the request length can cause the server
to copy adjacent memory data into the XKB structs. This data is then
available to the client via the XkbGetGeometry request. This can lead to
information disclosure issues, as well as possibly a denial of service if a
similar request can cause the server to crash (CVE-2015-0255).

(From OE-Core rev: 564e2f9732ac4318bb5923dd1ff771514c9afd2f)

Signed-off-by: Li Zhou <li.zhou@windriver.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
This commit is contained in:
Li Zhou
2015-04-07 15:49:56 +08:00
committed by Richard Purdie
parent 6cbfe0f354
commit 77a44c253b
3 changed files with 256 additions and 0 deletions

View File

@@ -0,0 +1,145 @@
From 20079c36cf7d377938ca5478447d8b9045cb7d43 Mon Sep 17 00:00:00 2001
From: Olivier Fourdan <ofourdan@redhat.com>
Date: Fri, 16 Jan 2015 08:44:45 +0100
Subject: [PATCH] xkb: Check strings length against request size
Ensure that the given strings length in an XkbSetGeometry request remain
within the limits of the size of the request.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Upstream-Status: backport
Signed-off-by: Li Zhou <li.zhou@windriver.com>
---
xkb/xkb.c | 65 +++++++++++++++++++++++++++++++++++++------------------------
1 file changed, 40 insertions(+), 25 deletions(-)
diff --git a/xkb/xkb.c b/xkb/xkb.c
index b9a3ac4..f3988f9 100644
--- a/xkb/xkb.c
+++ b/xkb/xkb.c
@@ -4957,25 +4957,29 @@ ProcXkbGetGeometry(ClientPtr client)
/***====================================================================***/
-static char *
-_GetCountedString(char **wire_inout, Bool swap)
+static Status
+_GetCountedString(char **wire_inout, ClientPtr client, char **str)
{
- char *wire, *str;
+ char *wire, *next;
CARD16 len;
wire = *wire_inout;
len = *(CARD16 *) wire;
- if (swap) {
+ if (client->swapped) {
swaps(&len);
}
- str = malloc(len + 1);
- if (str) {
- memcpy(str, &wire[2], len);
- str[len] = '\0';
- }
- wire += XkbPaddedSize(len + 2);
- *wire_inout = wire;
- return str;
+ next = wire + XkbPaddedSize(len + 2);
+ /* Check we're still within the size of the request */
+ if (client->req_len <
+ bytes_to_int32(next - (char *) client->requestBuffer))
+ return BadValue;
+ *str = malloc(len + 1);
+ if (!*str)
+ return BadAlloc;
+ memcpy(*str, &wire[2], len);
+ *(*str + len) = '\0';
+ *wire_inout = next;
+ return Success;
}
static Status
@@ -4987,6 +4991,7 @@ _CheckSetDoodad(char **wire_inout,
xkbAnyDoodadWireDesc any;
xkbTextDoodadWireDesc text;
XkbDoodadPtr doodad;
+ Status status;
dWire = (xkbDoodadWireDesc *) (*wire_inout);
any = dWire->any;
@@ -5036,8 +5041,14 @@ _CheckSetDoodad(char **wire_inout,
doodad->text.width = text.width;
doodad->text.height = text.height;
doodad->text.color_ndx = dWire->text.colorNdx;
- doodad->text.text = _GetCountedString(&wire, client->swapped);
- doodad->text.font = _GetCountedString(&wire, client->swapped);
+ status = _GetCountedString(&wire, client, &doodad->text.text);
+ if (status != Success)
+ return status;
+ status = _GetCountedString(&wire, client, &doodad->text.font);
+ if (status != Success) {
+ free (doodad->text.text);
+ return status;
+ }
break;
case XkbIndicatorDoodad:
if (dWire->indicator.onColorNdx >= geom->num_colors) {
@@ -5072,7 +5083,9 @@ _CheckSetDoodad(char **wire_inout,
}
doodad->logo.color_ndx = dWire->logo.colorNdx;
doodad->logo.shape_ndx = dWire->logo.shapeNdx;
- doodad->logo.logo_name = _GetCountedString(&wire, client->swapped);
+ status = _GetCountedString(&wire, client, &doodad->logo.logo_name);
+ if (status != Success)
+ return status;
break;
default:
client->errorValue = _XkbErrCode2(0x4F, dWire->any.type);
@@ -5304,18 +5317,20 @@ _CheckSetGeom(XkbGeometryPtr geom, xkbSetGeometryReq * req, ClientPtr client)
char *wire;
wire = (char *) &req[1];
- geom->label_font = _GetCountedString(&wire, client->swapped);
+ status = _GetCountedString(&wire, client, &geom->label_font);
+ if (status != Success)
+ return status;
for (i = 0; i < req->nProperties; i++) {
char *name, *val;
- name = _GetCountedString(&wire, client->swapped);
- if (!name)
- return BadAlloc;
- val = _GetCountedString(&wire, client->swapped);
- if (!val) {
+ status = _GetCountedString(&wire, client, &name);
+ if (status != Success)
+ return status;
+ status = _GetCountedString(&wire, client, &val);
+ if (status != Success) {
free(name);
- return BadAlloc;
+ return status;
}
if (XkbAddGeomProperty(geom, name, val) == NULL) {
free(name);
@@ -5349,9 +5364,9 @@ _CheckSetGeom(XkbGeometryPtr geom, xkbSetGeometryReq * req, ClientPtr client)
for (i = 0; i < req->nColors; i++) {
char *name;
- name = _GetCountedString(&wire, client->swapped);
- if (!name)
- return BadAlloc;
+ status = _GetCountedString(&wire, client, &name);
+ if (status != Success)
+ return status;
if (!XkbAddGeomColor(geom, name, geom->num_colors)) {
free(name);
return BadAlloc;
--
1.7.9.5

View File

@@ -0,0 +1,109 @@
From 81c90dc8f0aae3b65730409b1b615b5fa7280ebd Mon Sep 17 00:00:00 2001
From: Olivier Fourdan <ofourdan@redhat.com>
Date: Fri, 16 Jan 2015 20:08:59 +0100
Subject: [PATCH] xkb: Don't swap XkbSetGeometry data in the input buffer
The XkbSetGeometry request embeds data which needs to be swapped when the
server and the client have different endianess.
_XkbSetGeometry() invokes functions that swap these data directly in the
input buffer.
However, ProcXkbSetGeometry() may call _XkbSetGeometry() more than once
(if there is more than one keyboard), thus causing on swapped clients the
same data to be swapped twice in memory, further causing a server crash
because the strings lengths on the second time are way off bounds.
To allow _XkbSetGeometry() to run reliably more than once with swapped
clients, do not swap the data in the buffer, use variables instead.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Upstream-Status: backport
Signed-off-by: Li Zhou <li.zhou@windriver.com>
---
xkb/xkb.c | 35 +++++++++++++++++++----------------
1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/xkb/xkb.c b/xkb/xkb.c
index 15c7f34..b9a3ac4 100644
--- a/xkb/xkb.c
+++ b/xkb/xkb.c
@@ -4961,14 +4961,13 @@ static char *
_GetCountedString(char **wire_inout, Bool swap)
{
char *wire, *str;
- CARD16 len, *plen;
+ CARD16 len;
wire = *wire_inout;
- plen = (CARD16 *) wire;
+ len = *(CARD16 *) wire;
if (swap) {
- swaps(plen);
+ swaps(&len);
}
- len = *plen;
str = malloc(len + 1);
if (str) {
memcpy(str, &wire[2], len);
@@ -4985,25 +4984,28 @@ _CheckSetDoodad(char **wire_inout,
{
char *wire;
xkbDoodadWireDesc *dWire;
+ xkbAnyDoodadWireDesc any;
+ xkbTextDoodadWireDesc text;
XkbDoodadPtr doodad;
dWire = (xkbDoodadWireDesc *) (*wire_inout);
+ any = dWire->any;
wire = (char *) &dWire[1];
if (client->swapped) {
- swapl(&dWire->any.name);
- swaps(&dWire->any.top);
- swaps(&dWire->any.left);
- swaps(&dWire->any.angle);
+ swapl(&any.name);
+ swaps(&any.top);
+ swaps(&any.left);
+ swaps(&any.angle);
}
CHK_ATOM_ONLY(dWire->any.name);
- doodad = XkbAddGeomDoodad(geom, section, dWire->any.name);
+ doodad = XkbAddGeomDoodad(geom, section, any.name);
if (!doodad)
return BadAlloc;
doodad->any.type = dWire->any.type;
doodad->any.priority = dWire->any.priority;
- doodad->any.top = dWire->any.top;
- doodad->any.left = dWire->any.left;
- doodad->any.angle = dWire->any.angle;
+ doodad->any.top = any.top;
+ doodad->any.left = any.left;
+ doodad->any.angle = any.angle;
switch (doodad->any.type) {
case XkbOutlineDoodad:
case XkbSolidDoodad:
@@ -5026,12 +5028,13 @@ _CheckSetDoodad(char **wire_inout,
dWire->text.colorNdx);
return BadMatch;
}
+ text = dWire->text;
if (client->swapped) {
- swaps(&dWire->text.width);
- swaps(&dWire->text.height);
+ swaps(&text.width);
+ swaps(&text.height);
}
- doodad->text.width = dWire->text.width;
- doodad->text.height = dWire->text.height;
+ doodad->text.width = text.width;
+ doodad->text.height = text.height;
doodad->text.color_ndx = dWire->text.colorNdx;
doodad->text.text = _GetCountedString(&wire, client->swapped);
doodad->text.font = _GetCountedString(&wire, client->swapped);
--
1.7.9.5

View File

@@ -6,6 +6,8 @@ SRC_URI += "file://fix_open_max_preprocessor_error.patch \
file://xshmfence-option.patch \
file://Fix-subwindow-in-Xi-emulated-events.patch \
file://xtrans.patch \
file://0001-xkb-Don-t-swap-XkbSetGeometry-data-in-the-input-buff.patch \
file://0001-xkb-Check-strings-length-against-request-size.patch \
"
SRC_URI[md5sum] = "afd93977235584a9caa7528a737c1b52"