Re: Endless loop in version 1.6.26

Misquoting Gresham's law; "Bad software drives out good."

On Mon, Dec 26, 2016 at 8:17 AM, Glenn Randers-Pehrson <[email protected]>
wrote:


> It's motivated by some CAPTCHA-generator that doesn't bother to write a
> correct ADLER32 checksum,
> and apparently won't be bothered to fix it.
>

That's not the only thing that causes the checksum to mismatch.  It's a
checksum of the *original* (uncompressed) data, so when it mismatches all
we know is that the data zlib just uncompressed doesn't match the checksum.

The most likely explanation of this is that an undetectable change to the
deflate stream has occurred; there are any number of one-byte modifications
within the stream that are undetectable by zlib and just result in changes
(potential quite large ones) in the output data.   The checksum then fails
but nothing else does.

In this case the CRC may also be correct or it may be wrong, depending on
where the problem originated and whether someone fixed up the CRCs.

We were doing it with png_set_crc_action() but it wouldn't be a problem
> using a new
>  png_set_adler32_action(png_ptr, ERROR|WARN_USE|QUIET_USE) or similar API
> instead.
>

I think that is unnecessary complexity and is, anyway, confusing.  Errors
that cause the checksum to fail will often also cause an earlier error or
an truncation or extension (which is not always detected) of the data.  I
don't think there are any non-malicious changes that are even remotely
likely to cause the checksum to fail so turning off the checksum without
also disabling the detectable cases is just confusing.  There *is* a
recovery for every error in the IDAT stream.

In other words in the absence of malicious attacks the Adler32 checksum
reliably says, "You have the image data the original author encoded."
 There is only one alternative to this; "You have an image that matches the
IHDR."  It's definitely pointy-haired-manager when someone asks for a third
case to exist.

Hence a self documenting API might be:

png_set_handle_decode_errors(pp)

Of course, it doesn't matter much what it's called and there is some merit
to self-documenting the *request* then hiding the details of what can be
implemented in response inside the source code and detailed 'CHANGES'.
That way the pointy-haired-managers get what they want and the API
programmers can do what they were told both regardless of impossibility.


> 2) One advantage of png_error is that it changes a combinatorial test
>> problem into a linear one.  With png_error any detectable error in a
>> deflate stream stops the decoding.  However with png_warning different
>> combinations of errors cause potentially different code paths which all
>> need to be tested; this is difficult.
>>
>
> Fighting that in ImageMagick right now, there's a request to plunge ahead
> with a bad image returned by the PNG decoder instead of aborting, and I
> feel that this comment is pertinent.
>

I think that's reasonable, hence the changes in 1.7.

With JPEG errors libjpeg automatically resyncs on error (well, it used to)
and this results in output images with partial or skipped blocks.  This is
moderately obvious in the output but the result is still usable in many
cases.

PNG is a little different from JPEG; deflate doesn't resync as well and,
anyway, the content of a PNG is normally less amenable to such data loss
than the content of a JPEG.  Nevertheless it's a useful option and is
probably the most useful one for things like browsers and image editors
alike.

In 1.6 recovery is possible at present within the application; it's not
necessary to change libpng.  The image rows passed to libpng will be
partially filled in, but the app has to ensure that the remainder of the
data is initialized.  The way to do this is to simply memset the whole
thing to the byte of the app's choice before calling libpng.  If libpng
errors out the app stashes the error message and, after the longjmp,
displays the data with a warning to the user that there was a decode
problem.

It is possible to detect exactly where the error occurred with a check-code
on the original data.  Rather than memseting it set it to some complex code
(e.g. an OpenSSL 16-byte random number) and track back through the image on
error to the first changed byte.  This works in all versions of libpng.

The libpng error message is unpredictable and, unfortunately, neither
localized nor consistent across versions, but it's still potentially useful.

John Bowler

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

_______________________________________________
png-mng-implement mailing list
png-mng-implement-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/png-mng-implement