BUG: Swap a nifti_1_header as a NIFTI-1 header, not as whatever its magic claims - #107
Open
hjmjohnson wants to merge 2 commits into
Open
hjmjohnson wants to merge 2 commits into
hjmjohnson wants to merge 2 commits into
Conversation
…agic claims
swap_nifti_header() takes a void pointer and a version number, and picks
the struct to swap from the version:
if ( ni_ver == 0 ) nifti_swap_as_analyze((nifti_analyze75 *)hdr);
else if( ni_ver == 1 ) nifti_swap_as_nifti1((nifti_1_header *)hdr);
else if( ni_ver == 2 ) nifti_swap_as_nifti2((nifti_2_header *)hdr);
Two callers pass it a nifti_1_header, 348 bytes, together with the
version taken from that header's own magic string by NIFTI_VERSION().
A file whose magic is "n+2" therefore has 540 bytes swapped in place:
192 bytes of read and write past the end of the caller's stack object.
nifti_read_n1_hdr() reads the header straight from a file, so the bytes
that decide this come from the file being read. A 348 byte file with
sizeof_hdr = 348, dim[0] byte-swapped, magic = "n+2"
is enough; need_nhdr_swap() then reports that swapping is needed and the
overflow happens before any validity check. AddressSanitizer:
ERROR: AddressSanitizer: stack-buffer-overflow
READ of size 1 ...
#0 nifti_swap_4bytes nifti2_io.c:3051
#1 nifti_swap_as_nifti2 nifti2_io.c:3194
#2 nifti_read_n1_hdr nifti2_io.c:5394
Address ... is located in stack of thread T0 at offset 412 in frame
[64, 412) 'nhdr' (line 5339) <== Memory access ... overflows
nifti_convert_n1hdr2nim() has the same line and takes its header from
the caller, so any application that fills a nifti_1_header itself can
reach it too.
Both call sites know the struct they hold, so both now ask for the swap
that struct supports: analyze when the magic is absent, NIFTI-1
otherwise. Headers claiming versions 3 to 9, which swap_nifti_header()
previously refused with a message and left unswapped, are now swapped as
NIFTI-1 as well, which is the only interpretation 348 bytes allow.
nifti1_io.c is not affected: its swap_nifti_header() takes a
nifti_1_header * and a flag, so it cannot choose a wider struct.
Found by fuzzing nifti_convert_n1hdr2nim() with clang's libFuzzer under
AddressSanitizer.
(cherry picked from commit 16a6b67)
nifti_tool.c has the same defect this branch fixes in the library: it
passes NIFTI_VERSION(*nhdr), read from the magic string, to
swap_nifti_header(), which selects the struct to swap by that number.
act_mod_hdrs() and act_swap_hdrs() hold a nifti_1_header of 348 bytes.
A magic of "n+2" makes NIFTI_VERSION() return 2, so swap_nifti_header()
treats the allocation as a 540 byte nifti_2_header and reads and writes
past its end. Both functions refuse a header that is valid NIFTI-2, but
a header that is valid as neither version reaches the call.
$ printf ... > evil.nii # 348 bytes, sizeof_hdr byte swapped,
# magic "n+2"
$ nifti_tool -mod_hdr -mod_field descrip hello -overwrite \
-infiles evil.nii
==2747364==ERROR: AddressSanitizer: heap-buffer-overflow
#0 nifti_swap_4bytes nifti2_io.c:3029
#1 nifti_swap_as_nifti2 nifti2_io.c:3172
#2 swap_nifti_header nifti2_io.c:3131
#3 act_mod_hdrs nifti_tool.c:3389
0 bytes after 348-byte region allocated in nifti_read_n1_hdr()
Both now pass 1, or 0 for ANALYZE, which are the two 348 byte layouts.
act_mod_hdr2s() holds a nifti_2_header and gets the explicit 2, as
act_swap_hdrs() already does for its own NIFTI-2 display path.
old_swap_nifti_header() takes a nifti_1_header and a boolean, so the
calls beside these need no change.
(cherry picked from commit 522e624)
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 #63, 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.