diff --git a/contrib/libtests/pngunknown.c b/contrib/libtests/pngunknown.c index 8b1528296..d20d112e8 100644 --- a/contrib/libtests/pngunknown.c +++ b/contrib/libtests/pngunknown.c @@ -478,7 +478,7 @@ get_valid(display *d, png_infop info_ptr) png_textp text; png_uint_32 ntext = png_get_text(d->png_ptr, info_ptr, &text, NULL); - while (ntext-- > 0) switch (text[ntext].compression) + while (ntext > 0) switch (text[--ntext].compression) { case -1: flags |= PNG_INFO_tEXt; diff --git a/contrib/libtests/pngvalid.c b/contrib/libtests/pngvalid.c index 9d3beb5fa..2f2be9ec0 100644 --- a/contrib/libtests/pngvalid.c +++ b/contrib/libtests/pngvalid.c @@ -1242,7 +1242,7 @@ store_image_check(const png_store* ps, png_const_structp pp, int iImage) image += 2; /* skip image first row markers */ - while (rows-- > 0) + for (; rows > 0; --rows) { if (image[-2] != 190 || image[-1] != 239) png_error(pp, "row start overwritten"); @@ -11427,23 +11427,36 @@ perform_interlace_macro_validation(void) */ for (v=0;;) { + /* The first two tests overflow if the pass row or column is outside + * the possible range for a 32-bit result. In fact the values should + * never be outside the range for a 31-bit result, but checking for 32 + * bits here ensures that if an app uses a bogus pass row or column + * (just so long as it fits in a 32 bit integer) it won't get a + * possibly dangerous overflow. + */ /* First the base 0 stuff: */ - m = PNG_ROW_FROM_PASS_ROW(v, pass); - f = png_row_from_pass_row(v, pass); - if (m != f) + if (v < png_pass_rows(0xFFFFFFFFU, pass)) { - fprintf(stderr, "PNG_ROW_FROM_PASS_ROW(%u, %d) = %u != %x\n", - v, pass, m, f); - exit(99); + m = PNG_ROW_FROM_PASS_ROW(v, pass); + f = png_row_from_pass_row(v, pass); + if (m != f) + { + fprintf(stderr, "PNG_ROW_FROM_PASS_ROW(%u, %d) = %u != %x\n", + v, pass, m, f); + exit(99); + } } - m = PNG_COL_FROM_PASS_COL(v, pass); - f = png_col_from_pass_col(v, pass); - if (m != f) + if (v < png_pass_cols(0xFFFFFFFFU, pass)) { - fprintf(stderr, "PNG_COL_FROM_PASS_COL(%u, %d) = %u != %x\n", - v, pass, m, f); - exit(99); + m = PNG_COL_FROM_PASS_COL(v, pass); + f = png_col_from_pass_col(v, pass); + if (m != f) + { + fprintf(stderr, "PNG_COL_FROM_PASS_COL(%u, %d) = %u != %x\n", + v, pass, m, f); + exit(99); + } } m = PNG_ROW_IN_INTERLACE_PASS(v, pass); diff --git a/pngpriv.h b/pngpriv.h index ded59c0ab..233e62ed5 100644 --- a/pngpriv.h +++ b/pngpriv.h @@ -669,6 +669,24 @@ ((png_size_t)(width) * (((png_size_t)(pixel_bits)) >> 3)) : \ (( ((png_size_t)(width) * ((png_size_t)(pixel_bits))) + 7) >> 3) ) +/* This returns the number of trailing bits in the last byte of a row, 0 if the + * last byte is completely full of pixels. It is, in principle, (pixel_bits x + * width) % 8, but that would overflow for large 'width'. The second macro is + * the same except that it returns the number of unused bits in the last byte; + * (8-TRAILBITS), but 0 when TRAILBITS is 0. + * + * NOTE: these macros are intended to be self-evidently correct and never + * overflow on the assumption that pixel_bits is in the range 0..255. The + * arguments are evaluated only once and they can be signed (e.g. as a result of + * the integral promotions). The result of the expression always has type + * (png_uint_32), however the compiler always knows it is in the range 0..7. + */ +#define PNG_TRAILBITS(pixel_bits, width) \ + (((pixel_bits) * ((width) % (png_uint_32)8)) % 8) + +#define PNG_PADBITS(pixel_bits, width) \ + ((8 - PNG_TRAILBITS(pixel_bits, width)) % 8) + /* PNG_OUT_OF_RANGE returns true if value is outside the range * ideal-delta..ideal+delta. Each argument is evaluated twice. * "ideal" and "delta" should be constants, normally simple diff --git a/pngread.c b/pngread.c index a5374e5b8..85ca2353c 100644 --- a/pngread.c +++ b/pngread.c @@ -3231,7 +3231,7 @@ png_image_read_colormapped(png_voidp argument) png_uint_32 y = image->height; png_bytep row = png_voidcast(png_bytep, display->first_row); - while (y-- > 0) + for (; y > 0; --y) { png_read_row(png_ptr, row, NULL); row += row_bytes; @@ -4064,7 +4064,7 @@ png_image_read_direct(png_voidp argument) png_uint_32 y = image->height; png_bytep row = png_voidcast(png_bytep, display->first_row); - while (y-- > 0) + for (; y > 0; --y) { png_read_row(png_ptr, row, NULL); row += row_bytes; diff --git a/pngtrans.c b/pngtrans.c index 880c88892..e649f6601 100644 --- a/pngtrans.c +++ b/pngtrans.c @@ -693,7 +693,7 @@ png_do_check_palette_indexes(png_structrp png_ptr, png_row_infop row_info) * and this calculation is used because it avoids warnings that other * forms produced on either GCC or MSVC. */ - int padding = ((256 - row_info->pixel_depth) * (row_info->width & 7)) & 7; + int padding = PNG_PADBITS(row_info->pixel_depth, row_info->width); png_bytep rp = png_ptr->row_buf + row_info->rowbytes; switch (row_info->bit_depth) diff --git a/pngwrite.c b/pngwrite.c index aaa2b017d..db6ff61a7 100644 --- a/pngwrite.c +++ b/pngwrite.c @@ -1554,7 +1554,7 @@ png_write_image_16bit(png_voidp argument) */ row_end = output_row + image->width * (channels+1); - while (y-- > 0) + for (; y > 0; --y) { png_const_uint_16p in_ptr = input_row; png_uint_16p out_ptr = output_row; @@ -1705,7 +1705,7 @@ png_write_image_8bit(png_voidp argument) /* Use row_end in place of a loop counter: */ row_end = output_row + image->width * (channels+1); - while (y-- > 0) + for (; y > 0; --y) { png_const_uint_16p in_ptr = input_row; png_bytep out_ptr = output_row; @@ -1746,7 +1746,7 @@ png_write_image_8bit(png_voidp argument) */ png_bytep row_end = output_row + image->width * channels; - while (y-- > 0) + for (; y > 0; --y) { png_const_uint_16p in_ptr = input_row; png_bytep out_ptr = output_row; @@ -2136,7 +2136,7 @@ png_image_write_main(png_voidp argument) ptrdiff_t row_bytes = display->row_bytes; png_uint_32 y = image->height; - while (y-- > 0) + for (; y > 0; --y) { png_write_row(png_ptr, row); row += row_bytes;