Fix h5ls SIGSEGVs in Nbit and Fletcher32 filter decode paths

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.

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)) {

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

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);