Re: Endless loop in version 1.6.26

On Fri, Dec 30, 2016 at 6:07 AM, Glenn Randers-Pehrson <[email protected]>
 wrote:

> I was expecting this change to cause a regression of mozilla bug #1300661
> but it doesn't. It seems that I neglected to back out the change from the
> progressive reader, libpng16/pngpread.c lines 844-847:
>

This is the 'if' block that starts at line 673 in the git head (which is
now 1.6.28beta01).

I think the png_warning which precedes this is saying the wrong thing; it
says the compressed data is *truncated* but the test is for compressed data
after the last image byte has been delivered.

At this point there has been a detectable error in the stream (the zlib
return code) and, either, all the image has been output or the image is
truncated.

The tricky part is that it is necessary to read the compressed stream
beyond the end of the image because at the end of the image (when the last
image byte has been delivered) there are still at least four bytes in the
deflate stream.

In any case the original test of the zlib return code is in the wrong
place; libpng should check first to see if it has got all the required data
for the next row using zstruct::next_out/avail_out before it starts to
worry about the zlib return code.

The logic should be to process all the available data, padding an
incomplete row with stuff (for security), then decide whether a previously
detected problem (too little data, too much data, a zlib stream error) is
to be ignored, warned or errored.  How the problem was detected does not
give libpng enough information to know what the problem actually is and the
errors it produces are just misleading.

Changing the code to do this is, however, difficult.  Despite the
simplicity of its interface the zlib {next,avail}_{in.out} stuff makes
error handling more complex and getting it right in the presence of damaged
or weirdly constructed streams (e.g. extra 0 length IDAT changes) is tricky.

I'd recommend leaving it as it is with just the Mozilla fix to do
png_benign_error/png_error according to the png_set_option setting.  This
does mean that the captcha will, I think, have garbage in the last row
(libpng doesn't do the last png_push_process_row) but I assume that Mozilla
have signed off on the fix as is.  Technically the whole image has garbage
in it (i.e. libpng can't be sure it isn't corrupted from the first byte.)

-- 
John Bowler <[email protected]>
+1 (541) 450-9885
PO BOX 3151
KERBY OR 97531-3151
USA

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