From 1e81f62b8334824dbcd763b9d1ea08c26e0ea533 Mon Sep 17 00:00:00 2001 From: "Gabriel A. Devenyi" Date: Sat, 15 Aug 2026 00:27:23 -0400 Subject: [PATCH] BUG: Free the header on nifti_tool's duplicate-file failure paths act_mod_hdrs(), act_mod_hdr2s() and act_swap_hdrs() -- five functions across the two tools -- read a header, then, when -prefix is given, duplicate the dataset before writing the modified header back. Each of the three ways that duplication can fail returns without freeing the header, and the last of them also loses the strdup'd duplicate name: nhdr = nt_read_header(fname, &nver, &swap, 0, ...); ... if( opts->prefix ) { nim = nt_image_read(opts, fname, 1, 1); if( !nim ) { fprintf(...); return 1; } /* nhdr */ if( nifti_set_filenames(nim, opts->prefix, 1, 1) ) { nifti_image_free(nim); return 1; /* nhdr */ } dupname = nifti_strdup(nim->fname); if( nifti_image_write_status(nim) ) { nifti_image_free(nim); return 1; /* nhdr, dupname */ } } ... free(dupname); free(nhdr); The normal path frees both. Reproduced by pointing -prefix at a directory that cannot be written: nifti_tool -mod_hdr -prefix /anat1 -infiles anat0.nii \ -mod_field qoffset_x -17.325 before: definitely lost: 348 bytes in 1 blocks after: ERROR SUMMARY: 0 errors from 0 contexts Separately, act_diff_nims() in both tools releases the first image with free(nim0) when reading the second one fails. That is a shallow free: it loses nim0's fname, iname and any data or extensions. It now calls nifti_image_free() like the success path six lines below. Found by running the test suite under valgrind. On the normal paths the suite is clean -- 484 traced processes, no invalid access, no uninitialised value and no leak in any nifti binary -- so these are error-path defects that the tests do not otherwise reach. (cherry picked from commit 40009a92db6f327703a357412f91079460f3dd79) --- nifti2/nifti_tool.c | 14 +++++++++++++- niftilib/nifti1_tool.c | 10 +++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/nifti2/nifti_tool.c b/nifti2/nifti_tool.c index ac249bf8..f964d099 100644 --- a/nifti2/nifti_tool.c +++ b/nifti2/nifti_tool.c @@ -2843,7 +2843,7 @@ int act_diff_nims( nt_opts * opts ) if( ! nim0 ) return 1; /* errors have been printed */ nim1 = nt_image_read(opts, opts->infiles.list[1], 0, 0); - if( ! nim1 ){ free(nim0); return 1; } + if( ! nim1 ){ nifti_image_free(nim0); return 1; } if( g_debug > 1 ) fprintf(stderr,"\n-d nifti_image diffs between '%s' and '%s'...\n", @@ -3359,6 +3359,7 @@ int act_mod_hdrs( nt_opts * opts ) if( !nim ) { fprintf(stderr,"** failed to dup file '%s' before modifying\n", fname); + free(nhdr); return 1; } @@ -3369,6 +3370,7 @@ int act_mod_hdrs( nt_opts * opts ) { NTL_FERR(func,"failed to set prefix for new file: ",opts->prefix); nifti_image_free(nim); + free(nhdr); return 1; } dupname = nifti_strdup(nim->fname); /* so we know to free it */ @@ -3377,6 +3379,8 @@ int act_mod_hdrs( nt_opts * opts ) if( nifti_image_write_status(nim) ) { fprintf(stderr,"** failed to write image %s\n", nim->fname); nifti_image_free(nim); + free(dupname); + free(nhdr); return 1; } @@ -3477,6 +3481,7 @@ int act_mod_hdr2s( nt_opts * opts ) if( !nim ) { fprintf(stderr,"** failed to dup file '%s' before modifying\n", fname); + free(nhdr); return 1; } if( opts->keep_hist && nifti_add_extension(nim, opts->command, @@ -3486,6 +3491,7 @@ int act_mod_hdr2s( nt_opts * opts ) { NTL_FERR(func,"failed to set prefix for new file: ",opts->prefix); nifti_image_free(nim); + free(nhdr); return 1; } dupname = nifti_strdup(nim->fname); /* so we know to free it */ @@ -3494,6 +3500,8 @@ int act_mod_hdr2s( nt_opts * opts ) if( nifti_image_write_status(nim) ) { fprintf(stderr,"** failed to write image %s\n", nim->fname); nifti_image_free(nim); + free(dupname); + free(nhdr); return 1; } @@ -3623,6 +3631,7 @@ int act_swap_hdrs( nt_opts * opts ) if( !nim ) { fprintf(stderr,"** failed to dup file '%s' before modifying\n", fname); + free(nhdr); return 1; } if( opts->keep_hist && nifti_add_extension(nim, opts->command, @@ -3632,6 +3641,7 @@ int act_swap_hdrs( nt_opts * opts ) { NTL_FERR(func,"failed to set prefix for new file: ",opts->prefix); nifti_image_free(nim); + free(nhdr); return 1; } dupname = nifti_strdup(nim->fname); /* so we know to free it */ @@ -3640,6 +3650,8 @@ int act_swap_hdrs( nt_opts * opts ) if( nifti_image_write_status(nim) ) { fprintf(stderr,"** failed to write image %s\n", nim->fname); nifti_image_free(nim); + free(dupname); + free(nhdr); return 1; } diff --git a/niftilib/nifti1_tool.c b/niftilib/nifti1_tool.c index 15bb165a..5588b9c4 100644 --- a/niftilib/nifti1_tool.c +++ b/niftilib/nifti1_tool.c @@ -2277,7 +2277,7 @@ int act_diff_nims( nt_opts * opts ) if( ! nim0 ) return 1; /* errors have been printed */ nim1 = nt_image_read(opts, opts->infiles.list[1], 0); - if( ! nim1 ){ free(nim0); return 1; } + if( ! nim1 ){ nifti_image_free(nim0); return 1; } if( g_debug > 1 ) fprintf(stderr,"\n-d nifti_image diffs between '%s' and '%s'...\n", @@ -2606,6 +2606,7 @@ int act_mod_hdrs( nt_opts * opts ) if( !nim ) { fprintf(stderr,"** failed to dup file '%s' before modifying\n", fname); + free(nhdr); return 1; } if( opts->keep_hist && nifti_add_extension(nim, opts->command, @@ -2615,6 +2616,7 @@ int act_mod_hdrs( nt_opts * opts ) { NTL_FERR(func,"failed to set prefix for new file: ",opts->prefix); nifti_image_free(nim); + free(nhdr); return 1; } dupname = nifti_strdup(nim->fname); /* so we know to free it */ @@ -2623,6 +2625,8 @@ int act_mod_hdrs( nt_opts * opts ) if( nifti_image_write_status(nim) ) { fprintf(stderr,"** failed to write image %s\n", nim->fname); nifti_image_free(nim); + free(dupname); + free(nhdr); return 1; } @@ -2727,6 +2731,7 @@ int act_swap_hdrs( nt_opts * opts ) if( !nim ) { fprintf(stderr,"** failed to dup file '%s' before modifying\n", fname); + free(nhdr); return 1; } if( opts->keep_hist && nifti_add_extension(nim, opts->command, @@ -2736,6 +2741,7 @@ int act_swap_hdrs( nt_opts * opts ) { NTL_FERR(func,"failed to set prefix for new file: ",opts->prefix); nifti_image_free(nim); + free(nhdr); return 1; } dupname = nifti_strdup(nim->fname); /* so we know to free it */ @@ -2744,6 +2750,8 @@ int act_swap_hdrs( nt_opts * opts ) if( nifti_image_write_status(nim) ) { fprintf(stderr,"** failed to write image %s\n", nim->fname); nifti_image_free(nim); + free(dupname); + free(nhdr); return 1; }