From b51e2560c78c4fc457109ea50c67a5977bfe59b1 Mon Sep 17 00:00:00 2001 From: Owen Jessop Date: Thu, 22 Oct 2026 17:13:49 +0100 Subject: [PATCH 1/2] Fix buffer overflow in nbit and scaleoffset after another filter H5Z__filter_nbit and H5Z__filter_scaleoffset assume their input is exactly nelmts * datatype-size bytes, which is only true when they are the first filter in the pipeline. They only checked this with an assert(), which release builds remove. With e.g. deflate before nbit, the smaller buffer was over-read/over-written when writing a chunk. Return an error instead, so the pipeline skips these optional filters. Add a regression test and a changelog entry. --- release_docs/CHANGELOG.md | 10 ++++ src/H5Znbit.c | 7 ++- src/H5Zscaleoffset.c | 8 ++- test/dsets.c | 122 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 145 insertions(+), 2 deletions(-) diff --git a/release_docs/CHANGELOG.md b/release_docs/CHANGELOG.md index cc158dd4486..19bc91924d5 100644 --- a/release_docs/CHANGELOG.md +++ b/release_docs/CHANGELOG.md @@ -140,6 +140,16 @@ We would like to thank the many HDF5 community members who contributed to this r ## Library +### Fixed a heap buffer overflow in the N-bit and scale-offset filters + + The N-bit and scale-offset filters assume their input is exactly the size of a full + chunk, which is only true when they are the first filter in the pipeline. If another + filter such as deflate ran before them, the smaller buffer they received was + over-read or over-written when the chunk was written, because the only check was an + assert() that is compiled out of release builds. The filters now return an error + in that case, which makes the pipeline skip them (they are optional filters) and + store the chunk without them. + ### Fixed a deadlock in the ROS3 VFD on Windows When an HDF5 application running on Windows and using the ROS3 VFD exited normally, diff --git a/src/H5Znbit.c b/src/H5Znbit.c index 9dd2e5c6504..3b26ea8d38a 100644 --- a/src/H5Znbit.c +++ b/src/H5Znbit.c @@ -991,7 +991,12 @@ H5Z__filter_nbit(unsigned flags, size_t cd_nelmts, const unsigned cd_values[], s } /* end if */ /* output; compress */ else { - assert(nbytes == (size_t)d_nelmts * (size_t)cd_values[4]); + /* The compression loop below walks d_nelmts elements of size cd_values[4], so the + * input must be exactly that large. It is not if another filter (e.g. deflate) + * ran earlier in the pipeline and changed the buffer size. + */ + if (nbytes != (size_t)d_nelmts * (size_t)cd_values[4]) + HGOTO_ERROR(H5E_PLINE, H5E_CANTFILTER, 0, "nbit input size does not match the chunk's datatype"); size_out = nbytes; diff --git a/src/H5Zscaleoffset.c b/src/H5Zscaleoffset.c index d06e181aa1d..e8e2c88179d 100644 --- a/src/H5Zscaleoffset.c +++ b/src/H5Zscaleoffset.c @@ -1296,7 +1296,13 @@ H5Z__filter_scaleoffset(unsigned flags, size_t cd_nelmts, const unsigned cd_valu size_t used_bytes; size_t unused_bytes; - assert(nbytes == (size_t)d_nelmts * (size_t)p.size); + /* The pre-compression and packing code below reads d_nelmts elements of size p.size, + * so the input must be exactly that large. It is not if another filter (e.g. deflate) + * ran earlier in the pipeline and changed the buffer size. + */ + if (nbytes != (size_t)d_nelmts * (size_t)p.size) + HGOTO_ERROR(H5E_PLINE, H5E_CANTFILTER, 0, + "scaleoffset input size does not match the chunk's datatype"); /* before preprocess, convert to memory endianness order if needed */ if (need_convert) diff --git a/test/dsets.c b/test/dsets.c index 940c76fcb33..8be9c6c49da 100644 --- a/test/dsets.c +++ b/test/dsets.c @@ -148,6 +148,8 @@ static const char *FILENAME[] = {"dataset", /* 0 */ #define DSET_NBIT_COMPOUND_NAME_3 "nbit_compound_3" #define DSET_NBIT_INT_SIZE_NAME "nbit_int_size" #define DSET_NBIT_FLT_SIZE_NAME "nbit_flt_size" +#define DSET_NBIT_AFTER_DEFLATE_NAME "nbit_after_deflate" +#define DSET_SO_AFTER_DEFLATE_NAME "scaleoffset_after_deflate" #define DSET_SCALEOFFSET_INT_NAME "scaleoffset_int" #define DSET_SCALEOFFSET_INT_NAME_2 "scaleoffset_int_2" #define DSET_SCALEOFFSET_FLOAT_NAME "scaleoffset_float" @@ -5186,6 +5188,125 @@ test_nbit_flt_size(hid_t file) return FAIL; } /* end test_nbit_flt_size() */ +/*------------------------------------------------------------------------- + * Function: test_filter_after_deflate + * + * Purpose: Tests the nbit and scaleoffset filters when they come after + * the deflate filter in the pipeline. + * + * These filters expect a chunk of exactly nelmts * datatype + * size bytes, which is only true when they are the first + * filter. Deflate shrinks the chunk, so the input they get + * here is smaller than that. The data is chosen so that + * deflate does shrink it. The filters must reject such an + * input (and be skipped, as they are optional) instead of + * reading or writing past the end of their buffers. The + * data must round-trip either way. + * + * Return: Success: 0 + * Failure: -1 + *------------------------------------------------------------------------- + */ +#define AFTER_DEFLATE_NELMTS 64 +static herr_t +test_filter_after_deflate(hid_t file) +{ +#ifdef H5_HAVE_FILTER_DEFLATE + const char *names[2] = {DSET_NBIT_AFTER_DEFLATE_NAME, DSET_SO_AFTER_DEFLATE_NAME}; + hsize_t size[1] = {AFTER_DEFLATE_NELMTS}; + int orig_data[AFTER_DEFLATE_NELMTS]; + int new_data[AFTER_DEFLATE_NELMTS]; + size_t precision = 17, offset = 4; + size_t i; + int which; + hid_t dataset = H5I_INVALID_HID, datatype = H5I_INVALID_HID, space = H5I_INVALID_HID; + hid_t dc = H5I_INVALID_HID; + + TESTING("nbit and scaleoffset filters after deflate"); + + /* Few distinct values, so deflate shrinks the chunk well below its raw size */ + for (i = 0; i < AFTER_DEFLATE_NELMTS; i++) + orig_data[i] = (int)(i % 4) << offset; + + if ((space = H5Screate_simple(1, size, NULL)) < 0) + TEST_ERROR; + + for (which = 0; which < 2; which++) { + if ((datatype = H5Tcopy(H5T_NATIVE_INT)) < 0) + TEST_ERROR; + if (which == 0) { + if (H5Tset_precision(datatype, precision) < 0) + TEST_ERROR; + if (H5Tset_offset(datatype, offset) < 0) + TEST_ERROR; + } + + if ((dc = H5Pcreate(H5P_DATASET_CREATE)) < 0) + TEST_ERROR; + if (H5Pset_chunk(dc, 1, size) < 0) + TEST_ERROR; + if (H5Pset_deflate(dc, 6) < 0) + TEST_ERROR; + if (which == 0) { + if (H5Pset_nbit(dc) < 0) + TEST_ERROR; + } + else if (H5Pset_scaleoffset(dc, H5Z_SO_INT, H5Z_SO_INT_MINBITS_DEFAULT) < 0) + TEST_ERROR; + + if ((dataset = H5Dcreate2(file, names[which], datatype, space, H5P_DEFAULT, dc, H5P_DEFAULT)) < 0) + TEST_ERROR; + if (H5Dwrite(dataset, H5T_NATIVE_INT, H5S_ALL, H5S_ALL, H5P_DEFAULT, orig_data) < 0) + TEST_ERROR; + /* The chunk is filtered when it is evicted from the chunk cache */ + if (H5Dclose(dataset) < 0) + TEST_ERROR; + + if ((dataset = H5Dopen2(file, names[which], H5P_DEFAULT)) < 0) + TEST_ERROR; + memset(new_data, 0, sizeof(new_data)); + if (H5Dread(dataset, H5T_NATIVE_INT, H5S_ALL, H5S_ALL, H5P_DEFAULT, new_data) < 0) + TEST_ERROR; + for (i = 0; i < AFTER_DEFLATE_NELMTS; i++) + if (new_data[i] != orig_data[i]) { + H5_FAILED(); + printf(" %s: read different values than written at index %lu\n", names[which], + (unsigned long)i); + goto error; + } + + if (H5Dclose(dataset) < 0) + TEST_ERROR; + if (H5Pclose(dc) < 0) + TEST_ERROR; + if (H5Tclose(datatype) < 0) + TEST_ERROR; + } + + if (H5Sclose(space) < 0) + TEST_ERROR; + + PASSED(); + return SUCCEED; + +error: + H5E_BEGIN_TRY + { + H5Dclose(dataset); + H5Pclose(dc); + H5Tclose(datatype); + H5Sclose(space); + } + H5E_END_TRY + return FAIL; +#else + TESTING("nbit and scaleoffset filters after deflate"); + SKIPPED(); + puts(" Deflate filter not enabled"); + return SUCCEED; +#endif +} /* end test_filter_after_deflate() */ + /*------------------------------------------------------------------------- * Function: test_scaleoffset_int * @@ -19921,6 +20042,7 @@ main(void) nerrors += (test_nbit_compound_3(file) < 0 ? 1 : 0); nerrors += (test_nbit_int_size(file) < 0 ? 1 : 0); nerrors += (test_nbit_flt_size(file) < 0 ? 1 : 0); + nerrors += (test_filter_after_deflate(file) < 0 ? 1 : 0); nerrors += (test_scaleoffset_int(file) < 0 ? 1 : 0); nerrors += (test_scaleoffset_int_2(file) < 0 ? 1 : 0); nerrors += (test_scaleoffset_float(file) < 0 ? 1 : 0); From 838e54b5dace5b94b5574247e2a5e64aa09b2ec3 Mon Sep 17 00:00:00 2001 From: Owen Jessop Date: Thu, 22 Oct 2026 17:53:07 +0100 Subject: [PATCH 2/2] Check in the nbit/scaleoffset test that deflate actually ran Assert that deflate was applied and the second filter was skipped, so the test cannot pass without exercising the bug. Skip it when deflate is unregistered, which test_missing_filter() does for the rest of the run. --- test/dsets.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/dsets.c b/test/dsets.c index 8be9c6c49da..a3127c97b1b 100644 --- a/test/dsets.c +++ b/test/dsets.c @@ -5219,11 +5219,19 @@ test_filter_after_deflate(hid_t file) size_t precision = 17, offset = 4; size_t i; int which; + unsigned filter_mask = 0; hid_t dataset = H5I_INVALID_HID, datatype = H5I_INVALID_HID, space = H5I_INVALID_HID; hid_t dc = H5I_INVALID_HID; TESTING("nbit and scaleoffset filters after deflate"); + /* test_missing_filter() unregisters deflate for the rest of the run */ + if (H5Zfilter_avail(H5Z_FILTER_DEFLATE) != true) { + SKIPPED(); + puts(" Deflate filter not available"); + return SUCCEED; + } + /* Few distinct values, so deflate shrinks the chunk well below its raw size */ for (i = 0; i < AFTER_DEFLATE_NELMTS; i++) orig_data[i] = (int)(i % 4) << offset; @@ -5264,6 +5272,17 @@ test_filter_after_deflate(hid_t file) if ((dataset = H5Dopen2(file, names[which], H5P_DEFAULT)) < 0) TEST_ERROR; + + /* Deflate (filter 0) must have run, otherwise the case above was not + * exercised; the second filter must have been skipped */ + if (H5Dget_chunk_info(dataset, H5S_ALL, 0, NULL, &filter_mask, NULL, NULL) < 0) + TEST_ERROR; + if (filter_mask != 0x2) { + H5_FAILED(); + printf(" %s: unexpected chunk filter mask 0x%x\n", names[which], filter_mask); + goto error; + } + memset(new_data, 0, sizeof(new_data)); if (H5Dread(dataset, H5T_NATIVE_INT, H5S_ALL, H5S_ALL, H5P_DEFAULT, new_data) < 0) TEST_ERROR;