The internal read code change to stop sharing the palette was incompletely
implemented. The result is that unless palette index checking is turned off and
there are no read transformations the png_info palette gets deleted when the
png_struct is deleted. This is normally harmless (png_info gets deleted first)
but in the case of pngcp it results in use-after-free of the palette and,
therefore, palette corruption and maybe on some operating systems and access
violation.
This also updated pngcp 'search' mode to check a restricted range of memLevels;
there is an unrelated bug which means that lower zlib memLevels result in memory
corruption under some circumstances, probably less often than 1:1000.
Signed-off-by: John Bowler <jbowler@acm.org>
Also change the order of the 'level' and 'windowBits' searches to seach
windowBits first; this favours windowBits optimizations over compression level
ones on the basis that the latter should only affect the write code.
Signed-off-by: John Bowler <jbowler@acm.org>
The sequential read code failed to read to the end of the IDAT stream in about
1/820 cases, resulting in a spurious warning. The
png_set_compression_buffer_size API also would not work (or do bad things) if
the size of a zlib uInt was less than 32 bits.
This includes a quiet API change to alter png_set_compression_buffer_size to use
a png_alloc_size_t, not png_size_t and implement the correct checks.
Signed-off-by: John Bowler <jbowler@acm.org>
png_set_compression_buffer_size would result in a spurious debug assert if the
compression buffer size was set to something other than a multiple of
PNG_ROW_BUFFER_SIZE; the debug test failed to add the buffer 'start'
Signed-off-by: John Bowler <jbowler@acm.org>
This is still a work-in-progress but it seems fairly stable (if not exactly 100%
optimal). pngcp now allows 'all' for some options which iterates through all
possible settings (this reliably produces the smallest IDAT that libpng can
produce with those settings.) It also contains a --search command line option
which attempts to optimize this by skipping pointless tests; it is close, most
of the time, but not perfect.
Signed-off-by: John Bowler <jbowler@acm.org>
There are two separate problems. The first is that the CMINFO optimization code
gets run twice on any PNG IDAT stream longer than 2048 bytes and the second time
can overwrite bytes 2048,2049 destroying the output.
The second is that one of the (debug) checks was slightly wrong (< when <=
should have been used) and this causes write to abort maybe 1/2048 times.
Signed-off-by: John Bowler <jbowler@acm.org>
The handling of tIME and text chunks on read now records the location of the
chunks relative to PLTE and IDAT. Behavior on write is unchanged except that if
the position was recorded on read it will be re-used.
This involves an ABI change to the png_text_struct; a one byte location field is
added (with the same meaning as the one used to record unknown chunk location.)
Because this field is only used on read there is no API change unless a png_info
from a libpng read is passed to a subsequent libpng write (this did not work
very well before 1.7; the tIME chunk could get duplicated.)
png_set_text ignores the new field, resetting it to the current position in the
read or write stream. On write the position is set to the next location to be
written unless the write has not started (the position is before the signature)
in which case the location is set to PNG_HAVE_PLTE|PNG_AFTER_IDAT. When the
chunk is written the position is set to the actual write location (effectively
the position is frozen.)
Signed-off-by: John Bowler <jbowler@acm.org>
This adds code to iterate through the command line options when non-list options
have the 'all' parameter. Used to rapidly test the effect of different zlib
options.
Signed-off-by: John Bowler <jbowler@acm.org>
This seems safer; in fact a NULL dereference never happens because the test ends
up just doing arithmetic on NULL but it seems possible that the undefined
arithmetic can still happen if there is a bug elsewhere.
Signed-off-by: John Bowler <jbowler@acm.org>
Undefined behaviour sanitizer currently fires for this test
with an error about member access within null pointer to
struct; put the check that relies on pz->list being non-NULL
second.
Also add an example program, pngcp.c, which illustrates the problem when used to
copy pngtest.png (the result is an invalid PNG because the tIME chunk is
duplicated.)
Signed-off-by: John Bowler <jbowler@acm.org>
This rewrites the code used previously in the heuristics to make it easier to
debug and introduces the 'methodical' method, which is intended to be an
expensive but reliable way of reducing image size.
The code in this commit does not work; the 'methodical' test for success does
not take account of data buffered inside zlib and, anyway, it changes the
results of pngtest so that the test fails. This commit is just a checkpoint of
the current state; another commit will temporarily disable the 'methodical'
code.
Signed-off-by: John Bowler <jbowler@acm.org>
Undefined because (png_byte) << shift is undefined if it changes the signed bit
(because png_byte is promoted to int). The libpng exported functions
png_get_uint_32 and png_get_uint_16 handle this.
Bug reported by David Drysdale as a result of reports from UBSAN in clang 3.8.
This changes pngvalid to use BE random numbers; this used to produce errors but
these should not be fixed as a result of the previous changes.
Signed-off-by: John Bowler <jbowler@acm.org>
