From dd830b3ffe30a76fbe8c1f13ebc7483c9ff792e5 Mon Sep 17 00:00:00 2001 From: DRC Date: Fri, 9 Apr 2021 17:36:41 -0500 Subject: [PATCH] rdbmp.c/rdppm.c: Fix more innocuous UBSan errors - rdbmp.c: Because of 8fb37b81713a0cdc14622dc08892ebd28a3233aa, bfOffBits, biClrUsed, and headerSize were made into unsigned ints. Thus, if bPad would eventually be negative due to a malformed header, UBSan complained about unsigned math being used in the intermediate computations. It was unnecessary to make those variables unsigned, since they are only meant to hold small values, so this commit makes them signed again. The UBSan error was innocuous, because it is effectively (if not officially) the case that (int)((unsigned int)a - (unsigned int)b) == (int)a - (int)b. - rdbmp.c: If (biWidth * source->bits_per_pixel / 8) would overflow an unsigned int, then UBSan complained at the point at which row_width was set in start_input_bmp(), even though the overflow would have been detected later in the function. This commit adds overflow checks prior to setting row_width. - rdppm.c: read_pbm_integer() now bounds-checks the intermediate value computations in order to catch integer overflow caused by a malformed text PPM. It's possible, though extremely unlikely, that the intermediate value computations could have wrapped around to a value smaller than maxval, but the worst case is that this would have generated a bogus pixel in the uncompressed image rather than throwing an error. --- rdbmp.c | 10 +++++++--- rdppm.c | 5 ++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/rdbmp.c b/rdbmp.c index c6ce2948..56470c37 100644 --- a/rdbmp.c +++ b/rdbmp.c @@ -424,14 +424,14 @@ start_input_bmp(j_compress_ptr cinfo, cjpeg_source_ptr sinfo) (((unsigned int)UCH(array[offset + 2])) << 16) + \ (((unsigned int)UCH(array[offset + 3])) << 24)) - unsigned int bfOffBits; - unsigned int headerSize; + int bfOffBits; + int headerSize; int biWidth; int biHeight; unsigned short biPlanes; unsigned int biCompression; int biXPelsPerMeter, biYPelsPerMeter; - unsigned int biClrUsed = 0; + int biClrUsed = 0; int mapentrysize = 0; /* 0 indicates no colormap */ int bPad; JDIMENSION row_width = 0; @@ -575,6 +575,8 @@ start_input_bmp(j_compress_ptr cinfo, cjpeg_source_ptr sinfo) cinfo->input_components = 4; else ERREXIT(cinfo, JERR_BAD_IN_COLORSPACE); + if ((unsigned long long)biWidth * 3ULL > 0xFFFFFFFFULL) + ERREXIT(cinfo, JERR_WIDTH_OVERFLOW); row_width = (JDIMENSION)(biWidth * 3); break; case 32: @@ -586,6 +588,8 @@ start_input_bmp(j_compress_ptr cinfo, cjpeg_source_ptr sinfo) cinfo->input_components = 4; else ERREXIT(cinfo, JERR_BAD_IN_COLORSPACE); + if ((unsigned long long)biWidth * 4ULL > 0xFFFFFFFFULL) + ERREXIT(cinfo, JERR_WIDTH_OVERFLOW); row_width = (JDIMENSION)(biWidth * 4); break; default: diff --git a/rdppm.c b/rdppm.c index d4378dda..cea124ce 100644 --- a/rdppm.c +++ b/rdppm.c @@ -112,11 +112,10 @@ read_pbm_integer(j_compress_ptr cinfo, FILE *infile, unsigned int maxval) while ((ch = pbm_getc(infile)) >= '0' && ch <= '9') { val *= 10; val += ch - '0'; + if (val > maxval) + ERREXIT(cinfo, JERR_PPM_OUTOFRANGE); } - if (val > maxval) - ERREXIT(cinfo, JERR_PPM_OUTOFRANGE); - return val; }