From 52603840b1c5d923cc998335fb651a53d42a036c Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 7 Jan 2023 12:44:28 -0800 Subject: [PATCH libXpm 2/5] Fix CVE-2022-44617: Runaway loop with width of 0 and enormous height When reading XPM images from a file with libXpm 3.5.14 or older, if a image has a width of 0 and a very large height, the ParsePixels() function will loop over the entire height calling getc() and ungetc() repeatedly, or in some circumstances, may loop seemingly forever, which may cause a denial of service to the calling program when given a small crafted XPM file to parse. Closes: #2 Reported-by: Martin Ettl Signed-off-by: Alan Coopersmith --- src/data.c | 20 ++++++++++++++------ src/parse.c | 31 +++++++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/src/data.c b/src/data.c index bfad4ff..7524e65 100644 --- a/src/data.c +++ b/src/data.c @@ -195,19 +195,23 @@ xpmNextString(xpmData *data) register char c; /* get to the end of the current string */ - if (data->Eos) - while ((c = *data->cptr++) && c != data->Eos); + if (data->Eos) { + while ((c = *data->cptr++) && c != data->Eos && c != '\0'); + + if (c == '\0') + return XpmFileInvalid; + } /* * then get to the beginning of the next string looking for possible * comment */ if (data->Bos) { - while ((c = *data->cptr++) && c != data->Bos) + while ((c = *data->cptr++) && c != data->Bos && c != '\0') if (data->Bcmt && c == data->Bcmt[0]) ParseComment(data); } else if (data->Bcmt) { /* XPM2 natural */ - while ((c = *data->cptr++) == data->Bcmt[0]) + while (((c = *data->cptr++) == data->Bcmt[0]) && c != '\0') ParseComment(data); data->cptr--; } @@ -216,9 +220,13 @@ xpmNextString(xpmData *data) FILE *file = data->stream.file; /* get to the end of the current string */ - if (data->Eos) + if (data->Eos) { while ((c = Getc(data, file)) != data->Eos && c != EOF); + if (c == EOF) + return XpmFileInvalid; + } + /* * then get to the beginning of the next string looking for possible * comment @@ -234,7 +242,7 @@ xpmNextString(xpmData *data) Ungetc(data, c, file); } } - return 0; + return XpmSuccess; } diff --git a/src/parse.c b/src/parse.c index c19209c..e97d771 100644 --- a/src/parse.c +++ b/src/parse.c @@ -391,6 +391,13 @@ ParsePixels( { unsigned int *iptr, *iptr2 = NULL; /* found by Egbert Eich */ unsigned int a, x, y; + int ErrorStatus; + + if ((width == 0) && (height != 0)) + return (XpmFileInvalid); + + if ((height == 0) && (width != 0)) + return (XpmFileInvalid); if ((height > 0 && width >= UINT_MAX / height) || width * height >= UINT_MAX / sizeof(unsigned int)) @@ -428,7 +435,11 @@ ParsePixels( colidx[(unsigned char)colorTable[a].string[0]] = a + 1; for (y = 0; y < height; y++) { - xpmNextString(data); + ErrorStatus = xpmNextString(data); + if (ErrorStatus != XpmSuccess) { + XpmFree(iptr2); + return (ErrorStatus); + } for (x = 0; x < width; x++, iptr++) { int c = xpmGetC(data); @@ -475,7 +486,11 @@ do \ } for (y = 0; y < height; y++) { - xpmNextString(data); + ErrorStatus = xpmNextString(data); + if (ErrorStatus != XpmSuccess) { + XpmFree(iptr2); + return (ErrorStatus); + } for (x = 0; x < width; x++, iptr++) { int cc1 = xpmGetC(data); if (cc1 > 0 && cc1 < 256) { @@ -515,7 +530,11 @@ do \ xpmHashAtom *slot; for (y = 0; y < height; y++) { - xpmNextString(data); + ErrorStatus = xpmNextString(data); + if (ErrorStatus != XpmSuccess) { + XpmFree(iptr2); + return (ErrorStatus); + } for (x = 0; x < width; x++, iptr++) { for (a = 0, s = buf; a < cpp; a++, s++) { int c = xpmGetC(data); @@ -535,7 +554,11 @@ do \ } } else { for (y = 0; y < height; y++) { - xpmNextString(data); + ErrorStatus = xpmNextString(data); + if (ErrorStatus != XpmSuccess) { + XpmFree(iptr2); + return (ErrorStatus); + } for (x = 0; x < width; x++, iptr++) { for (a = 0, s = buf; a < cpp; a++, s++) { int c = xpmGetC(data); -- 2.39.0