From d7258444a876a65986212c10ddcaa1783af558bf Mon Sep 17 00:00:00 2001
From: Olivier Fourdan <ofourdan@redhat.com>
Date: Fri, 16 Jan 2015 08:44:45 +0100
Subject: [PATCH 4/4] 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.
v3: backport to nx-libs 3.6.x because this is
the CVE-2015-0255 fix (Mike DePaulo)
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>
(cherry picked from commit 20079c36cf7d377938ca5478447d8b9045cb7d43)
(cherry picked from commit f160e722672dbb2b5215870b47bcc51461d96ff1)
Signed-off-by: Julien Cristau <jcristau@debian.org>
---
nx-X11/programs/Xserver/xkb/xkb.c | 66 ++++++++++++++++++++++++---------------
1 file changed, 41 insertions(+), 25 deletions(-)
--- a/nx-X11/programs/Xserver/xkb/xkb.c
+++ b/nx-X11/programs/Xserver/xkb/xkb.c
@@ -4437,26 +4437,30 @@ 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) {
register int n;
swaps(&len, n);
}
- str= (char *)_XkbAlloc(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
@@ -4470,6 +4474,7 @@ xkbDoodadWireDesc * dWire;
xkbAnyDoodadWireDesc any;
xkbTextDoodadWireDesc text;
XkbDoodadPtr doodad;
+Status status;
dWire= (xkbDoodadWireDesc *)(*wire_inout);
any = dWire->any;
@@ -4521,8 +4526,14 @@ XkbDoodadPtr doodad;
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) {
@@ -4557,7 +4568,9 @@ XkbDoodadPtr doodad;
}
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);
@@ -4792,17 +4805,19 @@ Status status;
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) {
xfree(name);
- return BadAlloc;
+ return status;
}
if (XkbAddGeomProperty(geom,name,val)==NULL) {
xfree(name);
@@ -4833,9 +4848,10 @@ char * wire;
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)) {
xfree(name);
return BadAlloc;