InsightSoftwareConsortium / InsightSoftwareConsortium/ITK
PNGImageIO::WriteSlice: destroys existing file on mid-stream failures, leaks libpng structs on error, rowInc overflow for extreme widths
- Dominant language
- C++
- Stars
- 1.7k
- Forks
- 748
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 64
Description
Three pre-existing defects remain in `PNGImageIO::WriteSlice` after the early-validation fixes in #6614 (B70/B72 of #6575). All are in error paths that run after the output file has been opened, or in row-stride arithmetic.
1. Existing file still destroyed on mid-stream write failures
`PNGFileWrapper pngfp(fileName.c_str(), "wb")` truncates any existing file at the destination path. #6614 moved the component-type and component-count validation ahead of this open, but every failure mode after it still destroys the prior file:
- `png_create_write_struct` / `png_create_info_struct` failure
- any libpng error reaching the `setjmp` handler (e.g. disk full during `png_write_image`)
In those cases an exception is thrown *and* the original file has been replaced by an empty or partial PNG. A complete fix likely means writing to a temporary file and renaming into place on success.
2. setjmp error path leaks libpng structs (and palette buffer)
The `setjmp(png_jmpbuf(png_ptr))` branch throws via `itkExceptionMacro` without calling `png_destroy_write_struct(&png_ptr, &info_ptr)`, and when palette writing is enabled the `png_malloc`'d palette is never `png_free`'d. Every libpng write error leaks the write/info structs; an application that retries failed writes (e.g. a series writer looping slices onto a full disk) leaks on each attempt.
3. rowInc integer overflow for extreme image widths
`rowInc = width * numComp * bitDepth / 8` is computed in (unsigned then narrowed to) `int`. PNG allows widths up to 2^31-1, so a 16-bit RGBA slice with width ≥ 2^28 overflows `rowInc`, producing wrong `row_pointers` strides and out-of-bounds reads in `png_write_image`. The computation should use `size_t`/`SizeValueType` throughout.
Contributor guide
Research direction
Read Modules/IO/PNG/src/itkPNGImageIO.cxx around WriteSlice, especially the file open near line 535, setjmp handler near line 561, and rowInc near line 675. Trace each failure path and verify that failed writes preserve the destination, libpng resources and palette memory are released, and row-stride arithmetic remains safe for extreme widths.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- computer-vision
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100