From a15994ee549d8013518319a405e8f035c0793a2d Mon Sep 17 00:00:00 2001 From: Benjamin Beasley Date: Dec 13 2020 14:36:21 +0000 Subject: Bounds-check all accesses into enumerated-value name arrays, fixing some buffer overruns --- diff --git a/pngcheck-2.4.0-bounds-check-enumerated-names.patch b/pngcheck-2.4.0-bounds-check-enumerated-names.patch new file mode 100644 index 0000000..82b7499 --- /dev/null +++ b/pngcheck-2.4.0-bounds-check-enumerated-names.patch @@ -0,0 +1,383 @@ +In various cases, a value from the file being checked is used as an offest into +a buffer of enumerated value name strings. In some cases, the value is +adequately constrained; in others, especially when the -f option is given, it +is possible for a malformed file to cause an access outside the names array +(buffer overrun). Handle this by ensuring all accesses of this type are +bounds-checked. + +The following base-64-encoded file is an example that causes one such overrun +when examined with pngcheck -p -s -v: + +WYlQTsdlTTBkiVBORw0KGgoAAAANTUFHTkTnqwBZWJ9mubHCjwJvbAbG8elyOf/5BU8I0mqcAFSJ + +diff -Naur pngcheck-2.4.0-original/pngcheck.c pngcheck-2.4.0/pngcheck.c +--- pngcheck-2.4.0-original/pngcheck.c 2020-11-16 21:01:13.027413916 -0500 ++++ pngcheck-2.4.0/pngcheck.c 2020-11-16 21:01:20.417461088 -0500 +@@ -201,6 +201,8 @@ + char *keyword_name, char *chunkid, char *fname); + int check_text (uch *buffer, int maxsize, char *chunkid, char *fname); + int check_ascii_float (uch *buffer, int len, char *chunkid, char *fname); ++char const * u2name_helper(unsigned int value, const char **names, ++ size_t nnames); + + #define BS 32000 /* size of read block for CRC calculation (and zlib) */ + +@@ -231,6 +233,9 @@ + #define COLOR_WHITE "\033[40;37m" + + #define isASCIIalpha(x) (ascii_alpha_table[x] & 0x1) ++/* Map unsigned value to enumerated string name, safely with fallback */ ++#define U2NAME(x, names) (u2name_helper(x, &names[0], \ ++ sizeof(names) / sizeof(names[0]))) + + #define ANCILLARY(chunkID) ((chunkID)[0] & 0x20) + #define PRIVATE(chunkID) ((chunkID)[1] & 0x20) +@@ -1245,7 +1250,7 @@ + if (ityp == 2 || ityp == 4 || ityp == 6) { /* RGB or GA or RGBA */ + printf("%s invalid %ssample depth (%d) for %s image\n", + verbose? ":":fname, verbose? "":"IHDR ", sampledepth, +- png_type[ityp]); ++ U2NAME(ityp, png_type)); + set_err(kMinorError); + } + break; +@@ -1255,7 +1260,7 @@ + if (ityp == 3) { /* palette */ + printf("%s invalid %ssample depth (%d) for %s image\n", + verbose? ":":fname, verbose? "":"IHDR ", sampledepth, +- png_type[ityp]); ++ U2NAME(ityp, png_type)); + set_err(kMinorError); + } + break; +@@ -1309,7 +1314,7 @@ + } + if (verbose && no_err(kMinorError)) { + printf("\n %ld x %ld image, %d-bit %s, %sinterlaced\n", w, h, +- bitdepth, png_type[ityp], lace? "":"non-"); ++ bitdepth, U2NAME(ityp, png_type), lace? "":"non-"); + } + } + have_IHDR = 1; +@@ -1394,15 +1399,16 @@ + { + printf("%s invalid %salpha-channel bit depth (%d) for %s image\n" + , verbose? ":":fname, verbose? "":"JHDR ", alphadepth, +- jng_type[jtyp]); ++ U2NAME(jtyp, jng_type)); + set_err(kMinorError); + } else if (verbose && no_err(kMinorError)) { + if (jtyp < 2) + printf("\n %ld x %ld image, %d-bit %s%s%s\n", +- w, h, jbitd, and, jng_type[jtyp], lace? ", progressive":""); ++ w, h, jbitd, and, U2NAME(jtyp, jng_type), ++ lace? ", progressive":""); + else + printf("\n %ld x %ld image, %d-bit %s%s + %d-bit alpha%s\n", +- w, h, jbitd, and, jng_type[jtyp-2], alphadepth, ++ w, h, jbitd, and, U2NAME(jtyp-2, jng_type), alphadepth, + lace? ", progressive":""); + } + } +@@ -1588,7 +1594,7 @@ + set_err(kMinorError); + } else if (png && ityp != 3 && ityp != 2 && ityp != 6) { + printf("%s PLTE not allowed in %s image\n", verbose? ":":fname, +- png_type[ityp]); ++ U2NAME(ityp, png_type)); + set_err(kMinorError); + } else if (png && have_IDAT) { + printf("%s %smust precede IDAT\n", +@@ -1663,7 +1669,7 @@ + } + } else if (png && ityp == 3 && !have_PLTE) { + printf("%s %smust follow PLTE in %s image\n", +- verbose? ":":fname, verbose? "":"IDAT ", png_type[ityp]); ++ verbose? ":":fname, verbose? "":"IDAT ", U2NAME(ityp, png_type)); + set_err(kMajorError); + } else if (verbose) + printf("\n"); +@@ -1711,11 +1717,11 @@ + } else if (CM == 8) { + if (CINFO > 1) { + printf("deflated, %dK window, %s compression%s\n", +- (1 << (CINFO-2)), deflate_type[FLEVEL], ++ (1 << (CINFO-2)), U2NAME(FLEVEL, deflate_type), + FDICT? ", preset dictionary":""); + } else { + printf("deflated, %d-byte window, %s compression%s\n", +- (1 << (CINFO+8)), deflate_type[FLEVEL], ++ (1 << (CINFO+8)), U2NAME(FLEVEL, deflate_type), + FDICT? ", preset dictionary":""); + } + } else { +@@ -2893,7 +2899,8 @@ + set_err(kMinorError); + } + if (verbose && no_err(kMinorError)) { +- printf("\n rendering intent = %s\n", rendering_intent[buffer[0]]); ++ printf("\n rendering intent = %s\n", ++ U2NAME(buffer[0], rendering_intent)); + } + have_sRGB = 1; + last_is_IDAT = last_is_JDAT = 0; +@@ -3067,7 +3074,8 @@ + case 0: + if (sz != 2) { + printf("%s invalid %slength for %s image\n", +- verbose? ":":fname, verbose? "":"tRNS ", png_type[ityp]); ++ verbose? ":":fname, verbose? "":"tRNS ", ++ U2NAME(ityp, png_type)); + set_err(kMajorError); + } else if (verbose && no_err(kMinorError)) { + printf("\n gray = 0x%04x\n", SH(buffer)); +@@ -3076,7 +3084,8 @@ + case 2: + if (sz != 6) { + printf("%s invalid %slength for %s image\n", +- verbose? ":":fname, verbose? "":"tRNS ", png_type[ityp]); ++ verbose? ":":fname, verbose? "":"tRNS ", ++ U2NAME(ityp, png_type)); + set_err(kMajorError); + } else if (verbose && no_err(kMinorError)) { + printf("\n red = 0x%04x, green = 0x%04x, blue = 0x%04x\n", +@@ -3086,7 +3095,8 @@ + case 3: + if (sz > nplte) { + printf("%s invalid %slength for %s image\n", +- verbose? ":":fname, verbose? "":"tRNS ", png_type[ityp]); ++ verbose? ":":fname, verbose? "":"tRNS ", ++ U2NAME(ityp, png_type)); + set_err(kMajorError); + } else if ((verbose || (printpal && !quiet)) && no_err(kMinorError)) { + if (!verbose && printpal && !quiet) +@@ -3108,7 +3118,8 @@ + break; + default: + printf("%s %snot allowed in %s image\n", +- verbose? ":":fname, verbose? "":"tRNS ", png_type[ityp]); ++ verbose? ":":fname, verbose? "":"tRNS ", ++ U2NAME(ityp, png_type)); + set_err(kMinorError); + break; + } +@@ -3294,8 +3305,7 @@ + + printf("\n object ID = %u, image type = %s, delta type = %s\n", + SH(buffer), buffer[2]? "PNG":"unspecified", +- (dtype < sizeof(delta_type)/sizeof(char *))? +- delta_type[dtype] : inv); ++ U2NAME(dtype, delta_type)); + if (sz > 4) { + if (dtype == 7) { + printf("%s invalid %slength for delta type %d\n", +@@ -3340,8 +3350,7 @@ + uch fmode = buffer[0]; + + printf(": mode %d\n %s\n", fmode, +- (fmode < sizeof(framing_mode)/sizeof(char *))? +- framing_mode[fmode] : inv); ++ U2NAME(fmode, framing_mode)); + if (sz > 1) { + uch *p = buffer+1; + int bytes_left, found_null=0; +@@ -3379,7 +3388,7 @@ + set_err(kMinorError); + } else { + bytes_left -= 4; +- printf(" %s\n", change_interframe_delay[cid]); ++ printf(" %s\n", U2NAME(cid, change_interframe_delay)); + /* GRR: need real error-checking here: */ + if (cid && bytes_left >= 4) { + ulg delay = LG(p); +@@ -3389,7 +3398,7 @@ + p += 4; + bytes_left -= 4; + } +- printf(" %s\n", change_timeout_and_termination[ctt]); ++ printf(" %s\n", U2NAME(ctt, change_timeout_and_termination)); + /* GRR: need real error-checking here: */ + if (ctt && bytes_left >= 4) { + ulg val = LG(p); +@@ -3402,7 +3411,8 @@ + p += 4; + bytes_left -= 4; + } +- printf(" %s\n", change_subframe_clipping_boundaries[cscb]); ++ printf(" %s\n", ++ U2NAME(cscb, change_subframe_clipping_boundaries)); + /* GRR: need real error-checking here: */ + if (cscb && bytes_left >= 17) { + printf(" new frame clipping boundaries (%s):\n", (*p++)? +@@ -3413,7 +3423,7 @@ + p += 16; + bytes_left -= 17; + } +- printf(" %s\n", change_sync_id_list[csil]); ++ printf(" %s\n", U2NAME(csil, change_sync_id_list)); + if (csil) { + if (bytes_left) { + while (bytes_left >= 4) { +@@ -3469,8 +3479,7 @@ + set_err(kMinorError); + break; + } +- printf(" entry type = %s", (type < +- sizeof(entry_type)/sizeof(char *))? entry_type[type] : inv); ++ printf(" entry type = %s", U2NAME(type, entry_type)); + ++p; + if (type <= 1) { + ulg first4 = LG(p); +@@ -3705,8 +3714,7 @@ + printf("\n parent object ID = %u, clone object ID = %u\n", + SH(buffer), SH(buffer+2)); + printf(" clone type = %s, %s, %s\n", +- (ct < sizeof(clone_type)/sizeof(char *))? clone_type[ct] : inv, +- (dns < sizeof(do_not_show)/sizeof(char *))? do_not_show[dns] : inv, ++ U2NAME(ct, clone_type), U2NAME(dns, do_not_show), + cf? "same concreteness as parent":"abstract"); + if (ldt) + printf(" difference from parent's position: delta-x = %ld," +@@ -3743,8 +3751,7 @@ + if (sz > 4) + smode = buffer[4]; + printf("\n first object = %u, last object = %u\n", first, last); +- printf(" %s\n", +- (smode < sizeof(show_mode)/sizeof(char *))? show_mode[smode] : inv); ++ printf(" %s\n", U2NAME(smode, show_mode)); + } + last_is_IDAT = last_is_JDAT = 0; + +@@ -3786,7 +3793,8 @@ + if (verbose && no_err(kMinorError)) { + printf(": nest level = %u\n count = %lu, termination = %s\n", + (unsigned)(buffer[0]), LG(buffer+1), sz == 5? +- termination_condition[0] : termination_condition[buffer[5] & 0x3]); ++ termination_condition[0] : ++ U2NAME(buffer[5] & 0x3, termination_condition)); + /* GRR: not checking for valid buffer[1] values */ + if (sz > 6) { + printf(" iteration min = %lu", LG(buffer+6)); +@@ -3933,7 +3941,7 @@ + if (ityp == 2 || ityp == 4 || ityp == 6) { /* RGB or GA or RGBA */ + printf("%s invalid %sbit depth (%d) for %s image\n", + verbose? ":":fname, verbose? "":"BASI ", bitdepth, +- png_type[ityp]); ++ U2NAME(ityp, png_type)); + set_err(kMinorError); + } + break; +@@ -3943,7 +3951,7 @@ + if (ityp == 3) { /* palette */ + printf("%s invalid %sbit depth (%d) for %s image\n", + verbose? ":":fname, verbose? "":"BASI ", bitdepth, +- png_type[ityp]); ++ U2NAME(ityp, png_type)); + set_err(kMinorError); + } + break; +@@ -3967,7 +3975,8 @@ + } + if (verbose && no_err(kMinorError)) { + printf("\n %ld x %ld image, %d-bit %s, %sinterlaced\n", w, h, +- bitdepth, (ityp > 6)? png_type[1]:png_type[ityp], lace? "":"non-"); ++ bitdepth, (ityp > 6)? png_type[1]:U2NAME(ityp, png_type), ++ lace? "":"non-"); + } + if (sz > 13) { + ush red, green, blue; +@@ -4040,8 +4049,7 @@ + if (!verbose && printpal && !quiet) + printf(" PPLT chunk"); + if (verbose) +- printf(": %s\n", (dtype < sizeof(pplt_delta_type)/sizeof(char *))? +- pplt_delta_type[dtype] : inv); ++ printf(": %s\n", U2NAME(dtype, pplt_delta_type)); + plus = (dtype & 1)? "+" : ""; + if (dtype < 2) + samples = 3; +@@ -4162,8 +4170,9 @@ + + if (verbose) { + printf(" source ID = %u: composition mode = %s,\n", +- src_id, composition_mode[comp_mode]); +- printf(" orientation = %s,\n", orientation[orient >> 1]); ++ src_id, U2NAME(comp_mode, composition_mode)); ++ printf(" orientation = %s,\n", ++ U2NAME(orient >> 1, orientation)); + printf(" offset = {%ld,%ld} measured from {%ld,%ld} in " + "destination image,\n", xoff, yoff, + offset_origin? x:0, offset_origin? y:0); +@@ -4205,12 +4214,13 @@ + set_err(kMinorError); + } + if (verbose && no_err(kMinorError)) { +- printf("\n action = %s\n", termination_action[buffer[0] /* & 3 */]); ++ printf("\n action = %s\n", ++ U2NAME(buffer[0] /* & 3 */, termination_action)); + if (sz >= 10) { + ulg val = LG(buffer+2); + + printf(" action after iterations = %s\n", +- termination_action[buffer[1]]); ++ U2NAME(buffer[1], termination_action)); + printf(" inter-iteration delay = %lu tick%s, max iterations = ", + val, (val == 1)? "":"s"); + val = LG(buffer+6); +@@ -4443,7 +4453,7 @@ + if (!no_err(kMinorError)) + break; + if (verbose) +- printf(" %.*s: %s\n", 4, buf, order_type[buf[4]]); ++ printf(" %.*s: %s\n", 4, buf, U2NAME(buf[4], order_type)); + buf += 5; + bytes_left -= 5; + } +@@ -4517,10 +4527,11 @@ + else + printf("s = %u to %u\n", first, last); + if (xmeth == ymeth) +- printf(" method = %s\n", magnification_method[xmeth]); ++ printf(" method = %s\n", U2NAME(xmeth, magnification_method)); + else + printf(" X method = %s\n Y method = %s\n", +- magnification_method[xmeth], magnification_method[ymeth]); ++ U2NAME(xmeth, magnification_method), ++ U2NAME(ymeth, magnification_method)); + printf(" X mag = %u, left mag = %u, right mag = %u\n", + mx, ml, mr); + printf(" Y mag = %u, top mag = %u, bottom mag = %u\n", +@@ -4719,13 +4730,13 @@ + printf("%s: %s%s%s (%ldx%ld, %d-bit %s%s%s, %s%d.%d%%).\n", + global_error? brief_warn : brief_OK, + color? COLOR_YELLOW:"", fname, color? COLOR_NORMAL:"", +- w, h, jbitd, and, jng_type[jtyp], ++ w, h, jbitd, and, U2NAME(jtyp, jng_type), + lace? ", progressive":"", sgn, cfactor/10, cfactor%10); + else + printf("%s: %s%s%s (%ldx%ld, %d-bit %s%s + %d-bit alpha%s, %s%d.%d%%)" + ".\n", global_error? brief_warn : brief_OK, + color? COLOR_YELLOW:"", fname, color? COLOR_NORMAL:"", +- w, h, jbitd, and, jng_type[jtyp-2], ++ w, h, jbitd, and, U2NAME(jtyp-2, jng_type), + alphadepth, lace? ", progressive":"", sgn, cfactor/10, cfactor%10); + } + +@@ -4751,7 +4762,7 @@ + printf("%s: %s%s%s (%ldx%ld, %d-bit %s%s, %sinterlaced, %s%d.%d%%).\n", + global_error? brief_warn : brief_OK, + color? COLOR_YELLOW:"", fname, color? COLOR_NORMAL:"", +- w, h, bitdepth, (ityp > 6)? png_type[1] : png_type[ityp], ++ w, h, bitdepth, (ityp > 6)? png_type[1] : U2NAME(ityp, png_type), + (ityp == 3 && have_tRNS)? "+trns" : "", + lace? "" : "non-", sgn, cfactor/10, cfactor%10); + } +@@ -5137,3 +5148,8 @@ + + return rc; + } ++ ++char const * u2name_helper(unsigned int value, const char **names, ++ size_t nnames) { ++ return (value < nnames) ? names[value] : inv; ++} diff --git a/pngcheck.spec b/pngcheck.spec index 2858d84..6e27864 100644 --- a/pngcheck.spec +++ b/pngcheck.spec @@ -20,6 +20,9 @@ Patch0: %{name}-2.4.0-overflow-bz1897485.patch # https://bugzilla.redhat.com/show_bug.cgi?id=1902730 # Patch sent upstream by email to Greg Roelofs 2020-11-28. Patch1: %{name}-2.4.0-null-dereference-sCAL-no-height.patch +# Bounds-check all accesses into enumerated-value name arrays; a malformed file +# could have caused a buffer overrun in several of these cases. +Patch2: pngcheck-2.4.0-bounds-check-enumerated-names.patch BuildRequires: gcc BuildRequires: pkgconfig(zlib)