Commit ae898ff1 authored by Mihai Moldovan's avatar Mihai Moldovan Committed by Mike Gabriel

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".
parent f74f59d2
...@@ -197,6 +197,13 @@ nx-libs (2:3.5.0.29-0x2go2) UNRELEASED; urgency=medium ...@@ -197,6 +197,13 @@ nx-libs (2:3.5.0.29-0x2go2) UNRELEASED; urgency=medium
[ Mihai Moldovan ] [ Mihai Moldovan ]
* Change string "X2go" to "X2Go" where appropriate. * 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 <mike.gabriel@das-netzwerkteam.de> Thu, 13 Nov 2014 21:59:00 +0100 -- Mike Gabriel <mike.gabriel@das-netzwerkteam.de> Thu, 13 Nov 2014 21:59:00 +0100
......
From 94c6de0649cd295044b1e4ff7265949c9c787519 Mon Sep 17 00:00:00 2001 From 31322c2bd9be76493a5a04a23ea68e063fe3b7e6 Mon Sep 17 00:00:00 2001
From: Mike DePaulo <mikedep333@gmail.com> From: Mike DePaulo <mikedep333@gmail.com>
Date: Sun, 8 Feb 2015 21:03:33 -0500 Date: Sun, 8 Feb 2015 21:03:33 -0500
Subject: [PATCH 07/40] CVE-2014-0210: unvalidated length in 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 ...@@ -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 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 make sure it wasn't copying more data to that buffer than the size
it had allocated. 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 ++++++++++++++++++--- nx-X11/lib/font/fc/fserve.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-) 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 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 --- a/nx-X11/lib/font/fc/fserve.c
+++ b/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; int ret;
fsConnSetup *setup; fsConnSetup *setup;
FSFpeAltPtr alts; FSFpeAltPtr alts;
...@@ -30,7 +32,7 @@ index bac0b8e..0fdcc1d 100644 ...@@ -30,7 +32,7 @@ index bac0b8e..0fdcc1d 100644
int setup_len; int setup_len;
char *alt_save, *alt_names; 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) if (setup->num_alternates)
{ {
...@@ -42,7 +44,7 @@ index bac0b8e..0fdcc1d 100644 ...@@ -42,7 +44,7 @@ index bac0b8e..0fdcc1d 100644
if (alts) if (alts)
{ {
alt_names = (char *) (setup + 1); 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]; alts[i].subset = alt_names[0];
alt_len = alt_names[1]; alt_len = alt_names[1];
...@@ -57,7 +59,7 @@ index bac0b8e..0fdcc1d 100644 ...@@ -57,7 +59,7 @@ index bac0b8e..0fdcc1d 100644
+ "invalid alt list (length %lx >= %lx)\n", + "invalid alt list (length %lx >= %lx)\n",
+ (long) alt_len, (long) alt_name_len); + (long) alt_len, (long) alt_name_len);
+#endif +#endif
+ free(alts); + xfree(alts);
+ return FSIO_ERROR; + return FSIO_ERROR;
+ } + }
alts[i].name = alt_save; alts[i].name = alt_save;
......
From c6aebf9284855a0e24ad9c5ffdd36aa65e16bec7 Mon Sep 17 00:00:00 2001 From e29bbd5bf0565eaf7c02f85a57b87f66531fa6b3 Mon Sep 17 00:00:00 2001
From: Mike DePaulo <mikedep333@gmail.com> From: Mike DePaulo <mikedep333@gmail.com>
Date: Sun, 8 Feb 2015 22:08:09 -0500 Date: Sun, 8 Feb 2015 22:08:09 -0500
Subject: [PATCH 11/40] CVE-2014-0210: unvalidated length fields in 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 ...@@ -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 contains embedded length fields, none of which are validated. This
can cause out of bound reads in either fs_read_query_info() or in 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. _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/fsconvert.c | 19 ++++++++++++++-----
nx-X11/lib/font/fc/fserve.c | 40 ++++++++++++++++++++++++++++++++++++++-- nx-X11/lib/font/fc/fserve.c | 43 +++++++++++++++++++++++++++++++++++++++---
2 files changed, 52 insertions(+), 7 deletions(-) 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 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 --- a/nx-X11/lib/font/fc/fsconvert.c
+++ b/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, @@ -123,6 +123,10 @@ _fs_convert_props(fsPropInfo *pi, fsPropOffset *po, pointer pd,
...@@ -56,18 +58,18 @@ index 9ff54f5..d41e0b8 100644 ...@@ -56,18 +58,18 @@ index 9ff54f5..d41e0b8 100644
} }
off_adr += SIZEOF(fsPropOffset); off_adr += SIZEOF(fsPropOffset);
diff --git a/nx-X11/lib/font/fc/fserve.c b/nx-X11/lib/font/fc/fserve.c 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 --- a/nx-X11/lib/font/fc/fserve.c
+++ b/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; FSFpePtr conn = (FSFpePtr) fpe->private;
fsQueryXInfoReply *rep; fsQueryXInfoReply *rep;
char *buf; char *buf;
+ long bufleft; /* length of reply left to use */ + long bufleft = 0; /* length of reply left to use */
fsPropInfo *pi; fsPropInfo *pi;
fsPropOffset *po; fsPropOffset *po;
pointer pd; 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 = (char *) rep;
buf += SIZEOF(fsQueryXInfoReply); buf += SIZEOF(fsQueryXInfoReply);
...@@ -79,7 +81,7 @@ index 7762653..2a6f6c9 100644 ...@@ -79,7 +81,7 @@ index 7762653..2a6f6c9 100644
/* move the data over */ /* move the data over */
fsUnpack_XFontInfoHeader(rep, pInfo); 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); _fs_init_fontinfo(conn, pInfo);
/* Compute offsets into the reply */ /* Compute offsets into the reply */
...@@ -94,22 +96,24 @@ index 7762653..2a6f6c9 100644 ...@@ -94,22 +96,24 @@ index 7762653..2a6f6c9 100644
+ } + }
pi = (fsPropInfo *) buf; pi = (fsPropInfo *) buf;
buf += SIZEOF (fsPropInfo); buf += SIZEOF (fsPropInfo);
+ bufleft -= pi->num_offsets * SIZEOF(fsPropOffset); -
+ bufleft -= SIZEOF (fsPropInfo);
+ if (bufleft < pi->data_len) +
+ if ((bufleft / SIZEOF (fsPropOffset)) < pi->num_offsets)
+ { + {
+ ret = -1; + ret = -1;
+#ifdef DEBUG +#ifdef DEBUG
+ fprintf(stderr, + fprintf(stderr,
+ "fsQueryXInfo: bufleft (%ld) < data_len (%d)\n", + "fsQueryXInfo: (bufleft / SIZEOF (fsPropOffset)) (%ld) < pi->num_offsets (%d)\n",
+ bufleft, pi->data_len); + bufleft / SIZEOF (fsPropOffset), pi->num_offsets);
+#endif +#endif
+ goto bail; + goto bail;
+ } + }
po = (fsPropOffset *) buf; po = (fsPropOffset *) buf;
buf += pi->num_offsets * SIZEOF(fsPropOffset); buf += pi->num_offsets * SIZEOF(fsPropOffset);
+ bufleft -= pi->data_len; + bufleft -= pi->num_offsets * SIZEOF(fsPropOffset);
+ if (bufleft < pi->data_len)
+ { + {
+ ret = -1; + ret = -1;
+#ifdef DEBUG +#ifdef DEBUG
......
From d4c76981f7fddb364166464c571ed8d3de3086cd Mon Sep 17 00:00:00 2001 From b6b5b14e4190048fadbfbcf063d873d318127e81 Mon Sep 17 00:00:00 2001
From: Alan Coopersmith <alan.coopersmith@oracle.com> From: Alan Coopersmith <alan.coopersmith@oracle.com>
Date: Mon, 6 Jan 2014 23:30:14 -0800 Date: Mon, 6 Jan 2014 23:30:14 -0800
Subject: [PATCH 20/40] dix: integer overflow in GetHosts() [CVE-2014-8092 2/4] 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 ...@@ -14,6 +14,7 @@ This patch caps the list at 1mb, because multi-megabyte hostname
lists for X access control are insane. lists for X access control are insane.
v2: backport to nx-libs 3.6.x (Mike DePaulo) v2: backport to nx-libs 3.6.x (Mike DePaulo)
v3: human-readable version of "1 MB" (Mihai Moldovan)
Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com> Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
...@@ -25,7 +26,7 @@ Conflicts: ...@@ -25,7 +26,7 @@ Conflicts:
1 file changed, 6 insertions(+) 1 file changed, 6 insertions(+)
diff --git a/nx-X11/programs/Xserver/os/access.c b/nx-X11/programs/Xserver/os/access.c 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 --- a/nx-X11/programs/Xserver/os/access.c
+++ b/nx-X11/programs/Xserver/os/access.c +++ b/nx-X11/programs/Xserver/os/access.c
@@ -1719,6 +1719,10 @@ GetHosts ( @@ -1719,6 +1719,10 @@ GetHosts (
...@@ -34,7 +35,7 @@ index b6a70a7..0e9d138 100644 ...@@ -34,7 +35,7 @@ index b6a70a7..0e9d138 100644
n += (((host->len + 3) >> 2) << 2) + sizeof(xHostEntry); n += (((host->len + 3) >> 2) << 2) + sizeof(xHostEntry);
+ /* Could check for INT_MAX, but in reality having more than 1mb of + /* Could check for INT_MAX, but in reality having more than 1mb of
+ hostnames in the access list is ridiculous */ + hostnames in the access list is ridiculous */
+ if (n >= 1048576) + if (n >= 1024*1024)
+ break; + break;
} }
if (n) if (n)
......
From b04f11915e29d9563d279e1326f61b50ea414dba Mon Sep 17 00:00:00 2001
From: Mihai Moldovan <ionic@ionic.de>
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
From 6acafc9334828da22446380c81af81bde14b5d86 Mon Sep 17 00:00:00 2001
From: Joerg Sonnenberger <joerg@britannica.bec.de>
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 <matthieu.herrb@laas.fr>
Reviewed-by: Tomas Hoger <thoger@redhat.com>
---
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
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment