From da3bf7fa8b9f97a6a235ed38298a825df86c2690 Mon Sep 17 00:00:00 2001 From: "Gabriel A. Devenyi" Date: Fri, 14 Aug 2026 23:11:56 -0400 Subject: [PATCH 1/3] BUG: Fix an out-of-bounds indirect call in axio_show_mim_summary mind = get_map_index(xt->xchild[kid]); if( kid >= 0 ) MIM_disp_funcs[mind](ofp, xt->xchild[kid], verb); The guard tests kid, the loop counter, which is never negative, so it is always true. The index actually used is mind, and get_map_index() returns -1 for any element name not in MIM_kids[]. A CIFTI file containing an unrecognised element under MatrixIndicesMap therefore reads a function pointer from before the start of MIM_disp_funcs and calls it. Reported by the clang static analyzer as security.ArrayBound, "Out of bound access to memory preceding 'MIM_disp_funcs'". Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KSPnbwpDjVcAYqDdVqLkMU --- cifti/afni_xml_io.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cifti/afni_xml_io.c b/cifti/afni_xml_io.c index a59f5c3e..a06229e4 100644 --- a/cifti/afni_xml_io.c +++ b/cifti/afni_xml_io.c @@ -263,7 +263,8 @@ int axio_show_mim_summary(FILE * fp, const char * mesg, afni_xml_t * ax, int ver for( kid=0; kidnchild; kid++ ) { mind = get_map_index(xt->xchild[kid]); - if( kid >= 0 ) MIM_disp_funcs[mind](ofp, xt->xchild[kid], verb); + /* get_map_index() returns -1 for an unrecognized element name */ + if( mind >= 0 ) MIM_disp_funcs[mind](ofp, xt->xchild[kid], verb); } } From e2f30ed4a5ad4cacbee10035f1f9a919af6a6460 Mon Sep 17 00:00:00 2001 From: "Hans J. Johnson" Date: Tue, 22 Sep 2026 06:27:42 -0500 Subject: [PATCH 2/3] ENH: Cover the MatrixIndicesMap dispatch in axio_show_mim_summary Drives cifti_tool with -eval_type show_summary over two XML fixtures. The first holds a MatrixIndicesMap child whose name get_map_index() does not resolve, and fails without the preceding commit: the sanitizer legs report a global-buffer-overflow, an 8-byte read 8 bytes before MIM_disp_funcs. The second holds a BrainModel and asserts it is still displayed, so that skipping the unresolved name cannot pass by skipping every name. These are the first tests for the cifti library. The fixtures are small enough to keep in tree, so neither needs the external testing data. --- cifti/CMakeLists.txt | 16 ++++++++++++++++ cifti/testdata/mim_known_child.xml | 7 +++++++ cifti/testdata/mim_unknown_child.xml | 7 +++++++ 3 files changed, 30 insertions(+) create mode 100644 cifti/testdata/mim_known_child.xml create mode 100644 cifti/testdata/mim_unknown_child.xml diff --git a/cifti/CMakeLists.txt b/cifti/CMakeLists.txt index d19b7524..9e539c20 100644 --- a/cifti/CMakeLists.txt +++ b/cifti/CMakeLists.txt @@ -26,3 +26,19 @@ if(NIFTI_BUILD_APPLICATIONS) install_nifti_target(${NIFTI_PACKAGE_PREFIX}afni_xml_tool) install_nifti_target(${NIFTI_PACKAGE_PREFIX}cifti_tool) endif() + +if(NIFTI_BUILD_TESTING AND NIFTI_BUILD_APPLICATIONS) + set(TEST_PREFIX "${NIFTI_PACKAGE_PREFIX}cifti") + # An unrecognized MatrixIndicesMap child is skipped, not dispatched on. + add_test( NAME ${TEST_PREFIX}_mim_summary_unknown_child + COMMAND $ + -as_cext -eval_cext -eval_type show_summary + -input ${CMAKE_CURRENT_LIST_DIR}/testdata/mim_unknown_child.xml ) + # A recognized child is still displayed. + add_test( NAME ${TEST_PREFIX}_mim_summary_known_child + COMMAND $ + -as_cext -eval_cext -eval_type show_summary + -input ${CMAKE_CURRENT_LIST_DIR}/testdata/mim_known_child.xml ) + set_tests_properties( ${TEST_PREFIX}_mim_summary_known_child + PROPERTIES PASS_REGULAR_EXPRESSION "BrainModel" ) +endif() diff --git a/cifti/testdata/mim_known_child.xml b/cifti/testdata/mim_known_child.xml new file mode 100644 index 00000000..30eecbdf --- /dev/null +++ b/cifti/testdata/mim_known_child.xml @@ -0,0 +1,7 @@ + + + + + + + diff --git a/cifti/testdata/mim_unknown_child.xml b/cifti/testdata/mim_unknown_child.xml new file mode 100644 index 00000000..83c64772 --- /dev/null +++ b/cifti/testdata/mim_unknown_child.xml @@ -0,0 +1,7 @@ + + + + + + + From 4027649d2a8c64f9d93dea3297367f3e828a4bc6 Mon Sep 17 00:00:00 2001 From: "Hans J. Johnson" Date: Tue, 22 Sep 2026 06:39:14 -0500 Subject: [PATCH 3/3] BUG: Free the XML tree and image that cifti_tool parses process() returned without releasing either the afni_xml_t it parsed or the nifti_image the non-cext path fills in, so every run leaked the whole tree. Both free routines already accept NULL, so neither path needs a guard. LeakSanitizer is on by default under AddressSanitizer on Linux but not on Apple, so the leak ended the process with a non-zero status on the sanitizer job alone. That also discarded the buffered standard output, which is why the summary text went missing there rather than merely being followed by a leak report. --- cifti/cifti_tool.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cifti/cifti_tool.c b/cifti/cifti_tool.c index 488b70fb..b895c09f 100644 --- a/cifti/cifti_tool.c +++ b/cifti/cifti_tool.c @@ -193,6 +193,9 @@ int process(opts_t * opts) if( opts->disp_cext ) disp_cifti_extension(nim, opts); if( opts->eval_cext ) eval_cifti_extension(ax, opts); + axml_free_xml_t(ax); + nifti_image_free(nim); + return 0; }