From ae898ff13b4782239a152b43125aa1fc0b80ba3d Mon Sep 17 00:00:00 2001 From: Mihai Moldovan Date: Mon, 16 Feb 2015 09:35:47 +0100 Subject: CVE security review [1/2]. * CVE security review [1/2]: - Update 1007-CVE-2014-0210-unvalidated-length-in-_fs_recv_conn_se.patch. Use xfree() instead of free() in nx-libs. - Update 1011-CVE-2014-0210-unvalidated-length-fields-in-fs_read_q.patch. Apply correctly on nx-libs 3.6.x. - Update 1020-dix-integer-overflow-in-GetHosts-CVE-2014-8092-2-4.patch. Human-readable version of "1 MB". --- debian/changelog | 7 ++ ...10-unvalidated-length-in-_fs_recv_conn_se.patch | 14 +-- ...10-unvalidated-length-fields-in-fs_read_q.patch | 34 ++++--- ...er-overflow-in-GetHosts-CVE-2014-8092-2-4.patch | 7 +- ...font-fc-fserve.c-initialize-remaining-buf.patch | 35 +++++++ ...input-validation-to-fix-for-CVE-2011-2895.patch | 109 +++++++++++++++++++++ 6 files changed, 182 insertions(+), 24 deletions(-) create mode 100644 debian/patches/1041-nx-X11-lib-font-fc-fserve.c-initialize-remaining-buf.patch create mode 100644 debian/patches/1042-Do-proper-input-validation-to-fix-for-CVE-2011-2895.patch diff --git a/debian/changelog b/debian/changelog index efe018cf4..4f6833df2 100644 --- a/debian/changelog +++ b/debian/changelog @@ -197,6 +197,13 @@ nx-libs (2:3.5.0.29-0x2go2) UNRELEASED; urgency=medium [ Mihai Moldovan ] * Change string "X2go" to "X2Go" where appropriate. + * CVE security review: + - Update 1007-CVE-2014-0210-unvalidated-length-in-_fs_recv_conn_se.patch. + Use xfree() instead of free() in nx-libs. + - Update 1011-CVE-2014-0210-unvalidated-length-fields-in-fs_read_q.patch. + Apply correctly on nx-libs 3.6.x. + - Update 1020-dix-integer-overflow-in-GetHosts-CVE-2014-8092-2-4.patch. + Human-readable version of "1 MB". -- Mike Gabriel Thu, 13 Nov 2014 21:59:00 +0100 diff --git a/debian/patches/1007-CVE-2014-0210-unvalidated-length-in-_fs_recv_conn_se.patch b/debian/patches/1007-CVE-2014-0210-unvalidated-length-in-_fs_recv_conn_se.patch index b71627214..2b2fa76c8 100644 --- a/debian/patches/1007-CVE-2014-0210-unvalidated-length-in-_fs_recv_conn_se.patch +++ b/debian/patches/1007-CVE-2014-0210-unvalidated-length-in-_fs_recv_conn_se.patch @@ -1,4 +1,4 @@ -From 94c6de0649cd295044b1e4ff7265949c9c787519 Mon Sep 17 00:00:00 2001 +From 31322c2bd9be76493a5a04a23ea68e063fe3b7e6 Mon Sep 17 00:00:00 2001 From: Mike DePaulo Date: Sun, 8 Feb 2015 21:03:33 -0500 Subject: [PATCH 07/40] CVE-2014-0210: unvalidated length in @@ -13,15 +13,17 @@ then provides a list of names. _fs_recv_conn_setup() allocated the specified total size for copying the names to, but didn't check to make sure it wasn't copying more data to that buffer than the size it had allocated. + +v2: use xfree() instead of free() for nx-libs 3.6.x (Mihai Moldovan) --- nx-X11/lib/font/fc/fserve.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/nx-X11/lib/font/fc/fserve.c b/nx-X11/lib/font/fc/fserve.c -index bac0b8e..0fdcc1d 100644 +index 0d792c7..86b5753 100644 --- a/nx-X11/lib/font/fc/fserve.c +++ b/nx-X11/lib/font/fc/fserve.c -@@ -2782,7 +2782,7 @@ _fs_recv_conn_setup (FSFpePtr conn) +@@ -2985,7 +2985,7 @@ _fs_recv_conn_setup (FSFpePtr conn) int ret; fsConnSetup *setup; FSFpeAltPtr alts; @@ -30,7 +32,7 @@ index bac0b8e..0fdcc1d 100644 int setup_len; char *alt_save, *alt_names; -@@ -2809,9 +2809,9 @@ _fs_recv_conn_setup (FSFpePtr conn) +@@ -3012,9 +3012,9 @@ _fs_recv_conn_setup (FSFpePtr conn) } if (setup->num_alternates) { @@ -42,7 +44,7 @@ index bac0b8e..0fdcc1d 100644 if (alts) { alt_names = (char *) (setup + 1); -@@ -2820,10 +2820,25 @@ _fs_recv_conn_setup (FSFpePtr conn) +@@ -3023,10 +3023,25 @@ _fs_recv_conn_setup (FSFpePtr conn) { alts[i].subset = alt_names[0]; alt_len = alt_names[1]; @@ -57,7 +59,7 @@ index bac0b8e..0fdcc1d 100644 + "invalid alt list (length %lx >= %lx)\n", + (long) alt_len, (long) alt_name_len); +#endif -+ free(alts); ++ xfree(alts); + return FSIO_ERROR; + } alts[i].name = alt_save; diff --git a/debian/patches/1011-CVE-2014-0210-unvalidated-length-fields-in-fs_read_q.patch b/debian/patches/1011-CVE-2014-0210-unvalidated-length-fields-in-fs_read_q.patch index 96b7b9749..9d0f3f875 100644 --- a/debian/patches/1011-CVE-2014-0210-unvalidated-length-fields-in-fs_read_q.patch +++ b/debian/patches/1011-CVE-2014-0210-unvalidated-length-fields-in-fs_read_q.patch @@ -1,4 +1,4 @@ -From c6aebf9284855a0e24ad9c5ffdd36aa65e16bec7 Mon Sep 17 00:00:00 2001 +From e29bbd5bf0565eaf7c02f85a57b87f66531fa6b3 Mon Sep 17 00:00:00 2001 From: Mike DePaulo Date: Sun, 8 Feb 2015 22:08:09 -0500 Subject: [PATCH 11/40] CVE-2014-0210: unvalidated length fields in @@ -9,13 +9,15 @@ fs_read_query_info() parses a reply from the font server. The reply contains embedded length fields, none of which are validated. This can cause out of bound reads in either fs_read_query_info() or in _fs_convert_props() which it calls to parse the fsPropInfo in the reply. + +v2: apply correctly on nx-libs 3.6.x (Mihai Moldovan) --- nx-X11/lib/font/fc/fsconvert.c | 19 ++++++++++++++----- - nx-X11/lib/font/fc/fserve.c | 40 ++++++++++++++++++++++++++++++++++++++-- - 2 files changed, 52 insertions(+), 7 deletions(-) + nx-X11/lib/font/fc/fserve.c | 43 +++++++++++++++++++++++++++++++++++++++--- + 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/nx-X11/lib/font/fc/fsconvert.c b/nx-X11/lib/font/fc/fsconvert.c -index 9ff54f5..d41e0b8 100644 +index 9a5e194..afa2c32 100644 --- a/nx-X11/lib/font/fc/fsconvert.c +++ b/nx-X11/lib/font/fc/fsconvert.c @@ -123,6 +123,10 @@ _fs_convert_props(fsPropInfo *pi, fsPropOffset *po, pointer pd, @@ -56,18 +58,18 @@ index 9ff54f5..d41e0b8 100644 } off_adr += SIZEOF(fsPropOffset); diff --git a/nx-X11/lib/font/fc/fserve.c b/nx-X11/lib/font/fc/fserve.c -index 7762653..2a6f6c9 100644 +index 9e652d2..75cabdd 100644 --- a/nx-X11/lib/font/fc/fserve.c +++ b/nx-X11/lib/font/fc/fserve.c -@@ -865,6 +865,7 @@ fs_read_query_info(FontPathElementPtr fpe, FSBlockDataPtr blockrec) +@@ -866,6 +866,7 @@ fs_read_query_info(FontPathElementPtr fpe, FSBlockDataPtr blockrec) FSFpePtr conn = (FSFpePtr) fpe->private; fsQueryXInfoReply *rep; char *buf; -+ long bufleft; /* length of reply left to use */ ++ long bufleft = 0; /* length of reply left to use */ fsPropInfo *pi; fsPropOffset *po; pointer pd; -@@ -895,7 +896,10 @@ fs_read_query_info(FontPathElementPtr fpe, FSBlockDataPtr blockrec) +@@ -896,7 +897,10 @@ fs_read_query_info(FontPathElementPtr fpe, FSBlockDataPtr blockrec) buf = (char *) rep; buf += SIZEOF(fsQueryXInfoReply); @@ -79,7 +81,7 @@ index 7762653..2a6f6c9 100644 /* move the data over */ fsUnpack_XFontInfoHeader(rep, pInfo); -@@ -903,19 +907,51 @@ fs_read_query_info(FontPathElementPtr fpe, FSBlockDataPtr blockrec) +@@ -904,19 +908,52 @@ fs_read_query_info(FontPathElementPtr fpe, FSBlockDataPtr blockrec) _fs_init_fontinfo(conn, pInfo); /* Compute offsets into the reply */ @@ -94,22 +96,24 @@ index 7762653..2a6f6c9 100644 + } pi = (fsPropInfo *) buf; buf += SIZEOF (fsPropInfo); -+ bufleft -= pi->num_offsets * SIZEOF(fsPropOffset); - -+ if (bufleft < pi->data_len) +- ++ bufleft -= SIZEOF (fsPropInfo); ++ ++ if ((bufleft / SIZEOF (fsPropOffset)) < pi->num_offsets) + { + ret = -1; +#ifdef DEBUG + fprintf(stderr, -+ "fsQueryXInfo: bufleft (%ld) < data_len (%d)\n", -+ bufleft, pi->data_len); ++ "fsQueryXInfo: (bufleft / SIZEOF (fsPropOffset)) (%ld) < pi->num_offsets (%d)\n", ++ bufleft / SIZEOF (fsPropOffset), pi->num_offsets); +#endif + goto bail; + } po = (fsPropOffset *) buf; buf += pi->num_offsets * SIZEOF(fsPropOffset); -+ bufleft -= pi->data_len; ++ bufleft -= pi->num_offsets * SIZEOF(fsPropOffset); ++ if (bufleft < pi->data_len) + { + ret = -1; +#ifdef DEBUG diff --git a/debian/patches/1020-dix-integer-overflow-in-GetHosts-CVE-2014-8092-2-4.patch b/debian/patches/1020-dix-integer-overflow-in-GetHosts-CVE-2014-8092-2-4.patch index 619794f7e..1d880399f 100644 --- a/debian/patches/1020-dix-integer-overflow-in-GetHosts-CVE-2014-8092-2-4.patch +++ b/debian/patches/1020-dix-integer-overflow-in-GetHosts-CVE-2014-8092-2-4.patch @@ -1,4 +1,4 @@ -From d4c76981f7fddb364166464c571ed8d3de3086cd Mon Sep 17 00:00:00 2001 +From b6b5b14e4190048fadbfbcf063d873d318127e81 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Mon, 6 Jan 2014 23:30:14 -0800 Subject: [PATCH 20/40] dix: integer overflow in GetHosts() [CVE-2014-8092 2/4] @@ -14,6 +14,7 @@ This patch caps the list at 1mb, because multi-megabyte hostname lists for X access control are insane. v2: backport to nx-libs 3.6.x (Mike DePaulo) +v3: human-readable version of "1 MB" (Mihai Moldovan) Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Peter Hutterer @@ -25,7 +26,7 @@ Conflicts: 1 file changed, 6 insertions(+) diff --git a/nx-X11/programs/Xserver/os/access.c b/nx-X11/programs/Xserver/os/access.c -index b6a70a7..0e9d138 100644 +index b6a70a7..532a2f8 100644 --- a/nx-X11/programs/Xserver/os/access.c +++ b/nx-X11/programs/Xserver/os/access.c @@ -1719,6 +1719,10 @@ GetHosts ( @@ -34,7 +35,7 @@ index b6a70a7..0e9d138 100644 n += (((host->len + 3) >> 2) << 2) + sizeof(xHostEntry); + /* Could check for INT_MAX, but in reality having more than 1mb of + hostnames in the access list is ridiculous */ -+ if (n >= 1048576) ++ if (n >= 1024*1024) + break; } if (n) diff --git a/debian/patches/1041-nx-X11-lib-font-fc-fserve.c-initialize-remaining-buf.patch b/debian/patches/1041-nx-X11-lib-font-fc-fserve.c-initialize-remaining-buf.patch new file mode 100644 index 000000000..4203bf674 --- /dev/null +++ b/debian/patches/1041-nx-X11-lib-font-fc-fserve.c-initialize-remaining-buf.patch @@ -0,0 +1,35 @@ +From b04f11915e29d9563d279e1326f61b50ea414dba Mon Sep 17 00:00:00 2001 +From: Mihai Moldovan +Date: Mon, 16 Feb 2015 06:03:48 +0100 +Subject: [PATCH 07/15] nx-X11/lib/font/fc/fserve.c: initialize remaining + bufleft variables. + +--- + nx-X11/lib/font/fc/fserve.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/nx-X11/lib/font/fc/fserve.c b/nx-X11/lib/font/fc/fserve.c +index 86b5753..6bbb8c2 100644 +--- a/nx-X11/lib/font/fc/fserve.c ++++ b/nx-X11/lib/font/fc/fserve.c +@@ -1917,7 +1917,7 @@ fs_read_glyphs(FontPathElementPtr fpe, FSBlockDataPtr blockrec) + FontInfoPtr pfi = &pfont->info; + fsQueryXBitmaps16Reply *rep; + char *buf; +- long bufleft; /* length of reply left to use */ ++ long bufleft = 0; /* length of reply left to use */ + fsOffset32 *ppbits; + fsOffset32 local_off; + char *off_adr; +@@ -2501,7 +2501,7 @@ fs_read_list_info(FontPathElementPtr fpe, FSBlockDataPtr blockrec) + FSBlockedListInfoPtr binfo = (FSBlockedListInfoPtr) blockrec->data; + fsListFontsWithXInfoReply *rep; + char *buf; +- long bufleft; ++ long bufleft = 0; + FSFpePtr conn = (FSFpePtr) fpe->private; + fsPropInfo *pi; + fsPropOffset *po; +-- +2.1.4 + diff --git a/debian/patches/1042-Do-proper-input-validation-to-fix-for-CVE-2011-2895.patch b/debian/patches/1042-Do-proper-input-validation-to-fix-for-CVE-2011-2895.patch new file mode 100644 index 000000000..9e5d00e98 --- /dev/null +++ b/debian/patches/1042-Do-proper-input-validation-to-fix-for-CVE-2011-2895.patch @@ -0,0 +1,109 @@ +From 6acafc9334828da22446380c81af81bde14b5d86 Mon Sep 17 00:00:00 2001 +From: Joerg Sonnenberger +Date: Sun, 21 Aug 2011 18:51:53 +0200 +Subject: [PATCH 08/15] Do proper input validation to fix for CVE-2011-2895. + +It ensures that all valid input can be decompressed, checks that the +overflow conditions doesn't happen and generally tightens the +validation of the LZW stream and doesn't pessimize the inner loop for +no good reason. It's derived from a change in libarchive from 2004. + +v2: backports to nx-libs 3.6.x (Mihai Moldovan) +Signed-off-by: Matthieu Herrb +Reviewed-by: Tomas Hoger +--- + nx-X11/lib/font/fontfile/decompress.c | 31 +++++++++++++++++-------------- + 1 file changed, 17 insertions(+), 14 deletions(-) + +diff --git a/nx-X11/lib/font/fontfile/decompress.c b/nx-X11/lib/font/fontfile/decompress.c +index 553b315..12b9f0a 100644 +--- a/nx-X11/lib/font/fontfile/decompress.c ++++ b/nx-X11/lib/font/fontfile/decompress.c +@@ -99,7 +99,7 @@ static char_type magic_header[] = { "\037\235" }; /* 1F 9D */ + #define FIRST 257 /* first free entry */ + #define CLEAR 256 /* table clear output code */ + +-#define STACK_SIZE 8192 ++#define STACK_SIZE 65300 + + typedef struct _compressedFILE { + BufFilePtr file; +@@ -180,14 +180,12 @@ BufFilePushCompressed (BufFilePtr f) + file->tab_suffix[code] = (char_type) code; + } + file->free_ent = ((file->block_compress) ? FIRST : 256 ); ++ file->oldcode = -1; + file->clear_flg = 0; + file->offset = 0; + file->size = 0; + file->stackp = file->de_stack; + bzero(file->buf, BITS); +- file->finchar = file->oldcode = getcode (file); +- if (file->oldcode != -1) +- *file->stackp++ = file->finchar; + return BufFileCreate ((char *) file, + BufCompressedFill, + 0, +@@ -232,9 +230,6 @@ BufCompressedFill (BufFilePtr f) + if (buf == bufend) + break; + +- if (oldcode == -1) +- break; +- + code = getcode (file); + if (code == -1) + break; +@@ -243,26 +238,34 @@ BufCompressedFill (BufFilePtr f) + for ( code = 255; code >= 0; code-- ) + file->tab_prefix[code] = 0; + file->clear_flg = 1; +- file->free_ent = FIRST - 1; +- if ( (code = getcode (file)) == -1 ) /* O, untimely death! */ +- break; ++ file->free_ent = FIRST; ++ oldcode = -1; ++ continue; + } + incode = code; + /* + * Special case for KwKwK string. + */ + if ( code >= file->free_ent ) { ++ if ( code > file->free_ent || oldcode == -1 ) { ++ /* Bad stream. */ ++ return BUFFILEEOF; ++ } + *stackp++ = finchar; + code = oldcode; + } +- +++ /* +++ * The above condition ensures that code < free_ent. +++ * The construction of tab_prefixof in turn guarantees that +++ * each iteration decreases code and therefore stack usage is +++ * bound by 1 << BITS - 256. +++ */ ++ + /* + * Generate output characters in reverse order + */ + while ( code >= 256 ) + { +- if (stackp - de_stack >= STACK_SIZE - 1) +- return BUFFILEEOF; + *stackp++ = file->tab_suffix[code]; + code = file->tab_prefix[code]; + } +@@ -272,7 +275,7 @@ BufCompressedFill (BufFilePtr f) + /* + * Generate the new entry. + */ +- if ( (code=file->free_ent) < file->maxmaxcode ) { ++ if ( (code=file->free_ent) < file->maxmaxcode && oldcode != -1) { + file->tab_prefix[code] = (unsigned short)oldcode; + file->tab_suffix[code] = finchar; + file->free_ent = code+1; +-- +2.1.4 + -- cgit v1.2.3