BUG: Do not use sscanf's output when sscanf matched nothing - #115
Open
hjmjohnson wants to merge 1 commit into
Open
hjmjohnson wants to merge 1 commit into
hjmjohnson wants to merge 1 commit into
Conversation
nifti_image_from_ascii() scans its input three times with
ii = sscanf( str+spos , "%1023s%n" , lhs , &nn ) ; spos += nn ;
if( ii == 0 || strcmp(lhs,"<nifti_image") != 0 ) return NULL ;
Two things are wrong with that pair of lines. sscanf() returns EOF,
not 0, when the input ends before anything is matched, so ii == 0 does
not detect the failure; and nn is added to spos before ii is looked
at, so the position advances by an indeterminate amount.
For a string holding nothing but whitespace the first scan returns
EOF, the test passes, and strcmp() reads the uninitialized 1024 byte
stack buffer. MemorySanitizer:
WARNING: MemorySanitizer: use-of-uninitialized-value
#0 nifti_image_from_ascii nifti2_io.c:8836
All three now test for the one successful conversion and only then
advance spos, which is what the surrounding code assumed. The two
inside the loop cannot fail today, because whitespace is skipped and
end of string is checked before each, and the in-tree readers gate on
has_ascii_header(), so the first is reached only through a direct call
to this published function. The same three lines exist in
nifti1_io.c and are corrected there too.
Found by fuzzing nifti_image_from_ascii() with libFuzzer under
MemorySanitizer.
(cherry picked from commit 70fa3a6)
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 #67, 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.