BUG: Refuse a header whose dimensions overflow the voxel count - #106
Open
hjmjohnson wants to merge 2 commits into
Open
hjmjohnson wants to merge 2 commits into
hjmjohnson wants to merge 2 commits into
Conversation
Both header converters multiply the dimensions into nim->nvox without
checking the product:
for( ii=1, nim->nvox=1; ii <= nhdr.dim[0]; ii++ )
nim->nvox *= nhdr.dim[ii];
Seven NIFTI-1 dimensions of 32767 are enough to overflow int64_t, and a
NIFTI-2 header needs only two dimensions to do it:
nifti2_io.c:4838: runtime error: signed integer overflow:
1152780773560811521 * 32767 cannot be represented in type 'int64_t'
Signed overflow is undefined, and what the compiler does produce is a
voxel count that no longer describes the file. nvox then goes on to
nifti_get_volsize(), which multiplies it by nbyper for another
unchecked product, and that result is used as an allocation size and a
read length.
Both products are now checked before they are made, in the way the
surrounding code already reports a bad header. A header that overflows
is rejected with a message instead of producing a silently wrong image.
No valid image is affected: an int64_t voxel count is more than any
file can hold.
Found by fuzzing the header converters with clang's libFuzzer under
UndefinedBehaviorSanitizer.
(cherry picked from commit 66c76c2)
The first commit guards both converters in nifti2/nifti2_io.c.
niftilib/nifti1_io.c has the same loop in nifti_convert_nhdr2nim(), on
the same untrusted path, and was left unguarded.
$ nifti1_tool -disp_nim -infiles huge.nii # dim[0]=7, dim[1..7]=32767
dim 32 8 7 32767 32767 32767 32767 32767 32767 32767
nvox 64 1 -1073512449
The wrapped count then becomes the data allocation size and the result
of nifti_get_volsize(). With the guard the header is refused:
** ERROR: nifti_convert_nhdr2nim: dim[] overflows the voxel count
nifti_image.nvox is a size_t in this library and an int64_t in the
NIFTI-2 one, so the bound here is SIZE_MAX rather than INT64_MAX.
nifti_get_volsize() is size_t * size_t to match. dim[0] is already
bounded by need_nhdr_swap(), and the loop that raises every dim to at
least 1 still runs first, so the dim[ii] > 0 test never has to reject.
Two more nvox loops remain in each library, in
nifti_update_dims_from_array() and update_nifti_image_for_brick_list().
Both take dims that a caller has already set rather than dims read from
a file, and reach them from nifti_tool's command line, so they are left
for a separate change.
(cherry picked from commit 147a07a)
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 #68, 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.