accept a possible problematic png image silently

[boofish]

Hi, thank you for libspng! I recently did a comparasion of different png libs (libpng, lodepng), and found libspng very robust except some cases that other libs report an error or a warnning and libspng accepts it. I test it with example.c and the png file is attached test.zip!

Specifically, libpng report a warning libpng warning: IDAT: invalid distance too far back under its function png_read_IDAT_data at png_read_IDAT_data, and lodepng reject it with an error at inflateHuffmanBlock.
I also tested it with other online checker, such as extract-metadata, it also reports a warnning message Warning: [minor] Trailer data after PNG IEND chunk .
It seems that libspng should check something of the chunk but I am not sure, also unclear whether it will have other effects.

Hope it helps.

[boofish]

also with this case : test2.zip

[randy408]

I believe this has to do with invalid zlib headers, if you set SPNG_IMG_WINDOW_BITS to 0 before spng_decode_image() then zlib will take into account the (potentially invalid) window size instead of assuming the largest possible size. It will turn into a decode error if the window size ends up being too small.

It makes a small memory usage difference to ignore the (potentially invalid) window size so it's ignored by default.

[boofish]

Thank you for your reply!

I tried SPNG_IMG_WINDOW_BITS to 0 by spng_set_option(ctx, SPNG_IMG_WINDOW_BITS, 0) before spng_decoded_image_size(ctx, fmt, &image_size),
but it still can not report the decode error.

[boofish]

Hi, I further checked the details of libpng's implementation, and found some possible useful information.
Specifically, libpng use the function png_zlib_inflate(png_ptr, 0) to decode the zlib stream, and some comments before the function.
I quote it here

/* Handle the start of the inflate stream if we called inflateInit2(strm,0);
 * in this case some zlib versions skip validation of the CINFO field and, in
 * certain circumstances, libpng may end up displaying an invalid image, in
 * contrast to implementations that call zlib in the normal way (e.g. libpng
 * 1.5).
 */
int /* PRIVATE */
png_zlib_inflate(png_structrp png_ptr, int flush)
{
   if (png_ptr->zstream_start && png_ptr->zstream.avail_in > 0)
   {
      if ((*png_ptr->zstream.next_in >> 4) > 7)
      {
         png_ptr->zstream.msg = "invalid window size (libpng)";
         return Z_DATA_ERROR;
      }

      png_ptr->zstream_start = 0;
   }

   return inflate(&png_ptr->zstream, flush);
}

Hope this helps.

[randy408]

That's not where zlib is initialized, this is the init code with InflateReset2()/InflateInit2(): https://github.com/glennrp/libpng/blob/v1.6.39/pngrutil.c#L361-L422

The linked code explains why libpng doesn't ignore the window size:

the original writer of the PNG may have selected a lower window size, and we really must follow that because, for systems with with limited capabilities, we would otherwise reject the application's attempts to use a smaller window size (zlib doesn't have an interface to say "this or lower"!).

I don't agree with this. Sometimes the wrong size is written to the header but the image data itself is not corrupted. It's better to assume the worst and use a 32KB window and let users set SPNG_IMG_WINDOW_BITS to 0 on platforms where those few kilobytes matter.

libpng warning: IDAT: invalid distance too far back

This message is misleading by the way, it should be an error, if you run this project's testsuite on test.png the images don't match: index mismatch at x: 4 y: 1 spng: 2 png: 0.

Why doesn't spng fail with window bits set 0? Could be some edge case, the images are very small and the libraries don't use zlib exactly the same way. Both test images are paletted and 2 bits per sample, if you get back the same values you encoded then it's not a bug.