BUG: Bound the formatted write in znzprintf, and end the va_list on failure - #110
Open
hjmjohnson wants to merge 4 commits into
Open
hjmjohnson wants to merge 4 commits into
hjmjohnson wants to merge 4 commits into
Conversation
znzprintf() built its output with vsprintf(), which has no bound; the buffer was sized strlen(format) + 1000000 under a comment reading "overkill I hope". A single %s argument longer than a megabyte writes past the end of the allocation. vsnprintf() with the size already being computed turns that overflow into a reported truncation. The same block also returned on calloc failure without calling va_end(), leaving the va_list unterminated. This is the only unbounded formatted write in the tree; there is no sprintf() and no gets() anywhere. (cherry picked from commit 57a61a7)
The preceding commit bounds the write but still returned the gzprintf count after a truncation, so a caller could not tell. It also returned 0 for an allocation failure, and the function returns 0 for a NULL stream. The printf family reports failure with a negative value; 0 is a successful empty write, so it cannot carry either meaning. All three now return -1, and a truncated result is not written at all: a partial record reported as a whole one is worse than nothing. znzprintf sits inside COMPILE_NIFTIUNUSED_CODE in both znzlib.c and znzlib.h and is absent from libznz.a in a default build, so this is a latent defect in code no released configuration compiles. (cherry picked from commit ac7ec09)
znzprintf has no caller in this tree and is compiled only under COMPILE_NIFTIUNUSED_CODE, so nothing reached it. The test builds its own copy of znzlib.c with that definition and drives a 2 MB argument through the 1000001 byte buffer. Without the bound the write overflows the allocation, and the call reports success; the test pins the negative return, and pins the ordinary write and the resulting file content so an over-broad fix that refuses everything does not pass. (cherry picked from commit 0350b30)
znzflush() and znzeof() are defined inside COMPILE_NIFTIUNUSED_CODE and declared nowhere, unlike znzgets(), znzputc() and znzgetc() beside them. Nothing compiled that block until a test in this branch did, so the omission was invisible. They are declared where the rest of the block is declared, so the warning set the project already requires stays clean when the block is built. (cherry picked from commit 8449a1f)
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
3 participants
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.
Re-submission of #41, reverted from
masteron 2026-09-24 so it can bereviewed before merging. Content is unchanged from the original.
Base:
master. Independent: nothing has to land before it.Commits
Ordering for all the re-submitted work is tracked in #84.