[PATCH libXpm] Gracefully handle EOF while parsing files.

[Matthieu Herrb]

On Tue, Dec 06, 2016 at 10:34:33PM +0100, Matthieu Herrb wrote:

This is from https://bugs.freedesktop.org/show_bug.cgi?id=98878

> From: Tobias Stoeckmann <tobias at stoeckmann.org>
> 
> libXpm does not properly handle EOF conditions when xpmGetC is called
> multiple times in a row to construct a string. Instead of checking
> its return value for EOF, the result is automatically casted into a
> char and attached to a string.
> 
> By carefully crafting the color table in an XPM file, it is possible to
> send a libXpm program like gimp into a very long lasting loop and
> massive memory allocations.
> 
> Otherwise no memory issues arise, therefore this is just a purely
> functional patch to dismiss invalid input.
> ---
>  src/parse.c | 40 ++++++++++++++++++++++++++++++++--------
>  1 file changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/src/parse.c b/src/parse.c
> index ff23a47..c19209c 100644
> --- a/src/parse.c
> +++ b/src/parse.c
> @@ -234,8 +234,14 @@ xpmParseColors(
>  		xpmFreeColorTable(colorTable, ncolors);
>  		return (XpmNoMemory);
>  	    }
> -	    for (b = 0, s = color->string; b < cpp; b++, s++)
> -		*s = xpmGetC(data);
> +	    for (b = 0, s = color->string; b < cpp; b++, s++) {
> +		int c = xpmGetC(data);
> +		if (c < 0) {
> +		    xpmFreeColorTable(colorTable, ncolors);
> +		    return (XpmFileInvalid);
> +		}
> +		*s = (char) c;
> +	    }
>  	    *s = '\0';
>  
>  	    /*
> @@ -322,8 +328,14 @@ xpmParseColors(
>  		xpmFreeColorTable(colorTable, ncolors);
>  		return (XpmNoMemory);
>  	    }
> -	    for (b = 0, s = color->string; b < cpp; b++, s++)
> -		*s = xpmGetC(data);
> +	    for (b = 0, s = color->string; b < cpp; b++, s++) {
> +		int c = xpmGetC(data);
> +		if (c < 0) {
> +		    xpmFreeColorTable(colorTable, ncolors);
> +		    return (XpmFileInvalid);
> +		}
> +		*s = (char) c;
> +	    }
>  	    *s = '\0';
>  
>  	    /*
> @@ -505,8 +517,14 @@ do \
>  		for (y = 0; y < height; y++) {
>  		    xpmNextString(data);
>  		    for (x = 0; x < width; x++, iptr++) {
> -			for (a = 0, s = buf; a < cpp; a++, s++)
> -			    *s = xpmGetC(data); /* int assigned to char, not a problem here */
> +			for (a = 0, s = buf; a < cpp; a++, s++) {
> +			    int c = xpmGetC(data);
> +			    if (c < 0) {
> +				XpmFree(iptr2);
> +				return (XpmFileInvalid);
> +			    }
> +			    *s = (char) c;
> +			}
>  			slot = xpmHashSlot(hashtable, buf);
>  			if (!*slot) {	/* no color matches */
>  			    XpmFree(iptr2);
> @@ -519,8 +537,14 @@ do \
>  		for (y = 0; y < height; y++) {
>  		    xpmNextString(data);
>  		    for (x = 0; x < width; x++, iptr++) {
> -			for (a = 0, s = buf; a < cpp; a++, s++)
> -			    *s = xpmGetC(data); /* int assigned to char, not a problem here */
> +			for (a = 0, s = buf; a < cpp; a++, s++) {
> +			    int c = xpmGetC(data);
> +			    if (c < 0) {
> +				XpmFree(iptr2);
> +				return (XpmFileInvalid);
> +			    }
> +			    *s = (char) c;
> +			}
>  			for (a = 0; a < ncolors; a++)
>  			    if (!strcmp(colorTable[a].string, buf))
>  				break;
> -- 
> 2.10.2
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel

-- 
Matthieu Herrb
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 793 bytes
Desc: not available
URL: <https://lists.x.org/archives/xorg-devel/attachments/20161206/02cd3602/attachment.sig>