Fix memory and file handle leaks in contrib/visupng, and a compile error in contrib/gregbook - #910
Open
krleejihyeong wants to merge 3 commits into
Open
Conversation
- PngFile.c: free pbImageData in PngLoadImage error path before nulling it - rpng2-win.c: free image_data/row_pointers when rpng2_win_create_window() fails in rpng2_win_init()
…gbook - contrib/visupng/PngFile.c: PngLoadImage() error path frees the libpng read struct but never frees the already-allocated pbImageData buffer, leaking it on any PNG read error. - contrib/gregbook/rpng2-win.c: rpng2_win_init() has a long-standing (since 2017, commit 53f22ae) unterminated fprintf() call that produces a compile error, undetected because this Windows-only file is not part of libpng's CI/CMake/autotools build.
krleejihyeong
changed the title
Three error paths in PngLoadImage() (bad PNG signature, png_create_read_struct failure, png_create_info_struct failure) returned FALSE without closing the already-opened pfFile handle.
krleejihyeong
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes three independent issues found while manually reviewing contrib/ example code:
contrib/visupng/PngFile.c — PngLoadImage()
On any PNG read error, the Catch() block destroys the libpng read
struct but never frees the already-allocated pbImageData buffer
before nulling out the pointer, leaking it on every failed read.
contrib/gregbook/rpng2-win.c — rpng2_win_init()
Line 654 has an unterminated fprintf() call (missing a closing
parenthesis before the following statement), which is a compile
error. This has been present since commit 53f22ae (2017,
"check for integer overflow when allocating a pixel buffer") and
went unnoticed because this Windows-only file is built only via
contrib/gregbook/Makefile.w32 / Makefile.mingw32, which are not
part of libpng's CI, CMake, or autotools build.
contrib/visupng/PngFile.c — PngLoadImage()
Three early-return error paths (invalid PNG signature,
png_create_read_struct() failure, png_create_info_struct()
failure) leave the file handle opened by fopen() unclosed.
These paths return before entering the Try/Catch block, which
is where the function's normal cleanup (fclose) happens.
Testing: Manual code review and control-flow tracing for all three
issues. The compile error in #2 was confirmed via git blame and by
comparing against the correct fprintf(...) + separate-statement
pattern used elsewhere in the same file (e.g. lines 586-588).