From b211304fb8cd21a6911686b03562b5d55f7027a2 Mon Sep 17 00:00:00 2001 From: Stuart Geipel Date: Thu, 30 Jul 2026 14:34:21 -0400 Subject: [PATCH 1/6] Add error checking to zstd hdf5 plugin This PR adds error checking to the zstd hdf5 plugin. Previously it would return uninitialized memory from malloc when any zstd error occurred. It also prevents proceeding with the result of malloc(0) in the abnormal case where decompSize == 0. --- ZSTD/src/H5Zzstd.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ZSTD/src/H5Zzstd.c b/ZSTD/src/H5Zzstd.c index b96fa84c6..4f5270425 100644 --- a/ZSTD/src/H5Zzstd.c +++ b/ZSTD/src/H5Zzstd.c @@ -66,10 +66,16 @@ H5Z_filter_zstd(unsigned int flags, size_t cd_nelmts, const unsigned int cd_valu if (flags & H5Z_FLAG_REVERSE) { /* We're decompressing */ size_t decompSize = ZSTD_getFrameContentSize(*buf, origSize); + if (decompSize == 0 || decompSize == ZSTD_CONTENTSIZE_UNKNOWN || + decompSize == ZSTD_CONTENTSIZE_ERROR) + goto error; + if (NULL == (outbuf = malloc(decompSize))) goto error; decompSize = ZSTD_decompress(outbuf, decompSize, inbuf, origSize); + if (ZSTD_isError(decompSize)) + goto error; #ifdef ZSTD_DEBUG fprintf(stderr, " decompressing nbytes: %ld\n", decompSize); @@ -104,6 +110,8 @@ H5Z_filter_zstd(unsigned int flags, size_t cd_nelmts, const unsigned int cd_valu goto error; compSize = ZSTD_compress(outbuf, compSize, inbuf, origSize, aggression); + if (ZSTD_isError(compSize)) + goto error; #ifdef ZSTD_DEBUG fprintf(stderr, " compressing nbytes: %ld\n", compSize); From aad7cdda1e95da8d6d61fd7c5f1f6893ac509628 Mon Sep 17 00:00:00 2001 From: Stuart Geipel Date: Mon, 17 Aug 2026 14:04:37 -0400 Subject: [PATCH 2/6] Add error reporting with H5Epush This also changes the return type of ZSTD_getFrameContentSize to unsigned long long. --- ZSTD/src/H5Zzstd.c | 41 +++++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/ZSTD/src/H5Zzstd.c b/ZSTD/src/H5Zzstd.c index 4f5270425..02f35f88d 100644 --- a/ZSTD/src/H5Zzstd.c +++ b/ZSTD/src/H5Zzstd.c @@ -22,8 +22,8 @@ static size_t H5Z_filter_zstd(unsigned int flags, size_t cd_nelmts, const unsign #define H5Z_FILTER_ZSTD 32015 -#define PUSH_ERR(func, minor, str) \ - H5Epush(H5E_DEFAULT, __FILE__, func, __LINE__, H5E_ERR_CLS, H5E_PLINE, minor, str) +#define PUSH_ERR(func, minor, ...) \ + H5Epush(H5E_DEFAULT, __FILE__, func, __LINE__, H5E_ERR_CLS, H5E_PLINE, minor, __VA_ARGS__) const H5Z_class2_t H5Z_ZSTD[1] = {{ H5Z_CLASS_T_VERS, /* H5Z_class_t version */ @@ -65,17 +65,33 @@ H5Z_filter_zstd(unsigned int flags, size_t cd_nelmts, const unsigned int cd_valu if (flags & H5Z_FLAG_REVERSE) { /* We're decompressing */ - size_t decompSize = ZSTD_getFrameContentSize(*buf, origSize); - if (decompSize == 0 || decompSize == ZSTD_CONTENTSIZE_UNKNOWN || - decompSize == ZSTD_CONTENTSIZE_ERROR) + unsigned long long contentSize = ZSTD_getFrameContentSize(*buf, origSize); + if (contentSize == ZSTD_CONTENTSIZE_ERROR) { + PUSH_ERR("H5Z_filter_zstd", H5E_CALLBACK, "Input is not a valid zstd frame"); goto error; + } + if (contentSize == ZSTD_CONTENTSIZE_UNKNOWN) { + PUSH_ERR("H5Z_filter_zstd", H5E_CALLBACK, + "zstd frame does not record its decompressed size; the data was likely compressed " + "with the zstd streaming API, which this filter does not support"); + goto error; + } + if (contentSize == 0) { + PUSH_ERR("H5Z_filter_zstd", H5E_CALLBACK, "zstd frame has zero decompressed size"); + goto error; + } - if (NULL == (outbuf = malloc(decompSize))) + if (NULL == (outbuf = malloc((size_t)contentSize))) { + PUSH_ERR("H5Z_filter_zstd", H5E_CALLBACK, "Can't allocate zstd decompression buffer"); goto error; + } - decompSize = ZSTD_decompress(outbuf, decompSize, inbuf, origSize); - if (ZSTD_isError(decompSize)) + size_t decompSize = ZSTD_decompress(outbuf, (size_t)contentSize, inbuf, origSize); + if (ZSTD_isError(decompSize)) { + PUSH_ERR("H5Z_filter_zstd", H5E_CALLBACK, "zstd decompression failed: %s", + ZSTD_getErrorName(decompSize)); goto error; + } #ifdef ZSTD_DEBUG fprintf(stderr, " decompressing nbytes: %ld\n", decompSize); @@ -106,12 +122,17 @@ H5Z_filter_zstd(unsigned int flags, size_t cd_nelmts, const unsigned int cd_valu aggression = ZSTD_maxCLevel(); size_t compSize = ZSTD_compressBound(origSize); - if (NULL == (outbuf = malloc(compSize))) + if (NULL == (outbuf = malloc(compSize))) { + PUSH_ERR("H5Z_filter_zstd", H5E_CALLBACK, "Can't allocate zstd compression buffer"); goto error; + } compSize = ZSTD_compress(outbuf, compSize, inbuf, origSize, aggression); - if (ZSTD_isError(compSize)) + if (ZSTD_isError(compSize)) { + PUSH_ERR("H5Z_filter_zstd", H5E_CALLBACK, "zstd compression failed: %s", + ZSTD_getErrorName(compSize)); goto error; + } #ifdef ZSTD_DEBUG fprintf(stderr, " compressing nbytes: %ld\n", compSize); From 1e7c055dbef50db2de89fa8ae31c72ce5f0775bc Mon Sep 17 00:00:00 2001 From: Stuart Geipel Date: Mon, 17 Aug 2026 14:14:41 -0400 Subject: [PATCH 3/6] Remove variadic macros The rest of the repo shies away from it --- ZSTD/src/H5Zzstd.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/ZSTD/src/H5Zzstd.c b/ZSTD/src/H5Zzstd.c index 02f35f88d..401a22e25 100644 --- a/ZSTD/src/H5Zzstd.c +++ b/ZSTD/src/H5Zzstd.c @@ -22,8 +22,10 @@ static size_t H5Z_filter_zstd(unsigned int flags, size_t cd_nelmts, const unsign #define H5Z_FILTER_ZSTD 32015 -#define PUSH_ERR(func, minor, ...) \ - H5Epush(H5E_DEFAULT, __FILE__, func, __LINE__, H5E_ERR_CLS, H5E_PLINE, minor, __VA_ARGS__) +#define PUSH_ERR(func, minor, str) \ + H5Epush(H5E_DEFAULT, __FILE__, func, __LINE__, H5E_ERR_CLS, H5E_PLINE, minor, str) +#define PUSH_ERR2(func, minor, str, arg) \ + H5Epush(H5E_DEFAULT, __FILE__, func, __LINE__, H5E_ERR_CLS, H5E_PLINE, minor, str, arg) const H5Z_class2_t H5Z_ZSTD[1] = {{ H5Z_CLASS_T_VERS, /* H5Z_class_t version */ @@ -88,8 +90,8 @@ H5Z_filter_zstd(unsigned int flags, size_t cd_nelmts, const unsigned int cd_valu size_t decompSize = ZSTD_decompress(outbuf, (size_t)contentSize, inbuf, origSize); if (ZSTD_isError(decompSize)) { - PUSH_ERR("H5Z_filter_zstd", H5E_CALLBACK, "zstd decompression failed: %s", - ZSTD_getErrorName(decompSize)); + PUSH_ERR2("H5Z_filter_zstd", H5E_CALLBACK, "zstd decompression failed: %s", + ZSTD_getErrorName(decompSize)); goto error; } @@ -129,8 +131,8 @@ H5Z_filter_zstd(unsigned int flags, size_t cd_nelmts, const unsigned int cd_valu compSize = ZSTD_compress(outbuf, compSize, inbuf, origSize, aggression); if (ZSTD_isError(compSize)) { - PUSH_ERR("H5Z_filter_zstd", H5E_CALLBACK, "zstd compression failed: %s", - ZSTD_getErrorName(compSize)); + PUSH_ERR2("H5Z_filter_zstd", H5E_CALLBACK, "zstd compression failed: %s", + ZSTD_getErrorName(compSize)); goto error; } From ed4e885a7d8a48afca5110a1361a69c9e2f335bb Mon Sep 17 00:00:00 2001 From: Stuart Geipel Date: Mon, 17 Aug 2026 14:21:37 -0400 Subject: [PATCH 4/6] better message for streaming --- ZSTD/src/H5Zzstd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ZSTD/src/H5Zzstd.c b/ZSTD/src/H5Zzstd.c index 401a22e25..52846435d 100644 --- a/ZSTD/src/H5Zzstd.c +++ b/ZSTD/src/H5Zzstd.c @@ -74,8 +74,8 @@ H5Z_filter_zstd(unsigned int flags, size_t cd_nelmts, const unsigned int cd_valu } if (contentSize == ZSTD_CONTENTSIZE_UNKNOWN) { PUSH_ERR("H5Z_filter_zstd", H5E_CALLBACK, - "zstd frame does not record its decompressed size; the data was likely compressed " - "with the zstd streaming API, which this filter does not support"); + "zstd frame missing decompressed size; data was likely compressed " + "with the zstd streaming API, which is not supported"); goto error; } if (contentSize == 0) { From d9b5f57f64b0643c5c2fac8ffe743efbd9a2adc5 Mon Sep 17 00:00:00 2001 From: Stuart Geipel Date: Wed, 26 Aug 2026 14:03:18 -0400 Subject: [PATCH 5/6] copilot feedback --- ZSTD/src/H5Zzstd.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/ZSTD/src/H5Zzstd.c b/ZSTD/src/H5Zzstd.c index 52846435d..4c4e11d69 100644 --- a/ZSTD/src/H5Zzstd.c +++ b/ZSTD/src/H5Zzstd.c @@ -8,6 +8,7 @@ */ #include +#include #include #include #include @@ -82,13 +83,19 @@ H5Z_filter_zstd(unsigned int flags, size_t cd_nelmts, const unsigned int cd_valu PUSH_ERR("H5Z_filter_zstd", H5E_CALLBACK, "zstd frame has zero decompressed size"); goto error; } + if (contentSize > SIZE_MAX) { + PUSH_ERR("H5Z_filter_zstd", H5E_CALLBACK, + "zstd frame decompressed size exceeds addressable memory"); + goto error; + } + size_t decompCapacity = (size_t)contentSize; - if (NULL == (outbuf = malloc((size_t)contentSize))) { + if (NULL == (outbuf = malloc(decompCapacity))) { PUSH_ERR("H5Z_filter_zstd", H5E_CALLBACK, "Can't allocate zstd decompression buffer"); goto error; } - size_t decompSize = ZSTD_decompress(outbuf, (size_t)contentSize, inbuf, origSize); + size_t decompSize = ZSTD_decompress(outbuf, decompCapacity, inbuf, origSize); if (ZSTD_isError(decompSize)) { PUSH_ERR2("H5Z_filter_zstd", H5E_CALLBACK, "zstd decompression failed: %s", ZSTD_getErrorName(decompSize)); @@ -96,7 +103,7 @@ H5Z_filter_zstd(unsigned int flags, size_t cd_nelmts, const unsigned int cd_valu } #ifdef ZSTD_DEBUG - fprintf(stderr, " decompressing nbytes: %ld\n", decompSize); + fprintf(stderr, " decompressing nbytes: %zu\n", decompSize); #endif buf_size_out = decompSize; @@ -137,7 +144,7 @@ H5Z_filter_zstd(unsigned int flags, size_t cd_nelmts, const unsigned int cd_valu } #ifdef ZSTD_DEBUG - fprintf(stderr, " compressing nbytes: %ld\n", compSize); + fprintf(stderr, " compressing nbytes: %zu\n", compSize); #endif buf_size_out = compSize; From 33c3991fa1e38e0ab8d6ff1af380a13c1007c763 Mon Sep 17 00:00:00 2001 From: Stuart Geipel Date: Mon, 31 Aug 2026 20:47:13 -0400 Subject: [PATCH 6/6] add corrupt zstd tests --- ZSTD/config/cmake/runTest.cmake | 14 ++++---- ZSTD/example/CMakeLists.txt | 32 ++++++++++++++++++ ZSTD/example/testfiles/h5ex_d_zstd_corrupt.h5 | Bin 0 -> 3540 bytes .../testfiles/h5ex_d_zstd_streaming.h5 | Bin 0 -> 3761 bytes 4 files changed, 40 insertions(+), 6 deletions(-) create mode 100644 ZSTD/example/testfiles/h5ex_d_zstd_corrupt.h5 create mode 100644 ZSTD/example/testfiles/h5ex_d_zstd_streaming.h5 diff --git a/ZSTD/config/cmake/runTest.cmake b/ZSTD/config/cmake/runTest.cmake index edf46a867..5409772ec 100644 --- a/ZSTD/config/cmake/runTest.cmake +++ b/ZSTD/config/cmake/runTest.cmake @@ -362,12 +362,14 @@ if (TEST_GREP_COMPARE) message (FATAL_ERROR "Failed: The output of ${TEST_PROGRAM} did not contain ${TEST_REFERENCE}") endif () - string (REGEX MATCH "${TEST_FILTER}" TEST_MATCH ${TEST_STREAM}) - if (TEST_EXPECT) - # TEST_EXPECT (1) interprets TEST_FILTER as; NOT to match - string (LENGTH "${TEST_MATCH}" TEST_GREP_RESULT) - if (TEST_GREP_RESULT) - message (FATAL_ERROR "Failed: The output of ${TEST_PROGRAM} did contain ${TEST_FILTER}") + if (TEST_FILTER) + string (REGEX MATCH "${TEST_FILTER}" TEST_MATCH ${TEST_STREAM}) + if (TEST_EXPECT) + # TEST_EXPECT (1) interprets TEST_FILTER as; NOT to match + string (LENGTH "${TEST_MATCH}" TEST_GREP_RESULT) + if (TEST_GREP_RESULT) + message (FATAL_ERROR "Failed: The output of ${TEST_PROGRAM} did contain ${TEST_FILTER}") + endif () endif () endif () endif () diff --git a/ZSTD/example/CMakeLists.txt b/ZSTD/example/CMakeLists.txt index 44e946325..378040234 100644 --- a/ZSTD/example/CMakeLists.txt +++ b/ZSTD/example/CMakeLists.txt @@ -120,6 +120,33 @@ if (H5PL_BUILD_TESTING) endif () endmacro () + # Test that reading a .h5 with corrupt zstd data properly throws an error + # via the zstd filter + macro (ADD_H5_DUMPFAIL_TEST testname errmsg) + if (HDF5_BUILD_TOOLS OR HDF5_PROVIDES_TOOLS) + add_test ( + NAME H5DUMP-${testname} + COMMAND "${CMAKE_COMMAND}" + -D "TEST_PROGRAM=${H5PL_HDF5_DUMP_EXECUTABLE}" + -D "TEST_ARGS:STRING=--enable-error-stack;-d;/DS1;${testname}.h5" + -D "TEST_FOLDER=${PROJECT_BINARY_DIR}" + -D "TEST_EXPECT=1" + -D "ERROR_APPEND=1" + -D "TEST_GREP_COMPARE=1" + -D "TEST_OUTPUT=${testname}.out" + -D "TEST_REFERENCE=${errmsg}" + -D "TEST_LIBRARY_DIRECTORY=${TESTLIBDIR}" + -D "TEST_ENV_VAR=HDF5_PLUGIN_PATH" + -D "TEST_ENV_VALUE=${CMAKE_BINARY_DIR}/plugins" + -P "${H5ZSTD_RESOURCES_DIR}/runTest.cmake" + ) + set_tests_properties (H5DUMP-${testname} PROPERTIES + WORKING_DIRECTORY "${PROJECT_BINARY_DIR}" + DEPENDS ${last_test}) + set (last_test "H5DUMP-${testname}") + endif () + endmacro () + macro (ADD_H5_UD_TEST testname resultcode resultfile) # Remove any output file left over from previous test run add_test ( @@ -183,6 +210,8 @@ if (H5PL_BUILD_TESTING) # Copy all the HDF5 files from the source directory into the test directory # -------------------------------------------------------------------- set (LIST_HDF5_TEST_FILES + h5ex_d_zstd_corrupt.h5 + h5ex_d_zstd_streaming.h5 # h5repack_layout.h5 ) set (LIST_OTHER_TEST_FILES @@ -199,6 +228,9 @@ if (H5PL_BUILD_TESTING) ADD_H5_TEST (h5ex_d_zstd) + ADD_H5_DUMPFAIL_TEST (h5ex_d_zstd_corrupt "Input is not a valid zstd frame") + ADD_H5_DUMPFAIL_TEST (h5ex_d_zstd_streaming "zstd frame missing decompressed size") + # if (NOT DISABLE_H5ZSTD_ENCODER) # #UD ZSTD # ADD_H5_UD_TEST (ud_convert 0 h5repack_layout.h5 --enable-error-stack -v -f UD=32015,0,0) diff --git a/ZSTD/example/testfiles/h5ex_d_zstd_corrupt.h5 b/ZSTD/example/testfiles/h5ex_d_zstd_corrupt.h5 new file mode 100644 index 0000000000000000000000000000000000000000..da9024b79dc070855d29cc3d352b2fdf073c5605 GIT binary patch literal 3540 zcmeD5aB<`1lHy_j0S*oZ76t(@6Gr@pf-Afb5f~pPp8#brLg@}Dy@CnCU}OM61_lYJ zxFFPgbaf#?uC5F~l`!*RG*lad0Sko10TURdM^p%SxH<-aJRAY_H7u2$fTlB8V7LSu zf|;=N=mC=;lV)UK1E*gI0ZO5el+4HowE<=klx7B|Zx9pYK8O$`#j-MRfcZ?!OkiPd zs5+Q2{IwAE98mL_nV2eAp>76hfEWelS3pGvh6?uccLC*SSUyx>9vF8Fbz4E@9Vi@O z76hfEWelS3pGvh6?uccLC*SSUyx>9vF8Fbz4E@9Vi@O zMC+6cQE@6%&_`l#-T_m6KOcR8m$^Ra4i{)Y8_`)zddHG%_|Z zH8Z!cw6eCbwX=6{baHlab#wRd^z!!c_45x13RUzF>}`J zIdkXDU$Ah|;w4L$Enl&6)#^2C*R9{Mant54TeofBv2)k%J$v`