This is an automated email from the git hooks/post-receive script. x2go pushed a commit to branch 3.5.0.x in repository nx-libs. commit 2b2a02f93f552a38de8f72a971fa3f3ff42b3298 Author: Mike DePaulo <mikedep333@gmail.com> Date: Wed Feb 18 07:43:20 2015 -0500 X.org CVE-2015-0255 patch and its 3 prereq patches 1101-Coverity-844-845-846-Fix-memory-leaks.patch 1102-include-introduce-byte-counting-functions.patch 1103-xkb-Don-t-swap-XkbSetGeometry-data-in-the-input-buff.patc 1104-xkb-Check-strings-length-against-request-size.patch (The last patch is the CVE-2015-0255 patch.) --- debian/changelog | 8 ++ ...101-Coverity-844-845-846-Fix-memory-leaks.patch | 63 +++++++++ ...include-introduce-byte-counting-functions.patch | 70 +++++++++ ...wap-XkbSetGeometry-data-in-the-input-buff.patch | 115 +++++++++++++++ ...Check-strings-length-against-request-size.patch | 148 ++++++++++++++++++++ debian/patches/series | 4 + 6 files changed, 408 insertions(+) diff --git a/debian/changelog b/debian/changelog index 05a86f4..c9dd06d 100644 --- a/debian/changelog +++ b/debian/changelog @@ -190,10 +190,18 @@ nx-libs (2:3.5.0.29-0x2go2) UNRELEASED; urgency=medium 1038-glx-Length-checking-for-non-generated-single-request.patch 1039-glx-Length-checking-for-RenderLarge-requests-v2-CVE-.patch 1040-glx-Pass-remaining-request-length-into-varsize-v2-CV.patch + - X.org CVE-2015-0255 + 1104-xkb-Check-strings-length-against-request-size.patch - Security fixes with no assigned CVE: 1008-Don-t-crash-when-we-receive-an-FS_Error-from-the-fon.patch + - Rebase the following patches that are prerequisites for the + CVE-2015-0255 patch: + 1101-Coverity-844-845-846-Fix-memory-leaks.patch + 1102-include-introduce-byte-counting-functions.patch + 1103-xkb-Don-t-swap-XkbSetGeometry-data-in-the-input-buff.patch + - Fix FTBFS due to the nxproxy executable already existing under /usr/lib/nx/bin/nx/ diff --git a/debian/patches/1101-Coverity-844-845-846-Fix-memory-leaks.patch b/debian/patches/1101-Coverity-844-845-846-Fix-memory-leaks.patch new file mode 100644 index 0000000..e373bae --- /dev/null +++ b/debian/patches/1101-Coverity-844-845-846-Fix-memory-leaks.patch @@ -0,0 +1,63 @@ +From d6ce946f9c0bb5746b8333b3f589aa8527739431 Mon Sep 17 00:00:00 2001 +From: Daniel Stone <daniel@fooishbar.org> +Date: Fri, 7 Apr 2006 16:07:50 +0000 +Subject: [PATCH 1/4] Coverity #844, #845, #846: Fix memory leaks. + +v2: backport to nx-libs 3.6.x as a prereq for +the CVE-2015-0255 fix (Mike DePaulo) +--- + nx-X11/programs/Xserver/xkb/xkb.c | 22 +++++++++++++++++++--- + 1 file changed, 19 insertions(+), 3 deletions(-) + +diff --git a/nx-X11/programs/Xserver/xkb/xkb.c b/nx-X11/programs/Xserver/xkb/xkb.c +index 2405090..2561c89 100644 +--- a/nx-X11/programs/Xserver/xkb/xkb.c ++++ b/nx-X11/programs/Xserver/xkb/xkb.c +@@ -4794,9 +4794,20 @@ char * wire; + 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 ((!name)||(!val)||(XkbAddGeomProperty(geom,name,val)==NULL)) ++ if (!val) { ++ xfree(name); ++ return BadAlloc; ++ } ++ if (XkbAddGeomProperty(geom,name,val)==NULL) { ++ xfree(name); ++ xfree(val); + return BadAlloc; ++ } ++ xfree(name); ++ xfree(val); + } + + if (req->nColors<2) { +@@ -4813,15 +4824,20 @@ char * wire; + } + if (req->labelColorNdx==req->baseColorNdx) { + client->errorValue= _XkbErrCode3(0x04,req->baseColorNdx, +- req->labelColorNdx); ++ req->labelColorNdx); + return BadMatch; + } + + for (i=0;i<req->nColors;i++) { + char *name; + name= _GetCountedString(&wire,client->swapped); +- if ((!name)||(!XkbAddGeomColor(geom,name,geom->num_colors))) ++ if (!name) ++ return BadAlloc; ++ if (!XkbAddGeomColor(geom,name,geom->num_colors)) { ++ xfree(name); + return BadAlloc; ++ } ++ xfree(name); + } + if (req->nColors!=geom->num_colors) { + client->errorValue= _XkbErrCode3(0x05,req->nColors,geom->num_colors); +-- +1.9.1 + diff --git a/debian/patches/1102-include-introduce-byte-counting-functions.patch b/debian/patches/1102-include-introduce-byte-counting-functions.patch new file mode 100644 index 0000000..eb6366a --- /dev/null +++ b/debian/patches/1102-include-introduce-byte-counting-functions.patch @@ -0,0 +1,70 @@ +From 3937db18a203f9936387286b95328f27013a5ffe Mon Sep 17 00:00:00 2001 +From: Peter Hutterer <peter.hutterer@who-t.net> +Date: Mon, 29 Jun 2009 13:09:57 +1000 +Subject: [PATCH 2/4] include: introduce byte counting functions. + +This patch adds the following three functions: + bits_to_bytes(bits) - the number of bytes needed to hold 'bits' + bytes_to_int32(bytes) - the number of 4-byte units to hold 'bytes' + pad_to_int32(bytes) - the closest multiple of 4 equal to or larger than + 'bytes'. + +All three operations are common in protocol processing and currently the +server has ((foo + 7)/8 + 3)/4 operations all over the place. A common set +of functions reduce the error rate of these (albeit simple) calculations and +improve readability of the code. + +The functions do not check for overflow. + +v2: backport to nx-libs 3.6.x as a prereq for +the CVE-2015-0255 fix (Mike DePaulo) + +Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net> +--- + nx-X11/programs/Xserver/include/misc.h | 30 ++++++++++++++++++++++++++++++ + 1 file changed, 30 insertions(+) + +diff --git a/nx-X11/programs/Xserver/include/misc.h b/nx-X11/programs/Xserver/include/misc.h +index 5944a42..849f1b5 100644 +--- a/nx-X11/programs/Xserver/include/misc.h ++++ b/nx-X11/programs/Xserver/include/misc.h +@@ -193,6 +193,36 @@ typedef struct _xReq *xReqPtr; + + #endif + ++/** ++ * Calculate the number of bytes needed to hold bits. ++ * @param bits The minimum number of bits needed. ++ * @return The number of bytes needed to hold bits. ++ */ ++static __inline__ int ++bits_to_bytes(const int bits) { ++ return ((bits + 7) >> 3); ++} ++/** ++ * Calculate the number of 4-byte units needed to hold the given number of ++ * bytes. ++ * @param bytes The minimum number of bytes needed. ++ * @return The number of 4-byte units needed to hold bytes. ++ */ ++static __inline__ int ++bytes_to_int32(const int bytes) { ++ return (((bytes) + 3) >> 2); ++} ++ ++/** ++ * Calculate the number of bytes (in multiples of 4) needed to hold bytes. ++ * @param bytes The minimum number of bytes needed. ++ * @return The closest multiple of 4 that is equal or higher than bytes. ++ */ ++static __inline__ int ++pad_to_int32(const int bytes) { ++ return (((bytes) + 3) & ~3); ++} ++ + /* some macros to help swap requests, replies, and events */ + + #define LengthRestB(stuff) \ +-- +1.9.1 + diff --git a/debian/patches/1103-xkb-Don-t-swap-XkbSetGeometry-data-in-the-input-buff.patch b/debian/patches/1103-xkb-Don-t-swap-XkbSetGeometry-data-in-the-input-buff.patch new file mode 100644 index 0000000..328853a --- /dev/null +++ b/debian/patches/1103-xkb-Don-t-swap-XkbSetGeometry-data-in-the-input-buff.patch @@ -0,0 +1,115 @@ +From 9308c79ba2757cb1a64e0040176b8290b435544f Mon Sep 17 00:00:00 2001 +From: Olivier Fourdan <ofourdan@redhat.com> +Date: Fri, 16 Jan 2015 20:08:59 +0100 +Subject: [PATCH 3/4] 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. + +v3: backport to nx-libs 3.6.x as a prereq for +the CVE-2015-0255 fix (Mike DePaulo) + +Signed-off-by: Olivier Fourdan <ofourdan@redhat.com> +Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net> +(cherry picked from commit 81c90dc8f0aae3b65730409b1b615b5fa7280ebd) +(cherry picked from commit 29be310c303914090298ddda93a5bd5d00a94945) +Signed-off-by: Julien Cristau <jcristau@debian.org> +index 2405090..7db0959 100644 +--- + nx-X11/programs/Xserver/xkb/xkb.c | 35 +++++++++++++++++++---------------- + 1 file changed, 19 insertions(+), 16 deletions(-) + +diff --git a/nx-X11/programs/Xserver/xkb/xkb.c b/nx-X11/programs/Xserver/xkb/xkb.c +index 2561c89..d8b5b2c 100644 +--- a/nx-X11/programs/Xserver/xkb/xkb.c ++++ b/nx-X11/programs/Xserver/xkb/xkb.c +@@ -4441,15 +4441,14 @@ 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) { + register int n; +- swaps(plen,n); ++ swaps(&len, n); + } +- len= *plen; + str= (char *)_XkbAlloc(len+1); + if (str) { + memcpy(str,&wire[2],len); +@@ -4468,26 +4467,29 @@ _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) { + register int n; +- swapl(&dWire->any.name,n); +- swaps(&dWire->any.top,n); +- swaps(&dWire->any.left,n); +- swaps(&dWire->any.angle,n); ++ swapl(&any.name, n); ++ swaps(&any.top, n); ++ swaps(&any.left, n); ++ swaps(&any.angle, n); + } + 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: +@@ -4510,13 +4512,14 @@ XkbDoodadPtr doodad; + dWire->text.colorNdx); + return BadMatch; + } ++ text = dWire->text; + if (client->swapped) { + register int n; +- swaps(&dWire->text.width,n); +- swaps(&dWire->text.height,n); ++ swaps(&text.width, n); ++ swaps(&text.height, n); + } +- 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.9.1 + diff --git a/debian/patches/1104-xkb-Check-strings-length-against-request-size.patch b/debian/patches/1104-xkb-Check-strings-length-against-request-size.patch new file mode 100644 index 0000000..533ddcc --- /dev/null +++ b/debian/patches/1104-xkb-Check-strings-length-against-request-size.patch @@ -0,0 +1,148 @@ +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(-) + +diff --git a/nx-X11/programs/Xserver/xkb/xkb.c b/nx-X11/programs/Xserver/xkb/xkb.c +index d8b5b2c..778269f 100644 +--- 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; +-- +1.9.1 + diff --git a/debian/patches/series b/debian/patches/series index 379704c..522b398 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -114,5 +114,9 @@ 1040-glx-Pass-remaining-request-length-into-varsize-v2-CV.patch 1041-nx-X11-lib-font-fc-fserve.c-initialize-remaining-buf.patch 1042-Do-proper-input-validation-to-fix-for-CVE-2011-2895.patch +1101-Coverity-844-845-846-Fix-memory-leaks.patch +1102-include-introduce-byte-counting-functions.patch +1103-xkb-Don-t-swap-XkbSetGeometry-data-in-the-input-buff.patch +1104-xkb-Check-strings-length-against-request-size.patch 0016_nx-X11_install-location.debian.patch 0102_xserver-xext_set-securitypolicy-path.debian.patch -- Alioth's /srv/git/_hooks_/post-receive-email on /srv/git/code.x2go.org/nx-libs.git