On Sun, Feb 15, 2015 at 2:18 PM, Mihai Moldovan <ionic@ionic.de> wrote:
On 14.02.2015 05:47 PM, git-admin@x2go.org wrote:
This is an automated email from the git hooks/post-receive script.
x2go pushed a commit to branch 3.6.x in repository nx-libs.
commit 94c6de0649cd295044b1e4ff7265949c9c787519 Author: Mike DePaulo <mikedep333@gmail.com> Date: Sun Feb 8 21:03:33 2015 -0500
CVE-2014-0210: unvalidated length in _fs_recv_conn_setup() from xorg/lib/libXfont commit 891e084b26837162b12f841060086a105edde86d The connection setup reply from the font server can include a list of alternate servers to contact if this font server stops working. The reply specifies a total size of all the font server names, and 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.
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 --- 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) int ret; fsConnSetup *setup; FSFpeAltPtr alts;
- int i, alt_len;
- unsigned int i, alt_len; int setup_len; char *alt_save, *alt_names;
@@ -2809,9 +2809,9 @@ _fs_recv_conn_setup (FSFpePtr conn) } if (setup->num_alternates) {
size_t alt_name_len = setup->alternate_len << 2; alts = (FSFpeAltPtr) xalloc (setup->num_alternates *
sizeof (FSFpeAltRec) +
(setup->alternate_len << 2));
@@ -2820,10 +2820,25 @@ _fs_recv_conn_setup (FSFpePtr conn) { alts[i].subset = alt_names[0]; alt_len = alt_names[1];sizeof (FSFpeAltRec) + alt_name_len); if (alts) { alt_names = (char *) (setup + 1);
if (alt_len >= alt_name_len) {
/*
* Length is longer than setup->alternate_len
* told us to allocate room for, assume entire
* alternate list is corrupted.
+#ifdef DEBUG*/
fprintf (stderr,
"invalid alt list (length %lx >= %lx)\n",
+#endif(long) alt_len, (long) alt_name_len);
free(alts);
Shouldn't this be xfree(alts) if using xalloc previously?
I am actually not sure, I need to learn memory management better. The upstream commit uses and free(alts), and malloc is used before it: http://cgit.freedesktop.org/xorg/lib/libXfont/commit/?id=891e084b26837162b12... And that is what I based my commit/patch on. However, the RHEL5 patch also uses free(alts), and xalloc is used before it: ftp://ftp.redhat.com/redhat/linux/enterprise/5Server/en/os/SRPMS/libXfont-1.2.2-1.0.6.el5_11.src.rpm (0003-CVE-2014-0210-unvalidated-length-in-_fs_recv_conn_se.patch) The patch doesn't specify who resolved the RHEL5 conflict, but it was probably Adam Jackson. (ajax) -Mike