[Zlib-devel] known bugs in distance-checking code?

[Mark Adler]

On Feb 23, 2005, at 8:59 AM, Greg Roelofs wrote:
> Could you pop this one through
> your tools, too?  It dies after 100 bytes:
>
> 	http://audio.rightmark.org/test/audiotrak-prodigy192-1644/thd.png

That one doesn't have any literals at all before the first match!

gromit% pngdat < thd.png | infgen
! infgen 1.0 output
!
zlib
!
last
dynamic
code 0 7
...
dist 29 6
match 258 258
match 257 257
literal 1 1 1
match 3 3
...

> I fully agree that we need to flag these, and in the case that they're
> actually reading beyond the bounds of the allocated array (not the case
> here?), we should barf rather than allow unpredictable results.

The old inflate assured that the copies were always done from within 
the sliding window, which was treated as circular.

> What we're likely to see is patches that hack the code to change the
> error to a warning (and maybe that force the buffer to be initialized
> or whatever).  Some guidance from you on the most proper compromise
> might be a good idea. ;-)

That may be a little difficult, since one of the ways that the new 
inflate is faster than the old one is that it doesn't even create a 
sliding window until it needs to, and then only uses it sparingly.  As 
a result, when inflate first starts up, it uses the user-supplied 
output buffer as the window.  That means without the distance check, it 
would try to access memory before the user-supplied output buffer, 
which may cause an exception.  There would need to be special code 
added to write zeros for matches with distances too far back.

I still think it's far better to refuse to display bad png images.  
That will correct itself in time.  If we put in patches to allow them, 
then we are deliberately encouraging bad png images.

mark