From 1ec33006a9e4214b390045b820464e24297dc6c0 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Tue, 6 Dec 2016 22:34:33 +0100 Subject: 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. Signed-off-by: Matthieu Herrb Reviewed-by: Matthieu Herrb --- 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; -- cgit v1.2.1