Fixed asserts due to H5Pset_est_link_info() values (HDFGroup#4081)

* Fixed asserts due to H5Pset_est_link_info() values

If large values for est_num_entries and/or est_name_len were passed
to H5Pset_est_link_info(), the library would attempt to create an
object header NIL message to reserve enough space to hold the links in
compact form (i.e., concatenated), which could exceed allowable object
header message size limits and trip asserts in the library.

This bug only occurred when using the HDF5 1.8 file format or later and
required the product of the two values to be ~64k more than the size
of any links written to the group, which would cause the library to
write out a too-large NIL spacer message to reserve the space for the
unwritten links.

The library now inspects the phase change values to see if the dataset
is likely to be compact and checks the size to ensure any NIL spacer
messages won't be larger than the library allows.

Fixes GitHub HDFGroup#1632

* Fix copy-paste comments

release_docs/RELEASE.txt

@@ -219,6 +219,26 @@ Bug Fixes since HDF5-1.14.3 release
 ===================================
     Library
     -------
+    - Fixed asserts raised by large values of H5Pset_est_link_info() parameters
+
+      If large values for est_num_entries and/or est_name_len were passed
+      to H5Pset_est_link_info(), the library would attempt to create an
+      object header NIL message to reserve enough space to hold the links in
+      compact form (i.e., concatenated), which could exceed allowable object
+      header message size limits and trip asserts in the library.
+
+      This bug only occurred when using the HDF5 1.8 file format or later and
+      required the product of the two values to be ~64k more than the size
+      of any links written to the group, which would cause the library to
+      write out a too-large NIL spacer message to reserve the space for the
+      unwritten links.
+
+      The library now inspects the phase change values to see if the dataset
+      is likely to be compact and checks the size to ensure any NIL spacer
+      messages won't be larger than the library allows.
+
+      Fixes GitHub #1632
+
     - Fixed a bug where H5Tset_fields does not account for any offset
       set for a floating-point datatype when determining if values set
       for spos, epos, esize, mpos and msize make sense for the datatype

src/H5Gobj.c

@@ -218,7 +218,25 @@ H5G__obj_create_real(H5F_t *f, const H5O_ginfo_t *ginfo, const H5O_linfo_t *linf
         assert(link_size);
 
         /* Compute size of header to use for creation */
-        hdr_size = linfo_size + ginfo_size + pline_size + (ginfo->est_num_entries * link_size);
+
+        /* Basic header size */
+        hdr_size = linfo_size + ginfo_size + pline_size;
+
+        /* If this is likely to be a compact group, add space for the link
+         * messages, unless the size of the link messages is greater than
+         * the largest allowable object header message size, since the size
+         * of the link messages is the size of the NIL spacer message that
+         * would have to be written out to reserve enough space to hold the
+         * links if the group were left empty.
+         */
+        bool compact = ginfo->est_num_entries <= ginfo->max_compact;
+        if (compact) {
+
+            size_t size_of_links = ginfo->est_num_entries * link_size;
+
+            if (size_of_links < H5O_MESG_MAX_SIZE)
+                hdr_size += size_of_links;
+        }
     } /* end if */
     else
         hdr_size = (size_t)(4 + 2 * H5F_SIZEOF_ADDR(f));

test/tmisc.c

@@ -338,6 +338,7 @@ typedef struct {
 #define CVE_2020_10812_FILENAME "cve_2020_10812.h5"
 
 #define MISC38_FILE "type_conversion_path_table_issue.h5"
+#define MISC39_FILE "set_est_link_info.h5"
 
 /****************************************************************
 **
@@ -6445,6 +6446,103 @@ test_misc38(void)
     CHECK(ret, FAIL, "H5Sclose");
 }
 
+/****************************************************************
+**
+**  test_misc39(): Ensure H5Pset_est_link_info() handles large
+**                 values
+**
+**  H5Pset_est_link_info() values can be set to large values,
+**  which could cause the library to attempt to create large
+**  object headers that exceed limits and trip asserts in
+**  the library.
+**
+**  This test ensures that the library doesn't error regardless
+**  of the values passed to H5Pset_est_link_info() and
+**  H5Pset_link_phase_change().
+**
+****************************************************************/
+static void
+test_misc39(void)
+{
+    hid_t  fid  = H5I_INVALID_HID; /* File ID */
+    hid_t  gid  = H5I_INVALID_HID; /* Group ID */
+    hid_t  fapl = H5I_INVALID_HID; /* File access property list ID */
+    hid_t  gcpl = H5I_INVALID_HID; /* Group creation property list ID */
+    herr_t ret  = H5I_INVALID_HID; /* Generic return value */
+
+    /* Output message about test being performed */
+    MESSAGE(5, ("Ensure H5Pset_est_link_info handles large values\n"));
+
+    /* Compose file access property list
+     *
+     * NOTE: The bug in question only occurs in new-style groups
+     */
+    fapl = H5Pcreate(H5P_FILE_ACCESS);
+    CHECK(fapl, H5I_INVALID_HID, "H5Pcreate");
+    ret = H5Pset_libver_bounds(fapl, H5F_LIBVER_LATEST, H5F_LIBVER_LATEST);
+    CHECK(ret, FAIL, "H5Pset_libver_bounds");
+
+    /* Create the file */
+    fid = H5Fcreate(MISC39_FILE, H5F_ACC_TRUNC, H5P_DEFAULT, fapl);
+    CHECK(fid, H5I_INVALID_HID, "H5Fcreate");
+
+    /* Compose group creation property list */
+    gcpl = H5Pcreate(H5P_GROUP_CREATE);
+    CHECK(gcpl, H5I_INVALID_HID, "H5Pcreate");
+
+    /* Set the estimated link info values to large numbers */
+    ret = H5Pset_est_link_info(gcpl, UINT16_MAX, UINT16_MAX);
+    CHECK(ret, FAIL, "H5Pset_est_link_info");
+
+    /* Create a group */
+    gid = H5Gcreate2(fid, "foo", H5P_DEFAULT, gcpl, H5P_DEFAULT);
+    CHECK(gid, H5I_INVALID_HID, "H5Gcreate2");
+
+    /* Close the group */
+    ret = H5Gclose(gid);
+    CHECK(ret, FAIL, "H5Gclose");
+
+    /* Close the file. Asserts typically occur here, when the metadata cache
+     * objects are flushed.
+     */
+    ret = H5Fclose(fid);
+    CHECK(ret, FAIL, "H5Fclose");
+
+    /* Re-open the file */
+    fid = H5Fopen(MISC25C_FILE, H5F_ACC_RDWR, H5P_DEFAULT);
+    CHECK(fid, H5I_INVALID_HID, "H5Fopen");
+
+    /* Set the compact/dense value high, to see if we can trick the
+     * library into creating a dense group object header that is
+     * larger than the maximum allowed size.
+     */
+    ret = H5Pset_link_phase_change(gcpl, UINT16_MAX, UINT16_MAX);
+    CHECK(ret, FAIL, "H5Pset_link_phase_change");
+
+    /* Set the estimated link info values to large numbers */
+    ret = H5Pset_est_link_info(gcpl, UINT16_MAX / 2, UINT16_MAX);
+    CHECK(ret, FAIL, "H5Pset_est_link_info");
+
+    /* Create another group */
+    gid = H5Gcreate2(fid, "bar", H5P_DEFAULT, gcpl, H5P_DEFAULT);
+    CHECK(gid, H5I_INVALID_HID, "H5Gcreate2");
+
+    /* Close the group */
+    ret = H5Gclose(gid);
+    CHECK(ret, FAIL, "H5Gclose");
+
+    /* Close the file */
+    ret = H5Fclose(fid);
+    CHECK(ret, FAIL, "H5Fclose");
+
+    /* Close the property lists */
+    ret = H5Pclose(fapl);
+    CHECK(ret, FAIL, "H5Pclose");
+    ret = H5Pclose(gcpl);
+    CHECK(ret, FAIL, "H5Pclose");
+
+} /* end test_misc39() */
+
 /****************************************************************
 **
 **  test_misc(): Main misc. test routine.
@@ -6514,6 +6612,7 @@ test_misc(void)
     test_misc36(); /* Exercise H5atclose and H5is_library_terminating */
     test_misc37(); /* Test for seg fault failure at file close */
     test_misc38(); /* Test for type conversion path table issue */
+    test_misc39(); /* Ensure H5Pset_est_link_info() handles large values */
 
 } /* test_misc() */
 
@@ -6570,6 +6669,7 @@ cleanup_misc(void)
         H5Fdelete(MISC31_FILE, H5P_DEFAULT);
 #endif /* H5_NO_DEPRECATED_SYMBOLS */
         H5Fdelete(MISC38_FILE, H5P_DEFAULT);
+        H5Fdelete(MISC39_FILE, H5P_DEFAULT);
     }
     H5E_END_TRY
 } /* end cleanup_misc() */