From 8bc103394fb2cdaf608fca74113a7773f53a8bd4 Mon Sep 17 00:00:00 2001 From: "Gabriel A. Devenyi" Date: Fri, 14 Aug 2026 23:50:01 -0400 Subject: [PATCH 1/2] BUG: Stop returning -1 from functions that return size_t znzread(), znzwrite() and nifti_read_buffer() all return size_t and all returned -1 to report an error. In a size_t that value is SIZE_MAX, larger than any length a caller can have asked for, so a caller that tests the result with '<' reads the error as a complete transfer: ii = nifti_read_buffer(fp, nim->data, ntot, nim); if( ii < ntot ){ ... } /* SIZE_MAX < ntot is false */ The visible effect is that a truncated image loads as if it were whole: nifti_image_read() prints its short-read warning, ignores the failure and returns an image whose tail is uninitialized heap. FslReadVolumes() has the same problem one level up, dividing the returned byte count by the volume size to report SIZE_MAX/volbytes volumes read. These now return 0, which every one of these callers already treats as failure and which is the only value a size_t function has for "nothing was transferred". The published return types in znzlib.h and nifti1_io.h are unchanged. The nifti2 equivalents return int64_t, where -1 is representable, and are left alone. (cherry picked from commit b344fe40f478a55c23b9769869721f4183be8821) --- niftilib/nifti1_io.c | 4 ++-- znzlib/znzlib.c | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/niftilib/nifti1_io.c b/niftilib/nifti1_io.c index eb8887f3..7792d635 100644 --- a/niftilib/nifti1_io.c +++ b/niftilib/nifti1_io.c @@ -5019,7 +5019,7 @@ size_t nifti_read_buffer(znzFile fp, void* dataptr, size_t ntot, if( dataptr == NULL ){ if( g_opts.debug > 0 ) fprintf(stderr,"** ERROR: nifti_read_buffer: NULL dataptr\n"); - return -1; + return 0; } ii = znzread( dataptr , 1 , ntot , fp ) ; /* data input */ @@ -5034,7 +5034,7 @@ size_t nifti_read_buffer(znzFile fp, void* dataptr, size_t ntot, nim->iname , (unsigned int)ntot , (unsigned int)ii , (unsigned int)(ntot-ii) ) ; /* memset( (char *)(dataptr)+ii , 0 , ntot-ii ) ; now failure [rickr] */ - return -1 ; + return 0 ; } if( g_opts.debug > 2 ) diff --git a/znzlib/znzlib.c b/znzlib/znzlib.c index 52b5665d..300a631e 100644 --- a/znzlib/znzlib.c +++ b/znzlib/znzlib.c @@ -145,7 +145,8 @@ size_t znzread(void* buf, size_t size, size_t nmemb, znzFile file) while( remain > 0 ) { n2read = (remain < ZNZ_MAX_BLOCK_SIZE) ? (unsigned)remain : ZNZ_MAX_BLOCK_SIZE; nread = gzread(file->zfptr, (void *)cbuf, n2read); - if( nread < 0 ) return nread; /* returns -1 on error */ + /* 0, not gzread's -1: this returns size_t, where -1 is SIZE_MAX. */ + if( nread < 0 ) return 0; remain -= nread; cbuf += nread; @@ -178,8 +179,8 @@ size_t znzwrite(const void* buf, size_t size, size_t nmemb, znzFile file) n2write = (remain < ZNZ_MAX_BLOCK_SIZE) ? (unsigned)remain : ZNZ_MAX_BLOCK_SIZE; nwritten = gzwrite(file->zfptr, (const void *)cbuf, n2write); - /* gzread returns 0 on error, but in case that ever changes... */ - if( nwritten < 0 ) return nwritten; + /* gzwrite returns 0 on error, but in case that ever changes... */ + if( nwritten < 0 ) return 0; remain -= nwritten; cbuf += nwritten; From d9f5d2df54b6d6a6e3fdcb7093bc89d1608453a7 Mon Sep 17 00:00:00 2001 From: "Hans J. Johnson" Date: Tue, 22 Sep 2026 08:50:13 -0500 Subject: [PATCH 2/2] ENH: Test that a truncated image is rejected rather than accepted Writes a 31x31x31 float image, copies it 1000 bytes short, and asserts nifti_image_read() returns NULL for the copy and a populated image for the original, so a blanket refusal cannot pass. Without the fix the truncated file comes back accepted with nvox=29791 and an uninitialized tail. (cherry picked from commit 9b31abf78b148947ad3f4b73bd0b09b93be33d4d) --- niftilib/CMakeLists.txt | 7 +++ niftilib/nifti_short_read_test.c | 89 ++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 niftilib/nifti_short_read_test.c diff --git a/niftilib/CMakeLists.txt b/niftilib/CMakeLists.txt index 09d522d2..bdef6587 100644 --- a/niftilib/CMakeLists.txt +++ b/niftilib/CMakeLists.txt @@ -40,6 +40,13 @@ if(NIFTI_BUILD_TESTING AND NIFTI_BUILD_APPLICATIONS) add_executable(nifti_first_test_program nifti_tester001.c) target_link_libraries(nifti_first_test_program ${NIFTI_PACKAGE_PREFIX}niftiio ) + add_executable(${NIFTI_PACKAGE_PREFIX}nifti_short_read_test nifti_short_read_test.c) + target_link_libraries(${NIFTI_PACKAGE_PREFIX}nifti_short_read_test ${NIFTI_NIFTILIB_NAME}) + add_test( + NAME nifti_short_read_rejected + COMMAND $ + ) + add_executable(nifti_second_test_program nifti_tester002.c) target_link_libraries(nifti_second_test_program ${NIFTI_PACKAGE_PREFIX}niftiio ) diff --git a/niftilib/nifti_short_read_test.c b/niftilib/nifti_short_read_test.c new file mode 100644 index 00000000..809fb684 --- /dev/null +++ b/niftilib/nifti_short_read_test.c @@ -0,0 +1,89 @@ +/* A .nii whose data section is short of the header's declared size must be + rejected, not handed back with uninitialized tail bytes. */ + +#include +#include +#include "nifti1_io.h" + +static const char *WHOLE = "short_read_whole.nii"; +static const char *SHORT = "short_read_short.nii"; +static const long MISSING = 1000; + +static int write_whole(void) +{ + int dims[8] = { 3, 31, 31, 31, 1, 1, 1, 1 }; + nifti_image *nim = nifti_make_new_nim(dims, DT_FLOAT32, 1); + if( nim == NULL ) return 1; + if( nifti_set_filenames(nim, WHOLE, 0, 1) != 0 ){ + nifti_image_free(nim); + return 1; + } + nifti_image_write(nim); + nifti_image_free(nim); + return 0; +} + +static int copy_truncated(void) +{ + FILE *in = fopen(WHOLE, "rb"), *out; + long size; + char *buf; + size_t got; + + if( in == NULL ) return 1; + fseek(in, 0, SEEK_END); + size = ftell(in); + fseek(in, 0, SEEK_SET); + if( size <= MISSING ){ fclose(in); return 1; } + + buf = (char *)malloc((size_t)size); + if( buf == NULL ){ fclose(in); return 1; } + got = fread(buf, 1, (size_t)size, in); + fclose(in); + if( got != (size_t)size ){ free(buf); return 1; } + + out = fopen(SHORT, "wb"); + if( out == NULL ){ free(buf); return 1; } + fwrite(buf, 1, (size_t)(size - MISSING), out); + fclose(out); + free(buf); + return 0; +} + +int main(void) +{ + nifti_image *nim; + + if( write_whole() ){ + fprintf(stderr, "FAILURE: could not write the reference image\n"); + return 1; + } + if( copy_truncated() ){ + fprintf(stderr, "FAILURE: could not write the truncated copy\n"); + return 1; + } + + nim = nifti_image_read(SHORT, 1); + if( nim != NULL ){ + fprintf(stderr, "FAILURE: truncated image was accepted, nvox=%d\n", + (int)nim->nvox); + nifti_image_free(nim); + return 1; + } + + /* the rejection must be specific to the short read */ + nim = nifti_image_read(WHOLE, 1); + if( nim == NULL ){ + fprintf(stderr, "FAILURE: the whole image was rejected\n"); + return 1; + } + if( nim->data == NULL ){ + fprintf(stderr, "FAILURE: the whole image came back with no data\n"); + nifti_image_free(nim); + return 1; + } + nifti_image_free(nim); + + printf("Short-read rejection test passed.\n"); + return 0; +}