Re: [X2Go-Dev] [X2Go-Commits] [nx-libs] 23/52: CVE-2014-0210: unvalidated length fields in fs_read_query_info() from xorg/lib/libXfont commit 491291cabf78efdeec8f18b09e14726a9030cc8f

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 c6aebf9284855a0e24ad9c5ffdd36aa65e16bec7 Author: Mike DePaulo <mikedep333@gmail.com> Date: Sun Feb 8 22:08:09 2015 -0500

CVE-2014-0210: unvalidated length fields in fs_read_query_info() from xorg/lib/libXfont commit 491291cabf78efdeec8f18b09e14726a9030cc8f

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.

---

nx-X11/lib/font/fc/fsconvert.c | 19 ++++++++++++++----- nx-X11/lib/font/fc/fserve.c | 40 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 7 deletions(-)

[...] diff --git a/nx-X11/lib/font/fc/fserve.c b/nx-X11/lib/font/fc/fserve.c index 7762653..2a6f6c9 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) FSFpePtr conn = (FSFpePtr) fpe->private; fsQueryXInfoReply *rep; char *buf;

• long bufleft; /* length of reply left to use */

Please initialize variables. Even if they are re-set later on. long bufleft = 0;

 fsPropInfo		*pi;
 fsPropOffset	*po;
 pointer		pd;

@@ -895,7 +896,10 @@ fs_read_query_info(FontPathElementPtr fpe, FSBlockDataPtr blockrec)

 buf = (char *) rep;
 buf += SIZEOF(fsQueryXInfoReply);

• bufleft = rep->length << 2;
• bufleft -= SIZEOF(fsQueryXInfoReply);
• /* move the data over */ fsUnpack_XFontInfoHeader(rep, pInfo);

@@ -903,19 +907,51 @@ fs_read_query_info(FontPathElementPtr fpe, FSBlockDataPtr blockrec) _fs_init_fontinfo(conn, pInfo);

 /* Compute offsets into the reply */

• if (bufleft < SIZEOF(fsPropInfo))
• {
• ret = -1; +#ifdef DEBUG
• fprintf(stderr, "fsQueryXInfo: bufleft (%ld) < SIZEOF(fsPropInfo)\n",

•   bufleft);

  +#endif
• goto bail;
• } pi = (fsPropInfo *) buf; buf += SIZEOF (fsPropInfo);
• bufleft -= pi->num_offsets * SIZEOF(fsPropOffset);

This looks wrong. You probably meant to use: bufleft -= SIZEOF (fsPropInfo);

• if (bufleft < pi->data_len)

This looks wrong, too. Check: https://git.centos.org/blob/rpms!libXfont.git/cf1e18c13a5ba2054e36a6f0b044c3... You probably meant to use: if ((bufleft / SIZEOF(fsPropOffset)) < pi->num_offsets) or if (bufleft < (SIZEOF(fsPropOffset) * pi->num_offsets)) and

• {
• ret = -1; +#ifdef DEBUG
• fprintf(stderr,

•   "fsQueryXInfo: bufleft (%ld) < data_len (%d)\n",

•   bufleft, pi->data_len);

here either: "fsQueryXInfo: (bufleft / SIZEOF(fsPropOffset)) (%ld) < pi->num_offsets (%d)\n", bufleft / SIZEOF(fsPropOffset), pi->num_offsets); or "fsQueryXInfo: bufleft (%ld) < (SIZEOF(fsPropOffset) * pi->num_offsets) (%d)\n", bufleft, SIZEOF(fsPropOffset) * pi->num_offsets); depending on what you used before.

+#endif

• goto bail;
• } po = (fsPropOffset *) buf; buf += pi->num_offsets * SIZEOF(fsPropOffset);
• bufleft -= pi->data_len;

Same problem here. Should be: bufleft -= pi->num_offsets * SIZEOF(fsPropOffset);

• {

This block will ALWAYS be executed and ALWAYS *FAIL*! You're missing (*before* the curly brace): if (bufleft < pi->data_len)

• ret = -1; +#ifdef DEBUG

• fprintf(stderr,

•   "fsQueryXInfo: bufleft (%ld) < data_len (%d)\n",

•   bufleft, pi->data_len);

  +#endif

• goto bail;

• } pd = (pointer) buf; buf += pi->data_len;

• bufleft -= pi->data_len;

  /* convert the properties and step over the reply */ ret = _fs_convert_props(pi, po, pd, pInfo);

• bail: _fs_done_read (conn, rep->length << 2);

• if (ret == -1) { fs_cleanup_bfont (bfont);

This looks OK again.