libpng icon indicating copy to clipboard operation
libpng copied to clipboard

Two possible Integer Overflows based on pixel depth

Open dns43 opened this issue 4 years ago • 2 comments

png_ptr->pixel_depth and png_ptr->rowbytes may be overflown in the following lines:

https://sourcegraph.com/github.com/glennrp/libpng@dbe3e0c43e549a1602286144d94b0666549b18e6/-/blob/pngrutil.c#L902

https://sourcegraph.com/github.com/glennrp/libpng@dbe3e0c43e549a1602286144d94b0666549b18e6/-/blob/pngrutil.c#L903 ( (size_t)(width) * (size_t)(pixel_bits) inside PNG_ROWBYTES )

dns43 avatar Feb 23 '21 06:02 dns43

png_ptr->pixel_depth and png_ptr->rowbytes may be overflown in the following lines:

https://sourcegraph.com/github.com/glennrp/libpng@dbe3e0c43e549a1602286144d94b0666549b18e6/-/blob/pngrutil.c#L902

How can this overflow? bit_depth is an int, and the maximum possible value for channels is 4, which is set immediately above the code you quote, and the maximum value for bit_depth is 16 here but it's set from a byte value so it could never be more than 256 even from a maliciously edited PNG file.

https://sourcegraph.com/github.com/glennrp/libpng@dbe3e0c43e549a1602286144d94b0666549b18e6/-/blob/pngrutil.c#L903 ( (size_t)(width) * (size_t)(pixel_bits) inside PNG_ROWBYTES )

The width value is checked against user_width_max which has a default value of 1_000_000 in png.c when calling png_get_IHDR, so an overflowing value should be caught at some point, I don't know if that is before or after this calculation though.

benkasminbullock avatar Feb 23 '21 07:02 benkasminbullock

Sorry for the poor bug description! Yes, bit_depth is checked to be <= 16, but the check happens after it is used. The function sequence is handle_IHDR() -> set_IHDR() -> check_IHDR() E.g. a large value of buf[8] in handle_IHDR() can cause an unexpected value for png_ptr->pixel_depth.

dns43 avatar Feb 26 '21 17:02 dns43