[X2Go-Dev] [X2Go-Commits] [nx-libs] 28/52: CVE-2014-0210: unvalidated length fields in fs_read_list() from xorg/lib/libXfont commit 5fa73ac18474be3032ee7af9c6e29deab163ea39

[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 ef439da38d3a4c00a4e03e7d8f83cb359cd9a230
> Author: Mike DePaulo <mikedep333 at gmail.com>
> Date:   Sun Feb 8 22:35:21 2015 -0500
>
>     CVE-2014-0210: unvalidated length fields in fs_read_list() from xorg/lib/libXfont commit 5fa73ac18474be3032ee7af9c6e29deab163ea39
>     
>     fs_read_list() parses a reply from the font server.  The reply
>     contains a list of strings with embedded length fields, none of
>     which are validated. This can cause out of bound reads when looping
>     over the strings in the reply.
> ---
>  nx-X11/lib/font/fc/fserve.c |   15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/nx-X11/lib/font/fc/fserve.c b/nx-X11/lib/font/fc/fserve.c
> index 26218e5..60d9017 100644
> --- a/nx-X11/lib/font/fc/fserve.c
> +++ b/nx-X11/lib/font/fc/fserve.c
> @@ -2365,6 +2365,7 @@ fs_read_list(FontPathElementPtr fpe, FSBlockDataPtr blockrec)
>      FSBlockedListPtr	blist = (FSBlockedListPtr) blockrec->data;
>      fsListFontsReply	*rep;
>      char		*data;
> +    long		dataleft; /* length of reply left to use */

Same here.

long		dataleft = 0;


>      int			length,
>  			i,
>  			ret;
> @@ -2382,16 +2383,30 @@ fs_read_list(FontPathElementPtr fpe, FSBlockDataPtr blockrec)
>  	return AllocError;
>      }
>      data = (char *) rep + SIZEOF (fsListFontsReply);
> +    dataleft = (rep->length << 2) - SIZEOF (fsListFontsReply);
>  
>      err = Successful;
>      /* copy data into FontPathRecord */
>      for (i = 0; i < rep->nFonts; i++) 
>      {
> +	if (dataleft < 1)
> +	    break;

Just as a heads-up: I would have moved this into the for loop condition
like so:

for (i = 0; (i < rep->nFonts) && (dataleft > 0); i++)

to make clear, that it's really part of the looping condition.

The current patch as provided by upstream is functionally equivalent and OK, though.



Everything else in the patch looks good.

-------------- 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/93f67cd7/attachment.pgp>