reject SOHM list message count exceeding list_max

release_docs/CHANGELOG.md

@@ -144,6 +144,10 @@ The `h5repack` tool now obtains its default low and high library version bounds
 
 ## Library
 
+### Fixed a heap buffer overflow when decoding a shared message list
+
+   When reading a shared object header message (SOHM) list from the metadata cache, `H5SM__cache_list_deserialize()` allocated the message array for `list_max` entries but drove the decode loop with the `num_messages` count read from the on-disk index header. A corrupted or malicious file whose `num_messages` exceeds `list_max` caused writes past the end of the array and reads past the end of the input buffer. The count is now validated against `list_max` before the loop runs.
+
 ### 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/H5SMcache.c

@@ -489,6 +489,12 @@ H5SM__cache_list_verify_chksum(const void *_image, size_t H5_ATTR_UNUSED len, vo
     assert(image);
     assert(udata);
 
+    /* The buffer only holds list_max messages; a corrupted header whose message
+     * count exceeds that would size the checksum region past the end of it.
+     */
+    if (udata->header->num_messages > udata->header->list_max)
+        HGOTO_ERROR(H5E_SOHM, H5E_BADVALUE, FAIL, "number of SOHM messages exceeds list size");
+
     /* Exact size with checksum at the end */
     chk_size = H5SM_LIST_SIZE(udata->f, udata->header->num_messages);
 
@@ -552,6 +558,14 @@ H5SM__cache_list_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUSED le
         HGOTO_ERROR(H5E_SOHM, H5E_CANTLOAD, NULL, "bad SOHM list signature");
     image += H5_SIZEOF_MAGIC;
 
+    /* The message array is sized for list_max entries; a list index always
+     * holds at most that many before it is promoted to a B-tree. Reject a
+     * corrupted header whose message count would drive the decode loop past
+     * the allocation and the input buffer.
+     */
+    if (udata->header->num_messages > udata->header->list_max)
+        HGOTO_ERROR(H5E_SOHM, H5E_CANTLOAD, NULL, "number of SOHM messages exceeds list size");
+
     /* Read messages into the list array */
     ctx.sizeof_addr = H5F_SIZEOF_ADDR(udata->f);
     for (u = 0; u < udata->header->num_messages; u++) {

test/tsohm.c

@@ -3702,6 +3702,175 @@ test_sohm_external_dtype(void)
     free(orig);
 } /* test_sohm_external_dtype */
 
+/*-------------------------------------------------------------------------
+ * Function:    test_sohm_reject_bad_count
+ *
+ * Purpose:     A shared-message list index holds at most list_max messages
+ *              on disk before it is promoted to a B-tree, and both the list
+ *              image buffer and the in-memory message array are sized for
+ *              list_max entries. Corrupt the on-disk message count so it
+ *              exceeds list_max and verify the list load rejects the file
+ *              instead of sizing a read or indexing the array past its end.
+ *
+ *-------------------------------------------------------------------------
+ */
+static void
+test_sohm_reject_bad_count(void)
+{
+    hid_t    fcpl_id   = H5I_INVALID_HID;
+    hid_t    fid       = H5I_INVALID_HID;
+    hid_t    sid       = H5I_INVALID_HID;
+    hid_t    did       = H5I_INVALID_HID;
+    hsize_t  dims[1]   = {4};
+    FILE    *fp        = NULL;
+    uint8_t *buf       = NULL;
+    long     fsize     = 0;
+    long     table_off = -1;
+    unsigned list_max  = 100;
+    uint32_t chksum;
+    size_t   pos;
+    int      i;
+    herr_t   ret;
+
+    /* On-disk shared-message table layout for a file with a single index and
+     * 8-byte addresses: a 4-byte "SMTB" signature, one index record, then a
+     * 4-byte checksum. Within the record the 16-bit message count follows the
+     * (version, index type, message types, minimum size) prefix and the 16-bit
+     * list and B-tree cutoffs.
+     */
+    const size_t rec_size     = 1 + 1 + 2 + 4 + 3 * 2 + 8 + 8;                     /* 30 */
+    const size_t table_body   = (size_t)H5_SIZEOF_MAGIC + rec_size;                /* 34 */
+    const size_t num_msgs_off = (size_t)H5_SIZEOF_MAGIC + (1 + 1 + 2 + 4 + 2 + 2); /* 16 */
+
+    MESSAGE(5, ("Testing rejection of an out-of-range SOHM list message count\n"));
+
+    /* Create a file whose single shared-message index is a list that can hold
+     * up to list_max messages before converting to a B-tree.
+     */
+    fcpl_id = H5Pcreate(H5P_FILE_CREATE);
+    CHECK_I(fcpl_id, "H5Pcreate");
+    ret = H5Pset_shared_mesg_nindexes(fcpl_id, 1);
+    CHECK_I(ret, "H5Pset_shared_mesg_nindexes");
+    ret = H5Pset_shared_mesg_index(fcpl_id, 0, H5O_SHMESG_SDSPACE_FLAG | H5O_SHMESG_DTYPE_FLAG, 1);
+    CHECK_I(ret, "H5Pset_shared_mesg_index");
+    ret = H5Pset_shared_mesg_phase_change(fcpl_id, list_max, 0);
+    CHECK_I(ret, "H5Pset_shared_mesg_phase_change");
+
+    fid = H5Fcreate(FILENAME, H5F_ACC_TRUNC, fcpl_id, H5P_DEFAULT);
+    CHECK_I(fid, "H5Fcreate");
+
+    /* Several datasets sharing one dataspace and datatype leave the index a
+     * list holding a couple of messages, well under list_max.
+     */
+    sid = H5Screate_simple(1, dims, NULL);
+    CHECK_I(sid, "H5Screate_simple");
+    for (i = 0; i < 5; i++) {
+        char name[16];
+
+        snprintf(name, sizeof(name), "dset%d", i);
+        did = H5Dcreate2(fid, name, H5T_NATIVE_INT, sid, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
+        CHECK_I(did, "H5Dcreate2");
+        ret = H5Dclose(did);
+        CHECK_I(ret, "H5Dclose");
+    }
+    ret = H5Sclose(sid);
+    CHECK_I(ret, "H5Sclose");
+    ret = H5Fclose(fid);
+    CHECK_I(ret, "H5Fclose");
+
+    /* Read the whole file so the shared-message table can be located and edited. */
+    fp = fopen(FILENAME, "rb");
+    CHECK_PTR(fp, "fopen");
+    if (fp) {
+        if (fseek(fp, 0, SEEK_END) != 0)
+            TestErrPrintf("fseek failed at line %d\n", __LINE__);
+        fsize = ftell(fp);
+        if (fsize <= (long)(table_body + 4))
+            TestErrPrintf("unexpected file size %ld at line %d\n", fsize, __LINE__);
+        rewind(fp);
+
+        buf = (uint8_t *)malloc((size_t)fsize);
+        CHECK_PTR(buf, "malloc");
+        if (buf && fread(buf, 1, (size_t)fsize, fp) != (size_t)fsize)
+            TestErrPrintf("fread failed at line %d\n", __LINE__);
+        if (fclose(fp) != 0)
+            TestErrPrintf("fclose failed at line %d\n", __LINE__);
+        fp = NULL;
+    }
+
+    /* Find the shared-message table by signature, confirming the match with the
+     * stored checksum so the correct bytes are edited.
+     */
+    for (pos = 0; buf && (pos + table_body + 4) <= (size_t)fsize; pos++) {
+        if (memcmp(buf + pos, H5SM_TABLE_MAGIC, (size_t)H5_SIZEOF_MAGIC) != 0)
+            continue;
+
+        chksum = (uint32_t)buf[pos + table_body] | ((uint32_t)buf[pos + table_body + 1] << 8) |
+                 ((uint32_t)buf[pos + table_body + 2] << 16) | ((uint32_t)buf[pos + table_body + 3] << 24);
+        if (chksum == H5_checksum_metadata(buf + pos, table_body, 0)) {
+            table_off = (long)pos;
+            break;
+        }
+    }
+    if (table_off < 0)
+        TestErrPrintf("could not locate the shared-message table in %s\n", FILENAME);
+
+    if (buf && table_off >= 0) {
+        unsigned bad_count = list_max + 200; /* well past the list_max cutoff */
+        size_t   base      = (size_t)table_off;
+
+        /* Overwrite the 16-bit message count and repair the table checksum so
+         * the table loads and the corruption is only caught at the list.
+         */
+        buf[base + num_msgs_off]     = (uint8_t)(bad_count & 0xff);
+        buf[base + num_msgs_off + 1] = (uint8_t)((bad_count >> 8) & 0xff);
+
+        chksum                     = H5_checksum_metadata(buf + base, table_body, 0);
+        buf[base + table_body]     = (uint8_t)(chksum & 0xff);
+        buf[base + table_body + 1] = (uint8_t)((chksum >> 8) & 0xff);
+        buf[base + table_body + 2] = (uint8_t)((chksum >> 16) & 0xff);
+        buf[base + table_body + 3] = (uint8_t)((chksum >> 24) & 0xff);
+
+        fp = fopen(FILENAME, "r+b");
+        CHECK_PTR(fp, "fopen");
+        if (fp) {
+            if (fwrite(buf, 1, (size_t)fsize, fp) != (size_t)fsize)
+                TestErrPrintf("fwrite failed at line %d\n", __LINE__);
+            if (fclose(fp) != 0)
+                TestErrPrintf("fclose failed at line %d\n", __LINE__);
+            fp = NULL;
+        }
+
+        /* Reopen and share a new message, which protects the list and drives
+         * the vulnerable decode. The load should reject the file cleanly.
+         */
+        fid = H5Fopen(FILENAME, H5F_ACC_RDWR, H5P_DEFAULT);
+        CHECK_I(fid, "H5Fopen");
+        sid = H5Screate_simple(1, dims, NULL);
+        CHECK_I(sid, "H5Screate_simple");
+
+        H5E_BEGIN_TRY
+        {
+            did = H5Dcreate2(fid, "trigger", H5T_NATIVE_INT, sid, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
+        }
+        H5E_END_TRY
+
+        if (did >= 0) {
+            TestErrPrintf("dataset creation succeeded on a corrupted SOHM list at line %d\n", __LINE__);
+            H5Dclose(did);
+        }
+
+        ret = H5Sclose(sid);
+        CHECK_I(ret, "H5Sclose");
+        ret = H5Fclose(fid);
+        CHECK_I(ret, "H5Fclose");
+    }
+
+    free(buf);
+    ret = H5Pclose(fcpl_id);
+    CHECK_I(ret, "H5Pclose");
+} /* test_sohm_reject_bad_count */
+
 /****************************************************************
 **
 **  test_sohm(): Main Shared Object Header Message testing routine.
@@ -3755,6 +3924,10 @@ test_sohm(void H5_ATTR_UNUSED *params)
 
     test_sohm_extend_dset();    /* Test extending shared datasets */
     test_sohm_external_dtype(); /* Test using datatype in another file */
+
+    /* Editing the on-disk table in place needs the single-file sec2 layout */
+    if (default_driver)
+        test_sohm_reject_bad_count(); /* Test rejecting a bad SOHM list message count */
 } /* test_sohm */
 
 /*-------------------------------------------------------------------------