[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

[Mihai Moldovan]

On 14.02.2015 05:47 PM, git-admin at 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 at 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/cf1e18c13a5ba2054e36a6f0b044c3ed10c1b358/SOURCES!0006-CVE-2014-0210-unvalidated-length-fields-in-fs_read_q.patch;jsessionid=12hi4vv3qj9yo2ercznrbzevc

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.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 884 bytes
Desc: OpenPGP digital signature
URL: <http://lists.x2go.org/pipermail/x2go-dev/attachments/20150215/255e93dc/attachment-0001.pgp>