diff --git a/release_docs/CHANGELOG.md b/release_docs/CHANGELOG.md index 49d4d6c4501f..1280c557a89a 100644 --- a/release_docs/CHANGELOG.md +++ b/release_docs/CHANGELOG.md @@ -144,6 +144,10 @@ The `h5repack` tool now obtains its default low and high library version bounds ## Library +### Fixed crashes when reading datasets with malformed N-Bit or Fletcher32 filter metadata + + Reading a dataset from a corrupted or maliciously crafted file could crash the library in the N-Bit and Fletcher32 filter decode paths. The N-Bit filter dereferenced its client-data parameter array before validating it, crashing when the array was empty or NULL, and walked the compressed chunk during decompression without bounding the input against the chunk size, causing out-of-bounds reads. The Fletcher32 filter subtracted the 4-byte checksum length from the chunk size without checking that the chunk was at least that large, underflowing the length passed to the checksum routine. These filters now validate their parameters and buffer sizes and fail with an error instead of crashing. This fixes GitHub issues #6489 and #6490. + ### HTTP 403 errors in the ROS3 VFD for object keys with special characters The ROS3 VFD did not URI-encode the S3 object key when building the HTTP request path, so keys containing characters that AWS Signature Version 4 requires to be percent-encoded — such as the '=' in Hive-style `key=value` partition prefixes, '+', or spaces — produced a signed request whose signature did not match S3's server-side recomputation. S3 rejects such requests with `SignatureDoesNotMatch`, which surfaces as an HTTP 403 error (indistinguishable from a permissions error on a HEAD request), even though tools like the AWS CLI could access the same object. The object key is now percent-encoded exactly once when the request path is built, matching the behavior of other S3 clients. Note that URLs must now be passed to the ROS3 VFD with their object keys unencoded; a key that was pre-encoded as a workaround for this issue will now be double-encoded and fail to resolve. diff --git a/src/H5Zfletcher32.c b/src/H5Zfletcher32.c index c3ec5f7ccb03..b89724ad0a33 100644 --- a/src/H5Zfletcher32.c +++ b/src/H5Zfletcher32.c @@ -62,6 +62,14 @@ H5Z__filter_fletcher32(unsigned flags, size_t H5_ATTR_UNUSED cd_nelmts, assert(sizeof(uint32_t) >= 4); if (flags & H5Z_FLAG_REVERSE) { /* Read */ + /* A Fletcher32-filtered buffer must contain at least the trailing + * checksum. Reject anything smaller to avoid a size_t underflow + * when computing the data length. + */ + if (nbytes < FLETCHER_LEN) + HGOTO_ERROR(H5E_STORAGE, H5E_OVERFLOW, 0, + "fletcher32 filter input buffer too small to contain checksum"); + /* Do checksum if it's enabled for read; otherwise skip it * to save performance. */ if (!(flags & H5Z_FLAG_SKIP_EDC)) { diff --git a/src/H5Znbit.c b/src/H5Znbit.c index fc113e56f0d3..f8d5fc04b853 100644 --- a/src/H5Znbit.c +++ b/src/H5Znbit.c @@ -51,26 +51,28 @@ static herr_t H5Z__set_parms_compound(const H5T_t *type, unsigned *cd_values_ind bool *need_not_compress); static void H5Z__nbit_next_byte(size_t *j, size_t *buf_len); -static void H5Z__nbit_decompress_one_byte(unsigned char *data, size_t data_offset, unsigned k, +static herr_t H5Z__nbit_decompress_one_byte(unsigned char *data, size_t data_offset, unsigned k, unsigned begin_i, unsigned end_i, const unsigned char *buffer, - size_t *j, size_t *buf_len, const parms_atomic *p, - size_t datatype_len); + size_t buffer_size, size_t *j, size_t *buf_len, + const parms_atomic *p, size_t datatype_len); static void H5Z__nbit_compress_one_byte(const unsigned char *data, size_t data_offset, unsigned k, unsigned begin_i, unsigned end_i, unsigned char *buffer, size_t *j, size_t *buf_len, const parms_atomic *p, size_t datatype_len); -static void H5Z__nbit_decompress_one_nooptype(unsigned char *data, size_t data_offset, - const unsigned char *buffer, size_t *j, size_t *buf_len, - unsigned size); -static void H5Z__nbit_decompress_one_atomic(unsigned char *data, size_t data_offset, unsigned char *buffer, - size_t *j, size_t *buf_len, const parms_atomic *p); +static herr_t H5Z__nbit_decompress_one_nooptype(unsigned char *data, size_t data_offset, + const unsigned char *buffer, size_t buffer_size, size_t *j, + size_t *buf_len, unsigned size); +static herr_t H5Z__nbit_decompress_one_atomic(unsigned char *data, size_t data_offset, unsigned char *buffer, + size_t buffer_size, size_t *j, size_t *buf_len, + const parms_atomic *p); static herr_t H5Z__nbit_decompress_one_array(unsigned char *data, size_t data_offset, unsigned char *buffer, - size_t *j, size_t *buf_len, const unsigned parms[], - unsigned *parms_index); + size_t buffer_size, size_t *j, size_t *buf_len, + const unsigned parms[], unsigned *parms_index); static herr_t H5Z__nbit_decompress_one_compound(unsigned char *data, size_t data_offset, - unsigned char *buffer, size_t *j, size_t *buf_len, - const unsigned parms[], unsigned *parms_index); + unsigned char *buffer, size_t buffer_size, size_t *j, + size_t *buf_len, const unsigned parms[], + unsigned *parms_index); static herr_t H5Z__nbit_decompress(unsigned char *data, unsigned d_nelmts, unsigned char *buffer, - const unsigned parms[]); + size_t buffer_size, const unsigned parms[]); static void H5Z__nbit_compress_one_nooptype(const unsigned char *data, size_t data_offset, unsigned char *buffer, size_t *j, size_t *buf_len, unsigned size); static void H5Z__nbit_compress_one_array(unsigned char *data, size_t data_offset, unsigned char *buffer, @@ -931,7 +933,16 @@ H5Z__filter_nbit(unsigned flags, size_t cd_nelmts, const unsigned cd_values[], s /* check arguments * cd_values[0] stores actual number of parameters in cd_values[] + * + * The filter header always occupies cd_values[0..4] (number of + * parameters, no-compression flag, number of elements, datatype class + * and datatype size), all of which are read unconditionally below. + * Reject a NULL array or one shorter than that to avoid a bad + * dereference. */ + if (cd_values == NULL || cd_nelmts < 5) + HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, 0, "invalid nbit parameters"); + if (cd_nelmts != cd_values[0]) HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, 0, "invalid nbit aggression level"); @@ -944,6 +955,18 @@ H5Z__filter_nbit(unsigned flags, size_t cd_nelmts, const unsigned cd_values[], s /* copy a filter parameter to d_nelmts */ d_nelmts = cd_values[2]; + /* cd_values[4] stores the datatype size, which is used together with + * d_nelmts to size the (de)compression buffer. A zero size is invalid, + * and a d_nelmts * size product that overflows size_t would produce an + * undersized allocation and out-of-bounds accesses (this matters on + * platforms with a 32-bit size_t). Reject both, as they can only arise + * from a corrupted or crafted file. + */ + if (cd_values[4] == 0) + HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, 0, "invalid nbit datatype size"); + if (d_nelmts != 0 && (size_t)cd_values[4] > SIZE_MAX / d_nelmts) + HGOTO_ERROR(H5E_ARGS, H5E_OVERFLOW, 0, "nbit (de)compression buffer size overflow"); + /* input; decompress */ if (flags & H5Z_FLAG_REVERSE) { size_out = d_nelmts * (size_t)cd_values[4]; /* cd_values[4] stores datatype size */ @@ -953,7 +976,7 @@ H5Z__filter_nbit(unsigned flags, size_t cd_nelmts, const unsigned cd_values[], s HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, 0, "memory allocation failed for nbit decompression"); /* decompress the buffer */ - if (H5Z__nbit_decompress(outbuf, d_nelmts, (unsigned char *)*buf, cd_values) < 0) { + if (H5Z__nbit_decompress(outbuf, d_nelmts, (unsigned char *)*buf, nbytes, cd_values) < 0) { H5MM_xfree(outbuf); HGOTO_ERROR(H5E_PLINE, H5E_CANTFILTER, 0, "can't decompress buffer"); } @@ -999,15 +1022,19 @@ H5Z__nbit_next_byte(size_t *j, size_t *buf_len) *buf_len = 8 * sizeof(unsigned char); } -static void +static herr_t H5Z__nbit_decompress_one_byte(unsigned char *data, size_t data_offset, unsigned k, unsigned begin_i, - unsigned end_i, const unsigned char *buffer, size_t *j, size_t *buf_len, - const parms_atomic *p, size_t datatype_len) + unsigned end_i, const unsigned char *buffer, size_t buffer_size, size_t *j, + size_t *buf_len, const parms_atomic *p, size_t datatype_len) { size_t dat_len; /* dat_len is the number of bits to be copied in each data byte */ size_t dat_offset; unsigned char val; /* value to be copied in each data byte */ + /* Ensure the byte to be read is within the bounds of the compressed buffer */ + if (*j >= buffer_size) + return FAIL; + /* initialize value and bits of unsigned char to be copied */ val = buffer[*j]; dat_offset = 0; @@ -1039,25 +1066,33 @@ H5Z__nbit_decompress_one_byte(unsigned char *data, size_t data_offset, unsigned dat_len -= *buf_len; H5Z__nbit_next_byte(j, buf_len); if (dat_len == 0) - return; + return SUCCEED; + if (*j >= buffer_size) + return FAIL; val = buffer[*j]; data[data_offset + k] |= (unsigned char)(((unsigned)(val >> (*buf_len - dat_len)) & (unsigned)(~((unsigned)(~0) << dat_len))) << dat_offset); *buf_len -= dat_len; } + + return SUCCEED; } -static void +static herr_t H5Z__nbit_decompress_one_nooptype(unsigned char *data, size_t data_offset, const unsigned char *buffer, - size_t *j, size_t *buf_len, unsigned size) + size_t buffer_size, size_t *j, size_t *buf_len, unsigned size) { unsigned i; /* index */ size_t dat_len; /* dat_len is the number of bits to be copied in each data byte */ unsigned char val; /* value to be copied in each data byte */ for (i = 0; i < size; i++) { + /* Ensure the byte to be read is within the bounds of the compressed buffer */ + if (*j >= buffer_size) + return FAIL; + /* initialize value and bits of unsigned char to be copied */ val = buffer[*j]; dat_len = sizeof(unsigned char) * 8; @@ -1069,16 +1104,20 @@ H5Z__nbit_decompress_one_nooptype(unsigned char *data, size_t data_offset, const if (dat_len == 0) continue; + if (*j >= buffer_size) + return FAIL; val = buffer[*j]; data[data_offset + i] |= (unsigned char)((unsigned)(val >> (*buf_len - dat_len)) & (unsigned)(~((unsigned)(~0) << dat_len))); *buf_len -= dat_len; } + + return SUCCEED; } -static void -H5Z__nbit_decompress_one_atomic(unsigned char *data, size_t data_offset, unsigned char *buffer, size_t *j, - size_t *buf_len, const parms_atomic *p) +static herr_t +H5Z__nbit_decompress_one_atomic(unsigned char *data, size_t data_offset, unsigned char *buffer, + size_t buffer_size, size_t *j, size_t *buf_len, const parms_atomic *p) { /* begin_i: the index of byte having first significant bit end_i: the index of byte having last significant bit */ @@ -1097,8 +1136,9 @@ H5Z__nbit_decompress_one_atomic(unsigned char *data, size_t data_offset, unsigne end_i = p->offset / 8; for (k = (int)begin_i; k >= (int)end_i; k--) - H5Z__nbit_decompress_one_byte(data, data_offset, (unsigned)k, begin_i, end_i, buffer, j, buf_len, - p, datatype_len); + if (H5Z__nbit_decompress_one_byte(data, data_offset, (unsigned)k, begin_i, end_i, buffer, + buffer_size, j, buf_len, p, datatype_len) < 0) + return FAIL; } else { /* big endian */ /* Sanity check */ @@ -1112,14 +1152,18 @@ H5Z__nbit_decompress_one_atomic(unsigned char *data, size_t data_offset, unsigne end_i = ((unsigned)datatype_len - p->offset) / 8 - 1; for (k = (int)begin_i; k <= (int)end_i; k++) - H5Z__nbit_decompress_one_byte(data, data_offset, (unsigned)k, begin_i, end_i, buffer, j, buf_len, - p, datatype_len); + if (H5Z__nbit_decompress_one_byte(data, data_offset, (unsigned)k, begin_i, end_i, buffer, + buffer_size, j, buf_len, p, datatype_len) < 0) + return FAIL; } + + return SUCCEED; } static herr_t -H5Z__nbit_decompress_one_array(unsigned char *data, size_t data_offset, unsigned char *buffer, size_t *j, - size_t *buf_len, const unsigned parms[], unsigned *parms_index) +H5Z__nbit_decompress_one_array(unsigned char *data, size_t data_offset, unsigned char *buffer, + size_t buffer_size, size_t *j, size_t *buf_len, const unsigned parms[], + unsigned *parms_index) { unsigned i, total_size, base_class, base_size, n, begin_index; parms_atomic p; @@ -1143,8 +1187,9 @@ H5Z__nbit_decompress_one_array(unsigned char *data, size_t data_offset, unsigned n = total_size / p.size; for (i = 0; i < n; i++) - H5Z__nbit_decompress_one_atomic(data, data_offset + i * (size_t)p.size, buffer, j, buf_len, - &p); + if (H5Z__nbit_decompress_one_atomic(data, data_offset + i * (size_t)p.size, buffer, + buffer_size, j, buf_len, &p) < 0) + HGOTO_ERROR(H5E_PLINE, H5E_CANTFILTER, FAIL, "can't decompress atomic"); break; case H5Z_NBIT_ARRAY: @@ -1152,8 +1197,8 @@ H5Z__nbit_decompress_one_array(unsigned char *data, size_t data_offset, unsigned n = total_size / base_size; /* number of base_type elements inside the array datatype */ begin_index = *parms_index; for (i = 0; i < n; i++) { - if (H5Z__nbit_decompress_one_array(data, data_offset + i * (size_t)base_size, buffer, j, - buf_len, parms, parms_index) < 0) + if (H5Z__nbit_decompress_one_array(data, data_offset + i * (size_t)base_size, buffer, + buffer_size, j, buf_len, parms, parms_index) < 0) HGOTO_ERROR(H5E_PLINE, H5E_CANTFILTER, FAIL, "can't decompress array"); *parms_index = begin_index; } @@ -1164,8 +1209,8 @@ H5Z__nbit_decompress_one_array(unsigned char *data, size_t data_offset, unsigned n = total_size / base_size; /* number of base_type elements inside the array datatype */ begin_index = *parms_index; for (i = 0; i < n; i++) { - if (H5Z__nbit_decompress_one_compound(data, data_offset + i * (size_t)base_size, buffer, j, - buf_len, parms, parms_index) < 0) + if (H5Z__nbit_decompress_one_compound(data, data_offset + i * (size_t)base_size, buffer, + buffer_size, j, buf_len, parms, parms_index) < 0) HGOTO_ERROR(H5E_PLINE, H5E_CANTFILTER, FAIL, "can't decompress compound"); *parms_index = begin_index; } @@ -1173,7 +1218,9 @@ H5Z__nbit_decompress_one_array(unsigned char *data, size_t data_offset, unsigned case H5Z_NBIT_NOOPTYPE: (*parms_index)++; /* skip size of no-op type */ - H5Z__nbit_decompress_one_nooptype(data, data_offset, buffer, j, buf_len, total_size); + if (H5Z__nbit_decompress_one_nooptype(data, data_offset, buffer, buffer_size, j, buf_len, + total_size) < 0) + HGOTO_ERROR(H5E_PLINE, H5E_CANTFILTER, FAIL, "can't decompress no-op type"); break; default: @@ -1185,8 +1232,9 @@ H5Z__nbit_decompress_one_array(unsigned char *data, size_t data_offset, unsigned } static herr_t -H5Z__nbit_decompress_one_compound(unsigned char *data, size_t data_offset, unsigned char *buffer, size_t *j, - size_t *buf_len, const unsigned parms[], unsigned *parms_index) +H5Z__nbit_decompress_one_compound(unsigned char *data, size_t data_offset, unsigned char *buffer, + size_t buffer_size, size_t *j, size_t *buf_len, const unsigned parms[], + unsigned *parms_index) { unsigned i, nmembers, member_offset, member_class, member_size, used_size = 0, prev_used_size, size; parms_atomic p; @@ -1224,26 +1272,29 @@ H5Z__nbit_decompress_one_compound(unsigned char *data, size_t data_offset, unsig if (p.precision > p.size * 8 || (p.precision + p.offset) > p.size * 8) HGOTO_ERROR(H5E_PLINE, H5E_BADTYPE, FAIL, "invalid datatype precision/offset"); - H5Z__nbit_decompress_one_atomic(data, data_offset + member_offset, buffer, j, buf_len, &p); + if (H5Z__nbit_decompress_one_atomic(data, data_offset + member_offset, buffer, buffer_size, j, + buf_len, &p) < 0) + HGOTO_ERROR(H5E_PLINE, H5E_CANTFILTER, FAIL, "can't decompress atomic"); break; case H5Z_NBIT_ARRAY: - if (H5Z__nbit_decompress_one_array(data, data_offset + member_offset, buffer, j, buf_len, - parms, parms_index) < 0) + if (H5Z__nbit_decompress_one_array(data, data_offset + member_offset, buffer, buffer_size, j, + buf_len, parms, parms_index) < 0) HGOTO_ERROR(H5E_PLINE, H5E_CANTFILTER, FAIL, "can't decompress array"); break; case H5Z_NBIT_COMPOUND: - if (H5Z__nbit_decompress_one_compound(data, data_offset + member_offset, buffer, j, buf_len, - parms, parms_index) < 0) + if (H5Z__nbit_decompress_one_compound(data, data_offset + member_offset, buffer, buffer_size, + j, buf_len, parms, parms_index) < 0) HGOTO_ERROR(H5E_PLINE, H5E_CANTFILTER, FAIL, "can't decompress compound"); break; case H5Z_NBIT_NOOPTYPE: /* Advance past member size */ (*parms_index)++; - H5Z__nbit_decompress_one_nooptype(data, data_offset + member_offset, buffer, j, buf_len, - member_size); + if (H5Z__nbit_decompress_one_nooptype(data, data_offset + member_offset, buffer, buffer_size, + j, buf_len, member_size) < 0) + HGOTO_ERROR(H5E_PLINE, H5E_CANTFILTER, FAIL, "can't decompress no-op type"); break; default: @@ -1256,7 +1307,8 @@ H5Z__nbit_decompress_one_compound(unsigned char *data, size_t data_offset, unsig } static herr_t -H5Z__nbit_decompress(unsigned char *data, unsigned d_nelmts, unsigned char *buffer, const unsigned parms[]) +H5Z__nbit_decompress(unsigned char *data, unsigned d_nelmts, unsigned char *buffer, size_t buffer_size, + const unsigned parms[]) { /* i: index of data, j: index of buffer, buf_len: number of bits to be filled in current byte */ @@ -1288,14 +1340,16 @@ H5Z__nbit_decompress(unsigned char *data, unsigned d_nelmts, unsigned char *buff HGOTO_ERROR(H5E_PLINE, H5E_BADTYPE, FAIL, "invalid datatype precision/offset"); for (i = 0; i < d_nelmts; i++) - H5Z__nbit_decompress_one_atomic(data, i * (size_t)p.size, buffer, &j, &buf_len, &p); + if (H5Z__nbit_decompress_one_atomic(data, i * (size_t)p.size, buffer, buffer_size, &j, + &buf_len, &p) < 0) + HGOTO_ERROR(H5E_PLINE, H5E_CANTFILTER, FAIL, "can't decompress atomic"); break; case H5Z_NBIT_ARRAY: size = parms[4]; parms_index = 4; /* set the index before goto function call */ for (i = 0; i < d_nelmts; i++) { - if (H5Z__nbit_decompress_one_array(data, i * size, buffer, &j, &buf_len, parms, + if (H5Z__nbit_decompress_one_array(data, i * size, buffer, buffer_size, &j, &buf_len, parms, &parms_index) < 0) HGOTO_ERROR(H5E_PLINE, H5E_CANTFILTER, FAIL, "can't decompress array"); parms_index = 4; @@ -1306,8 +1360,8 @@ H5Z__nbit_decompress(unsigned char *data, unsigned d_nelmts, unsigned char *buff size = parms[4]; parms_index = 4; /* set the index before goto function call */ for (i = 0; i < d_nelmts; i++) { - if (H5Z__nbit_decompress_one_compound(data, i * size, buffer, &j, &buf_len, parms, - &parms_index) < 0) + if (H5Z__nbit_decompress_one_compound(data, i * size, buffer, buffer_size, &j, &buf_len, + parms, &parms_index) < 0) HGOTO_ERROR(H5E_PLINE, H5E_CANTFILTER, FAIL, "can't decompress compound"); parms_index = 4; } diff --git a/test/CMakeTests.cmake b/test/CMakeTests.cmake index a41dbe746b65..5a7fe3b9b2ac 100644 --- a/test/CMakeTests.cmake +++ b/test/CMakeTests.cmake @@ -116,6 +116,9 @@ endforeach () set (HDF5_REFERENCE_TEST_FILES aggr.h5 bad_compound.h5 + bad_fletcher32.h5 + bad_nbit_decompress.h5 + bad_nbit_params.h5 bad_offset.h5 be_data.h5 be_extlink1.h5 @@ -1165,6 +1168,7 @@ endmacro () # generator executables set (H5_GENERATORS + gen_bad_filters gen_bad_offset gen_bad_ohdr gen_bogus diff --git a/test/dsets.c b/test/dsets.c index ca40d2df8d61..044bc11655d0 100644 --- a/test/dsets.c +++ b/test/dsets.c @@ -8031,6 +8031,102 @@ test_filters_endianess(void) return FAIL; } /* end test_filters_endianess() */ +/*------------------------------------------------------------------------- + * Function: test_filter_bad_params + * + * Purpose: Reads datasets from crafted files whose filter metadata has been + * deliberately corrupted, verifying that the library rejects them + * gracefully instead of crashing. These are regression tests for + * GitHub issues #6489 (N-Bit filter: NULL parameter array and + * compressed-buffer over-read) and #6490 (Fletcher32 filter: + * checksum length underflow). The files are produced by + * gen_bad_filters.c. + * + * Return: Success: 0 + * Failure: -1 + * + *------------------------------------------------------------------------- + */ +static herr_t +test_filter_bad_params(void) +{ + /* Each entry: crafted file name and the dataset whose read must fail. */ + static const struct { + const char *file; + const char *dset; + } cases[] = { + {"bad_nbit_params.h5", "Nbit_float_data_le"}, + {"bad_nbit_decompress.h5", "Nbit_float_data_le"}, + {"bad_fletcher32.h5", "Fletcher_float_data_be"}, + }; + hid_t fid = H5I_INVALID_HID; + hid_t did = H5I_INVALID_HID; + hid_t sid = H5I_INVALID_HID; + void *buf = NULL; + size_t i; + + TESTING("filter decode with corrupted filter metadata"); + + for (i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + const char *data_file = H5_get_srcdir_filename(cases[i].file); + hssize_t npoints; + herr_t status; + + if ((fid = H5Fopen(data_file, H5F_ACC_RDONLY, H5P_DEFAULT)) < 0) + FAIL_STACK_ERROR; + if ((did = H5Dopen2(fid, cases[i].dset, H5P_DEFAULT)) < 0) + FAIL_STACK_ERROR; + if ((sid = H5Dget_space(did)) < 0) + FAIL_STACK_ERROR; + if ((npoints = H5Sget_simple_extent_npoints(sid)) <= 0) + FAIL_STACK_ERROR; + if (NULL == (buf = calloc((size_t)npoints, sizeof(float)))) + TEST_ERROR; + + /* The corrupted filter metadata must make the read fail cleanly + * (return a negative value) rather than crash. */ + H5E_BEGIN_TRY + { + status = H5Dread(did, H5T_NATIVE_FLOAT, H5S_ALL, H5S_ALL, H5P_DEFAULT, buf); + } + H5E_END_TRY + + if (status >= 0) { + H5_FAILED(); + printf(" reading dataset \"%s\" from \"%s\" should have failed\n", cases[i].dset, + cases[i].file); + goto error; + } + + free(buf); + buf = NULL; + if (H5Sclose(sid) < 0) + FAIL_STACK_ERROR; + sid = H5I_INVALID_HID; + if (H5Dclose(did) < 0) + FAIL_STACK_ERROR; + did = H5I_INVALID_HID; + if (H5Fclose(fid) < 0) + FAIL_STACK_ERROR; + fid = H5I_INVALID_HID; + } + + PASSED(); + + return SUCCEED; + +error: + free(buf); + H5E_BEGIN_TRY + { + H5Sclose(sid); + H5Dclose(did); + H5Fclose(fid); + } + H5E_END_TRY + return FAIL; +} /* end test_filter_bad_params() */ + /*------------------------------------------------------------------------- * Function: test_zero_dims * @@ -19064,6 +19160,7 @@ main(void) if (driver_is_default_compatible) { nerrors += (test_filters_endianess() < 0 ? 1 : 0); + nerrors += (test_filter_bad_params() < 0 ? 1 : 0); } nerrors += (test_zero_dims(file) < 0 ? 1 : 0); diff --git a/test/gen_bad_filters.c b/test/gen_bad_filters.c new file mode 100644 index 000000000000..d6bc1a4c3d90 --- /dev/null +++ b/test/gen_bad_filters.c @@ -0,0 +1,355 @@ +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * Copyright by The HDF Group. * + * All rights reserved. * + * * + * This file is part of HDF5. The full HDF5 copyright notice, including * + * terms governing use, modification, and redistribution, is contained in * + * the LICENSE file, which can be found at the root of the source code * + * distribution tree, or in https://www.hdfgroup.org/licenses. * + * If you do not have access to either file, you may request a copy from * + * help@hdfgroup.org. * + * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ + +/* + * Generate HDF5 files with deliberately malformed filter metadata, used by + * the test_filter_bad_params() regression test in dsets.c. + * + * Each file contains a single chunked, filtered dataset whose on-disk filter + * metadata has been corrupted (by patching a few bytes after the file was + * written with the public API) so that reading the dataset exercises a filter + * decode path that used to crash: + * + * bad_nbit_params.h5 The N-Bit filter's stored client-data parameter + * count is set to 0, leaving cd_values == NULL at + * filter time (GitHub issue #6489 "Null_Pointer"). + * + * bad_nbit_decompress.h5 The N-Bit filter's stored element count (cd[2]) is + * inflated so decompression walks past the end of the + * small compressed chunk (GitHub issue #6489 + * "Heap_Corruption_1"). + * + * bad_fletcher32.h5 A Fletcher32-filtered chunk's stored size is set to + * 2 (smaller than the 4-byte trailing checksum), so + * the filter computes nbytes - FLETCHER_LEN and + * underflows (GitHub issue #6490 "Heap_Corruption_2"). + * A value of 2 (rather than 0) is used so the file is + * also exercisable in builds with assertions enabled, + * which otherwise trip the chunk B-tree's + * "nbytes > 0" sanity assertion before the filter + * runs. + * + * The files use the earliest library version bounds so the filter pipeline + * message lives in a version-1 (un-checksummed) object header; this lets us + * patch the pipeline parameters without having to recompute an object-header + * checksum. + */ + +#include "h5test.h" + +#define NBIT_PARAMS_FILE "bad_nbit_params.h5" +#define NBIT_DECOMPRESS_FILE "bad_nbit_decompress.h5" +#define FLETCHER32_FILE "bad_fletcher32.h5" + +#define NBIT_DATASET "Nbit_float_data_le" +#define FLETCHER32_DATASET "Fletcher_float_data_be" + +#define NX 7 +#define NY 6 +#define RANK 2 + +/*------------------------------------------------------------------------- + * Read an entire file into a newly allocated buffer. + *------------------------------------------------------------------------- + */ +static unsigned char * +slurp(const char *name, size_t *len_out) +{ + FILE *f = fopen(name, "rb"); + long len; + unsigned char *buf; + + if (!f) + return NULL; + if (fseek(f, 0, SEEK_END) != 0 || (len = ftell(f)) < 0 || fseek(f, 0, SEEK_SET) != 0) { + fclose(f); + return NULL; + } + if (NULL == (buf = malloc((size_t)len))) { + fclose(f); + return NULL; + } + if (fread(buf, 1, (size_t)len, f) != (size_t)len) { + free(buf); + fclose(f); + return NULL; + } + fclose(f); + *len_out = (size_t)len; + return buf; +} + +/* Write a buffer back out to a file. */ +static int +spew(const char *name, const unsigned char *buf, size_t len) +{ + FILE *f = fopen(name, "wb"); + if (!f) + return -1; + if (fwrite(buf, 1, len, f) != len) { + fclose(f); + return -1; + } + return fclose(f) == 0 ? 0 : -1; +} + +/* Find the unique occurrence of pattern in buf; return offset or (size_t)-1. */ +static size_t +find_once(const unsigned char *buf, size_t len, const unsigned char *pat, size_t patlen) +{ + size_t i, found = (size_t)-1; + + if (patlen == 0 || len < patlen) + return (size_t)-1; + for (i = 0; i <= len - patlen; i++) { + if (memcmp(buf + i, pat, patlen) == 0) { + if (found != (size_t)-1) + return (size_t)-1; /* not unique */ + found = i; + } + } + return found; +} + +/* Store a little-endian unsigned value of the given width. */ +static void +put_le(unsigned char *p, unsigned long long val, unsigned width) +{ + unsigned i; + for (i = 0; i < width; i++) + p[i] = (unsigned char)((val >> (8 * i)) & 0xff); +} + +/*------------------------------------------------------------------------- + * Create a single N-Bit-filtered float dataset matching the one produced by + * gen_cross.c, then corrupt it two different ways. + *------------------------------------------------------------------------- + */ +static int +create_nbit_files(void) +{ + hid_t fapl = H5I_INVALID_HID, file = H5I_INVALID_HID, sid = H5I_INVALID_HID; + hid_t dcpl = H5I_INVALID_HID, dtype = H5I_INVALID_HID, dset = H5I_INVALID_HID; + hsize_t dims[RANK] = {NX, NY}; + hsize_t chunk[RANK] = {2, 3}; + float data[NX][NY]; + float fillvalue = -2.2f; + unsigned char *buf = NULL; + size_t len, anchor, cd_off; + int i, j; + + /* The N-Bit cd_values stored for this dataset: + * [0]=nparms(8) [1]=need_not_compress(0) [2]=nelmts(6) [3]=class(atomic) + * [4]=size(4) [5]=order(LE) [6]=precision(20) [7]=offset(7) + */ + static const unsigned char cd_pattern[] = {8, 0, 0, 0, 0, 0, 0, 0, 6, 0, 0, 0, 1, 0, 0, 0, + 4, 0, 0, 0, 0, 0, 0, 0, 20, 0, 0, 0, 7, 0, 0, 0}; + /* Version-1 pipeline filter record header for the N-Bit filter: + * filter id = 5, name length = 8, flags = 1, cd_nelmts = 8, name "nbit" + */ + static const unsigned char filter_anchor[] = {5, 0, 8, 0, 1, 0, 8, 0, 'n', 'b', 'i', 't'}; + + for (j = 0; j < NX; j++) + for (i = 0; i < NY; i++) + data[j][i] = ((float)(i + j + 1)) / 3; + + if ((fapl = H5Pcreate(H5P_FILE_ACCESS)) < 0) + TEST_ERROR; + if (H5Pset_libver_bounds(fapl, H5F_LIBVER_EARLIEST, H5F_LIBVER_LATEST) < 0) + TEST_ERROR; + if ((file = H5Fcreate(NBIT_PARAMS_FILE, H5F_ACC_TRUNC, H5P_DEFAULT, fapl)) < 0) + TEST_ERROR; + if ((sid = H5Screate_simple(RANK, dims, NULL)) < 0) + TEST_ERROR; + + if ((dcpl = H5Pcreate(H5P_DATASET_CREATE)) < 0) + TEST_ERROR; + if (H5Pset_nbit(dcpl) < 0) + TEST_ERROR; + if (H5Pset_chunk(dcpl, RANK, chunk) < 0) + TEST_ERROR; + if (H5Pset_fill_value(dcpl, H5T_NATIVE_FLOAT, &fillvalue) < 0) + TEST_ERROR; + + /* 20-bit little-endian floating-point type (same as gen_cross.c) */ + if ((dtype = H5Tcopy(H5T_IEEE_F32LE)) < 0) + TEST_ERROR; + if (H5Tset_fields(dtype, (size_t)26, (size_t)20, (size_t)6, (size_t)7, (size_t)13) < 0) + TEST_ERROR; + if (H5Tset_offset(dtype, (size_t)7) < 0) + TEST_ERROR; + if (H5Tset_precision(dtype, (size_t)20) < 0) + TEST_ERROR; + if (H5Tset_size(dtype, (size_t)4) < 0) + TEST_ERROR; + if (H5Tset_ebias(dtype, (size_t)31) < 0) + TEST_ERROR; + + if ((dset = H5Dcreate2(file, NBIT_DATASET, dtype, sid, H5P_DEFAULT, dcpl, H5P_DEFAULT)) < 0) + TEST_ERROR; + if (H5Dwrite(dset, H5T_NATIVE_FLOAT, H5S_ALL, H5S_ALL, H5P_DEFAULT, data) < 0) + TEST_ERROR; + + if (H5Dclose(dset) < 0 || H5Tclose(dtype) < 0 || H5Pclose(dcpl) < 0 || H5Sclose(sid) < 0 || + H5Fclose(file) < 0 || H5Pclose(fapl) < 0) + TEST_ERROR; + + /* Slurp the freshly written (clean) file once and locate the filter + * record and its cd_values; both bad files are derived from this buffer. */ + if (NULL == (buf = slurp(NBIT_PARAMS_FILE, &len))) + TEST_ERROR; + if ((cd_off = find_once(buf, len, cd_pattern, sizeof(cd_pattern))) == (size_t)-1) + TEST_ERROR; + if ((anchor = find_once(buf, len, filter_anchor, sizeof(filter_anchor))) == (size_t)-1) + TEST_ERROR; + + /* File 2 (decompress over-read): inflate cd[2] (element count, the third + * 4-byte value of cd_values) on a copy of the clean buffer so the chunk is + * too small for the claimed element count. Done first, before the clean + * buffer is mutated for file 1. */ + { + unsigned char *copy = malloc(len); + if (NULL == copy) + TEST_ERROR; + memcpy(copy, buf, len); + put_le(copy + cd_off + 8, 100000, 4); + if (spew(NBIT_DECOMPRESS_FILE, copy, len) < 0) { + free(copy); + TEST_ERROR; + } + free(copy); + } + + /* File 1 (NULL parameters): zero the stored cd_nelmts (2 bytes at + * anchor+6) so the filter is invoked with cd_nelmts == 0 / cd_values == + * NULL. Overwrites the clean base file in place. */ + put_le(buf + anchor + 6, 0, 2); + if (spew(NBIT_PARAMS_FILE, buf, len) < 0) + TEST_ERROR; + + free(buf); + return 0; + +error: + free(buf); + H5E_BEGIN_TRY + { + H5Dclose(dset); + H5Tclose(dtype); + H5Pclose(dcpl); + H5Sclose(sid); + H5Fclose(file); + H5Pclose(fapl); + } + H5E_END_TRY + return -1; +} + +/*------------------------------------------------------------------------- + * Create a single Fletcher32-filtered float dataset, then set its one chunk's + * stored size to 2 (smaller than the 4-byte checksum) to trigger the + * checksum-length underflow. + *------------------------------------------------------------------------- + */ +static int +create_fletcher32_file(void) +{ + hid_t fapl = H5I_INVALID_HID, file = H5I_INVALID_HID, sid = H5I_INVALID_HID; + hid_t dcpl = H5I_INVALID_HID, dtype = H5I_INVALID_HID, dset = H5I_INVALID_HID; + hsize_t dims[RANK] = {NX, NY}; + hsize_t chunk[RANK] = {NX, NY}; /* single chunk holds the whole dataset */ + float data[NX][NY]; + float fillvalue = -2.2f; + unsigned char *buf = NULL; + size_t len, i; + int r, c; + + for (r = 0; r < NX; r++) + for (c = 0; c < NY; c++) + data[r][c] = ((float)(c + r + 1)) / 3; + + if ((fapl = H5Pcreate(H5P_FILE_ACCESS)) < 0) + TEST_ERROR; + if (H5Pset_libver_bounds(fapl, H5F_LIBVER_EARLIEST, H5F_LIBVER_LATEST) < 0) + TEST_ERROR; + if ((file = H5Fcreate(FLETCHER32_FILE, H5F_ACC_TRUNC, H5P_DEFAULT, fapl)) < 0) + TEST_ERROR; + if ((sid = H5Screate_simple(RANK, dims, NULL)) < 0) + TEST_ERROR; + + if ((dcpl = H5Pcreate(H5P_DATASET_CREATE)) < 0) + TEST_ERROR; + if (H5Pset_fletcher32(dcpl) < 0) + TEST_ERROR; + if (H5Pset_chunk(dcpl, RANK, chunk) < 0) + TEST_ERROR; + if (H5Pset_fill_value(dcpl, H5T_NATIVE_FLOAT, &fillvalue) < 0) + TEST_ERROR; + + if ((dtype = H5Tcopy(H5T_IEEE_F32BE)) < 0) + TEST_ERROR; + if ((dset = H5Dcreate2(file, FLETCHER32_DATASET, dtype, sid, H5P_DEFAULT, dcpl, H5P_DEFAULT)) < 0) + TEST_ERROR; + if (H5Dwrite(dset, H5T_NATIVE_FLOAT, H5S_ALL, H5S_ALL, H5P_DEFAULT, data) < 0) + TEST_ERROR; + + if (H5Dclose(dset) < 0 || H5Tclose(dtype) < 0 || H5Pclose(dcpl) < 0 || H5Sclose(sid) < 0 || + H5Fclose(file) < 0 || H5Pclose(fapl) < 0) + TEST_ERROR; + + /* Find the raw-data chunk's version-1 B-tree node ("TREE" with a node type + * byte of 1) and shrink the first key's chunk size to 2 (4 bytes at + * node + 24: sig(4) + type(1) + level(1) + entries(2) + left(8) + right(8)). + * 2 is smaller than the 4-byte Fletcher32 checksum, so the read underflows, + * but it is still > 0 so assertion-enabled builds reach the filter. */ + if (NULL == (buf = slurp(FLETCHER32_FILE, &len))) + TEST_ERROR; + for (i = 0; i + 24 + 4 <= len; i++) { + if (memcmp(buf + i, "TREE", 4) == 0 && buf[i + 4] == 1) { + put_le(buf + i + 24, 2, 4); + if (spew(FLETCHER32_FILE, buf, len) < 0) + TEST_ERROR; + free(buf); + return 0; + } + } + TEST_ERROR; /* no raw-data B-tree found */ + +error: + free(buf); + H5E_BEGIN_TRY + { + H5Dclose(dset); + H5Tclose(dtype); + H5Pclose(dcpl); + H5Sclose(sid); + H5Fclose(file); + H5Pclose(fapl); + } + H5E_END_TRY + return -1; +} + +int +main(void) +{ + if (create_nbit_files() < 0) { + fprintf(stderr, "failed to generate N-Bit bad-filter files\n"); + return EXIT_FAILURE; + } + if (create_fletcher32_file() < 0) { + fprintf(stderr, "failed to generate Fletcher32 bad-filter file\n"); + return EXIT_FAILURE; + } + printf("Generated %s, %s, %s\n", NBIT_PARAMS_FILE, NBIT_DECOMPRESS_FILE, FLETCHER32_FILE); + return EXIT_SUCCESS; +} diff --git a/test/testfiles/bad_fletcher32.h5 b/test/testfiles/bad_fletcher32.h5 new file mode 100644 index 000000000000..f77c289ce337 Binary files /dev/null and b/test/testfiles/bad_fletcher32.h5 differ diff --git a/test/testfiles/bad_nbit_decompress.h5 b/test/testfiles/bad_nbit_decompress.h5 new file mode 100644 index 000000000000..15b803894519 Binary files /dev/null and b/test/testfiles/bad_nbit_decompress.h5 differ diff --git a/test/testfiles/bad_nbit_params.h5 b/test/testfiles/bad_nbit_params.h5 new file mode 100644 index 000000000000..586e37aa5f5f Binary files /dev/null and b/test/testfiles/bad_nbit_params.h5 differ