Apply the same class of robustness fix from the previous commit to
`png_set_text`, `png_set_sPLT` and `png_set_unknown_chunks`. These
append-style setters used `png_realloc_array` to grow the internal
array, then freed the old array before copying from the caller's
input. If the caller's pointer was obtained from the corresponding
getter, it aliased the freed array.
The fix defers the freeing of the old array until after the copy loop.
Also extend the pnggetset regression test to cover all three setters.
Apply a robustness fix for a caller-side API usage pattern involving
the getters and the setters for PLTE, tRNS, and hIST.
Passing a pointer returned by the PLTE, tRNS, or hIST getters back
into the corresponding setters used to cause the setters to read from
a stale pointer. The fix consists in snapshotting the caller's data
into a stack-local buffer before freeing the old internal storage.
Fixespnggroup/libpng#836
Reported-by: Iv4n <Iv4n550@noreply.github.com>
Guard the pointer advance with `y+1 < height` inside the function
`compare_two_images` to skip this advance on the last iteration.
With a negative stride, the unconditional `row += stride` in the
for-statement produced a pointer before the allocated object on
the final iteration. Standard C permits one-after-end but not
one-before-beginning; this is undefined behavior regardless of
whether the pointer is dereferenced or not.
Add the option --negative-stride to pngstest. When set, the row stride
is negated after buffer allocation but before calling the library, so
that `png_image_finish_read` and `png_image_write_to_file` exercise the
bottom-up (negative stride) code paths.
Add CI targets for the CMake build and for the configure build:
- pngstest-negative-stride:
Bottom-up layout with images covering colormapped, truecolor, alpha,
8-bit, 16-bit, and short-height paths.
- pngstest-negative-stride-extra:
Same with --stride-extra 7 for non-aligned padding with bottom-up
layout.
Note: the interlaced pngsuite images (`ibasn*.png`) are incompatible
with pngstest's format conversion comparison framework, so the
`png_image_read_direct_scaled` path (interlaced 16-to-8 conversion) is
not exercised by this test. A dedicated test for that path will require
interlaced images generated with the correct gamma properties for
pngstest's comparison logic, which is TODO.
Replace the calls to `png_set_scale_16`, `png_set_packing` and
`png_set_expand`, which were incorrectly combined with
`png_read_png(..., PNG_TRANSFORM_IDENTITY, ...)`, with the equivalent
`PNG_TRANSFORM_*` flags passed directly to `png_read_png`.
The libpng manual states that applications must use transforms, and not
call any `png_set_*` transform functions when they use `png_read_png`.
Reviewed-by: Cosmin Truta <ctruta@gmail.com>
Signed-off-by: Cosmin Truta <ctruta@gmail.com>
- Fix row buffer leak: the row buffer, allocated after `setjmp`, was
not freed when `png_read_row` triggered `longjmp`. Declare `row` as
volatile before `setjmp` and free it in the error handler. This fixes
a LeakSanitizer false positive that prevents the fuzzer from starting.
- Fix palette buffer leak (same pattern as the row buffer leak):
declare `palette` as volatile before `setjmp` and free it in the
error handler.
- Fix API misuse: replace `png_set_*` calls before `png_read_png` with
`PNG_TRANSFORM_*` flags.
Reviewed-by: Cosmin Truta <ctruta@gmail.com>
Signed-off-by: Cosmin Truta <ctruta@gmail.com>
Split compound declarations of pointer variables for improved clarity.
Rename local loop boundary variables, remove their sterile const
qualifiers, and reorder their declarations to improve local cohesion.
This is a cherry-pick of commit 1ebf432e85
from branch 'libpng18'.
Remove the top-level const qualifiers from local variables and from
function parameters passed by value. These qualifiers only constrain
the local object within the function body: they neither affect function
signatures, nor constrain callers, nor protect any shared state.
Throughout the libpng codebase we apply const only where it
meaningfully constrains objects beyond the immediate scope, such as
true constants and pointed-to objects.
This is a cherry-pick of commit e973362b8b
from branch 'libpng18'.
The type of the row stride (`display->row_bytes`) is ptrdiff_t. Casting
to png_uint_16 before division will truncate large strides, causing
incorrect pointer arithmetic for images exceeding 65535 bytes per row.
For bottom-up images (negative stride), the truncation also corrupts
the sign, advancing the row pointer forward instead of backward.
Remove the erroneous casts and let the compiler handle the pointer
arithmetic correctly. Also replace `sizeof (png_uint_16)` with 2.
Add regression test via `pngstest --stride-extra N` where N > 32767
triggers the affected code paths.
A NOTE ABOUT HISTORY:
The original code in libpng 1.5.6 (2011) had no such casts. They were
introduced in libpng 1.6.26 (2016), likely to silence compiler warnings
on 16-bit systems where the cast would be a no-op. On 32/64-bit systems
the cast truncates the strides above 65535 and corrupts the negative
strides.
Integrate nalloc fuzzing to systematically test error handling paths
when memory allocations fail. This technique previously found a memory
leak in pCAL chunk handling and is already in production use via
OSS-Fuzz on libwebp and libssh.
This is enabled via the `_nalloc` target suffix or the `NALLOC_FREQ`
environment variable.
Reviewed-by: Cosmin Truta <ctruta@gmail.com>
Signed-off-by: Cosmin Truta <ctruta@gmail.com>
Split compound declarations of pointer variables for improved clarity.
Rename local loop boundary variables, remove their sterile const
qualifiers, and reorder their declarations to improve local cohesion.
There are some proposals to move to a new zlib backend.
My first step in writing a patch for this was to shed some of the
compatibility if statements.
Zlib 1.2.8 was release in 2013 according to the archive history
https://zlib.net/fossils/
I think that 12 years is pretty generous and should help shed some mental
load
Remove the top-level const qualifiers from local variables and from
function parameters passed by value. These qualifiers only constrain
the local object within the function body: they neither affect function
signatures, nor constrain callers, nor protect any shared state.
Throughout the libpng codebase we apply const only where it
meaningfully constrains objects beyond the immediate scope, such as
true constants and pointed-to objects.
In the previous commits
- "refactor: Replace deprecated pointer typedefs (part 1)"
- "refactor: Replace deprecated pointer typedefs (part 2)"
the volatile qualifiers in the rewritten declarations were mistakenly
placed on the left side of declaration types, effectively making them
bottom-level. (Oopsie!)
Move these qualifiers to the right side, restoring their intended
top-level status.
Following up on commit 7c67f70da1:
Replace 22 deprecated pointer typedefs with their underlying types.
The typedefs replaced are:
1. Core structure types:
- png_structp, png_structrp, png_structpp and const variants
- png_infop, png_inforp, png_infopp and const variants
2. Utility structure types:
- png_row_infop, png_row_infopp
- png_imagep, png_imagepp
- png_controlp
3. Color structure types:
- png_colorp, png_colorpp and const variants
- png_color_8p, png_color_8pp and const variants
- png_color_16p, png_color_16pp and const variants
4. Text and time structure types:
- png_textp, png_textpp and const variants
- png_timep, png_timepp and const variants
5. Chunk structure types:
- png_unknown_chunkp, png_unknown_chunkpp and const variants
- png_sPLT_tp, png_sPLT_tpp and const variants
- png_sPLT_entryp, png_sPLT_entrypp and const variants
6. Internal types:
- png_compression_bufferp
- png_dsortp, png_dsortpp
Remove all remaining uses of spurious PNG_RESTRICT qualifiers.
Non-aliasing has already been guaranteed by the type-based aliasing
rules combined with the libpng usage pattern of a single pointer per
type.
Update the user manual accordingly.
This completes the systematic replacement of deprecated pointer
typedefs across the entire libpng codebase. The typedef declarations
remain in png.h for backward compatibility, marked as deprecated.
Reviewed-by: Chris Blume <ProgramMax@gmail.com>
Signed-off-by: Cosmin Truta <ctruta@gmail.com>
Following up on commit ddce3ff853:
1. Replace 28 deprecated pointer typedefs with their underlying types.
The typedefs replaced are:
- Basic: png_voidp, png_charp and const variants
- Byte: png_bytep, png_const_bytep, png_bytepp
- Numeric:
- Integer: 16/32-bit integer pointer variants
- Fixed-point: png_fixed_point_p and variants
- Floating-point: png_doublep and variants
2. Remove a spurious PNG_RESTRICT qualifier. Non-aliasing has already
been guaranteed by the type-based aliasing rules.
3. Update the user manual accordingly.
Reviewed-by: Chris Blume <ProgramMax@gmail.com>
Signed-off-by: Cosmin Truta <ctruta@gmail.com>
