From 2e3b3eaa90b858b37373196ab42769e1100cfb2d Mon Sep 17 00:00:00 2001 From: "Gabriel A. Devenyi" Date: Fri, 14 Aug 2026 23:05:46 -0400 Subject: [PATCH 1/2] BUG: Fix cdfbin's argument range check, which could never fire cdfbin() validates its `which` selector like this: if(!(*which < 1 && *which > 4)) goto S30; A value cannot be both less than 1 and greater than 4, so the condition is always false, the negation is always true, and the jump to S30 is always taken -- skipping the entire range check. gcc reports it as -Wlogical-op, "logical 'and' of mutually exclusive tests is always false". The ten sibling functions in this file all spell the same guard with `||`: cdfbet 1527: if(!(*which < 1 || *which > 4)) goto S30; cdfchi 2255: if(!(*which < 1 || *which > 3)) goto S30; cdfchn 2553: if(!(*which < 1 || *which > 4)) goto S30; ... 7 more so this is a single-character typo rather than an intentional deviation. The effect is visible from the public API. cdfbin is declared in the installed nifticdf.h, and its contract is that an out-of-range input sets *status to -1 and *bound to the limit that was violated. Calling it with which = 9: before: status=999 bound=-999 (both left as the caller set them) after: status=-1 bound=4 (the documented error return) Before the fix the caller has no indication anything was wrong and the function proceeds to compute with an unhandled selector. --- nifticdf/nifticdf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nifticdf/nifticdf.c b/nifticdf/nifticdf.c index bdab933f..d9dba0b9 100644 --- a/nifticdf/nifticdf.c +++ b/nifticdf/nifticdf.c @@ -1892,7 +1892,7 @@ static double T5,T6,T7,T8,T9,T10,T12,T13; /* Check arguments */ - if(!(*which < 1 && *which > 4)) goto S30; + if(!(*which < 1 || *which > 4)) goto S30; if(!(*which < 1)) goto S10; *bound = 1.0e0; goto S20; From 663d349a74a3d5958d230a2407b87374b0c9e1a7 Mon Sep 17 00:00:00 2001 From: "Hans J. Johnson" Date: Tue, 22 Sep 2026 08:35:38 -0500 Subject: [PATCH 2/2] ENH: Cover cdfbin's out-of-range selector return The range check is reachable from the installed nifticdf.h, so assert the documented contract directly: which=9 and which=0 must set *status to -1 and *bound to the limit that was violated. A which=1 call is asserted to still return status 0, so a guard that rejected everything would not pass. --- nifticdf/CMakeLists.txt | 5 ++++ nifticdf/nifticdf_range_test.c | 50 ++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 nifticdf/nifticdf_range_test.c diff --git a/nifticdf/CMakeLists.txt b/nifticdf/CMakeLists.txt index b7d48480..8af8e395 100644 --- a/nifticdf/CMakeLists.txt +++ b/nifticdf/CMakeLists.txt @@ -33,6 +33,11 @@ endif() if(NIFTI_BUILD_TESTING AND NIFTI_BUILD_APPLICATIONS) + add_executable(${NIFTI_PACKAGE_PREFIX}nifticdf_range_test nifticdf_range_test.c) + target_link_libraries(${NIFTI_PACKAGE_PREFIX}nifticdf_range_test PRIVATE ${NIFTI_CDFLIB_NAME}) + add_test( NAME ${NIFTI_PACKAGE_PREFIX}nifticdf_range_test + COMMAND $ ) + foreach(DISTRIBUTION CORREL TTEST FTEST ZSCORE CHISQ BETA BINOM GAMMA POISSON NORMAL FTEST_NONC CHISQ_NONC LOGISTIC LAPLACE UNIFORM TTEST_NONC WEIBULL CHI INVGAUSS EXTVAL PVAL LOGPVAL LOG10PVAL ) add_test( NAME ${NIFTI_PACKAGE_PREFIX}nifti_stats_${DISTRIBUTION}_test COMMAND $ 0:4:1 ${DISTRIBUTION}) add_test( NAME q${NIFTI_PACKAGE_PREFIX}nifti_stats_${DISTRIBUTION}_test COMMAND $ -q 0:4:1 ${DISTRIBUTION}) diff --git a/nifticdf/nifticdf_range_test.c b/nifticdf/nifticdf_range_test.c new file mode 100644 index 00000000..63e607d6 --- /dev/null +++ b/nifticdf/nifticdf_range_test.c @@ -0,0 +1,50 @@ +/* cdfbin must reject an out-of-range "which" selector the way its ten + siblings in nifticdf.c do: *status = -1 and *bound = the limit. */ + +#include +#include "nifticdf.h" + +static int check(int which, double expected_bound) +{ + int status = 999; + double p = 0.5, q = 0.5, s = 2.0, xn = 5.0, pr = 0.5, ompr = 0.5; + double bound = -999.0; + + cdfbin(&which, &p, &q, &s, &xn, &pr, &ompr, &status, &bound); + + if( status != -1 || bound != expected_bound ) { + fprintf(stderr, "** cdfbin(which=%d): status=%d bound=%g," + " expected status=-1 bound=%g\n", + which, status, bound, expected_bound); + return 1; + } + printf("cdfbin(which=%d): status=%d bound=%g\n", which, status, bound); + return 0; +} + +int main(void) +{ + int errs = 0; + + errs += check(9, 4.0); /* above the legal range */ + errs += check(0, 1.0); /* below the legal range */ + + /* a legal selector must still compute, not report a range error */ + { + int which = 1, status = 999; + double p = 0.0, q = 0.0, s = 2.0, xn = 5.0, pr = 0.5, ompr = 0.5; + double bound = -999.0; + + cdfbin(&which, &p, &q, &s, &xn, &pr, &ompr, &status, &bound); + if( status != 0 ) { + fprintf(stderr, "** cdfbin(which=1): status=%d, expected 0\n", status); + errs++; + } else { + printf("cdfbin(which=1): status=0 p=%g\n", p); + } + } + + if( errs ) { fprintf(stderr, "** %d failure(s)\n", errs); return 1; } + printf("nifticdf range test passed\n"); + return 0; +}