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));

•                                  sizeof (FSFpeAltRec) + alt_name_len);
      if (alts)
      {
          alt_names = (char *) (setup + 1);

  @@ -2820,10 +2820,25 @@ _fs_recv_conn_setup (FSFpePtr conn) { alts[i].subset = alt_names[0]; alt_len = alt_names[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",

•                          (long) alt_len, (long) alt_name_len);

  +#endif

•                 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