Fixes crashes when size_hint > UINT32_MAX is passed to H5Gcreate1 (#611)

* Committing clang-format changes

* Fixes incorrect size_hint handling in H5Gcreate1

* Updates the size hint type for group creation

* Updates the RELEASE.txt note

* Revert "Updates the RELEASE.txt note"

This reverts commit 3df386acca.

* Reverts previous behavior to use a uint32_t struct field

* Updates RELEASE.txt

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

release_docs/RELEASE.txt

@@ -414,6 +414,24 @@ New Features
 
     Library:
     --------
+    - H5Gcreate1() now rejects size_hint parameters larger than UINT32_MAX
+
+      The size_hint value is ultimately stored in a uint32_t struct field,
+      so specifying a value larger than this on a 64-bit machine can cause
+      undefined behavior including crashing the system.
+
+      The documentation for this API call was also incorrect, stating that
+      passing a negative value would cause the library to use a default
+      value. Instead, passing a "negative" value actually passes a very large
+      value, which is probably not what the user intends and can cause
+      crashes on 64-bit systems.
+
+      The Doxygen documentation has been updated and passing values larger
+      than UINT32_MAX for size_hint will now produce a normal HDF5 error.
+      
+        (DER - 2021/04/29, HDFFV-11241)
+
+
     - H5Pset_fapl_log() no longer crashes when passed an invalid fapl ID
 
       When passed an invalid fapl ID, H5Pset_fapl_log() would usually

src/H5Gdeprec.c

@@ -141,10 +141,10 @@ H5G_map_obj_type(H5O_type_t obj_type)
  *		specified NAME.  The group is opened for write access
  *		and it's object ID is returned.
  *
- *		The optional SIZE_HINT specifies how much file space to
- *		reserve to store the names that will appear in this
- *		group. If a non-positive value is supplied for the SIZE_HINT
- *		then a default size is chosen.
+ *		The SIZE_HINT parameter specifies how much file space to reserve
+ *		to store the names that will appear in this group. This number
+ *		must be less than or equal to UINT32_MAX. If zero is supplied
+ *		for the SIZE_HINT then a default size is chosen.
  *
  * Note:	Deprecated in favor of H5Gcreate2
  *
@@ -174,6 +174,8 @@ H5Gcreate1(hid_t loc_id, const char *name, size_t size_hint)
     /* Check arguments */
     if (!name || !*name)
         HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, H5I_INVALID_HID, "no name given")
+    if (size_hint > UINT32_MAX)
+        HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, H5I_INVALID_HID, "size_hint cannot be larger than UINT32_MAX")
 
     /* Check if we need to create a non-standard GCPL */
     if (size_hint > 0) {

src/H5Gpublic.h

@@ -569,8 +569,8 @@ typedef struct H5G_stat_t {
  *
  * \fgdta_loc_id
  * \param[in] name      Name of the group to create
- * \param[in] size_hint Optional parameter indicating the number of bytes
- *                      to reserve for the names that will appear in the group
+ * \param[in] size_hint The number of bytes to reserve for the names
+ *                      that will appear in the group
  *
  * \return \hid_t{group}
  *
@@ -592,11 +592,9 @@ typedef struct H5G_stat_t {
  *          group, is not limited.
  *
  *          \p size_hint is a hint for the number of bytes to reserve to store
- *          the names which will be eventually added to the new group. Passing a
- *          value of zero for \p size_hint is usually adequate since the library
- *          is able to dynamically resize the name heap, but a correct hint may
- *          result in better performance. If a non-positive value is supplied
- *          for \p size_hint, then a default size is chosen.
+ *          the names which will be eventually added to the new group. This
+ *          value must be between 0 and UINT32_MAX (inclusive). If this
+ *          parameter is zero, a default value will be used.
  *
  *          The return value is a group identifier for the open group. This
  *          group identifier should be closed by calling H5Gclose() when it is

test/tmisc.c

@@ -4088,9 +4088,31 @@ test_misc23(void)
     H5E_END_TRY;
     VERIFY(tmp_id, FAIL, "H5Gcreate1");
 
+    /* Make sure that size_hint values that can't fit into a 32-bit
+     * unsigned integer are rejected. Only necessary on systems where
+     * size_t is a 64-bit type.
+     */
+    if (SIZE_MAX > UINT32_MAX) {
+        H5E_BEGIN_TRY
+        {
+            tmp_id = H5Gcreate1(file_id, "/size_hint_too_large", SIZE_MAX);
+        }
+        H5E_END_TRY;
+        VERIFY(tmp_id, FAIL, "H5Gcreate1");
+    }
+
+    /* Make sure the largest size_hint value works */
+    H5E_BEGIN_TRY
+    {
+        tmp_id = H5Gcreate1(file_id, "/largest_size_hint", UINT32_MAX);
+    }
+    H5E_END_TRY;
+    CHECK(tmp_id, FAIL, "H5Gcreate1");
+    status = H5Gclose(tmp_id);
+    CHECK(status, FAIL, "H5Gclose");
+
     tmp_id = H5Gcreate1(file_id, "/A/grp", (size_t)0);
     CHECK(tmp_id, FAIL, "H5Gcreate1");
-
     status = H5Gclose(tmp_id);
     CHECK(status, FAIL, "H5Gclose");
 
@@ -4103,7 +4125,6 @@ test_misc23(void)
 
     tmp_id = H5Dcreate1(file_id, "/A/dset", type_id, space_id, create_id);
     CHECK(tmp_id, FAIL, "H5Dcreate1");
-
     status = H5Dclose(tmp_id);
     CHECK(status, FAIL, "H5Dclose");
 #endif /* H5_NO_DEPRECATED_SYMBOLS */