Re: Endless loop in version 1.6.26

Pushed a changeset to the libpng16 branch of the GIT repositories, to add
PNG_IGNORE_ADLER32 option, and use that instead of
png_set_crc_action() to turn off adler32 evaluation by zlib (note that
as before, this also requires zlib-1.2.8.1 or later, or it's all defined
out).

Glenn


On Mon, Dec 26, 2016 at 1:11 PM, John Bowler <
[email protected]> wrote:

> 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
>
>

------------------------------------------------------------------------------
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