Attachment 128227 Details for Bug 98878 – My proposed patch

[patch] My proposed patch

xpm-getc.diff (text/plain), 2.83 KB, created by Tobias Stoeckmann on 2016-11-27 18:08:11 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tobias Stoeckmann

Created: 2016-11-27 18:08:11 UTC

Size: 2.83 KB

patch

obsolete

>From b42b004418b977af92c4dc508a4e117b0725d3f4 Mon Sep 17 00:00:00 2001
>From: Tobias Stoeckmann <tobias@stoeckmann.org>
>Date: Sun, 27 Nov 2016 19:02:33 +0100
>Subject: [PATCH] Gracefully handle EOF while parsing files.
>
>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
>

Actions: View | Splinter Review

Attachments on bug 98878: 128227

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.