Minor fixes after merge of file format security fixes (#4263)

* Update H5_IS_BUFFER_OVERFLOW to account for 'size' of 0 * Invert a few checks to avoid function call
HDFGroup · Mar 28, 2024 · 03b6575 · 03b6575
1 parent 372381c
commit 03b6575
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 12 deletions.
diff --git a/src/H5Dint.c b/src/H5Dint.c
@@ -947,8 +947,8 @@ H5D__update_oh_info(H5F_t *file, H5D_t *dset, hid_t dapl_id)
         HGOTO_ERROR(H5E_DATASET, H5E_CANTPIN, FAIL, "unable to pin dataset object header");
 
     /* Check for creating dataset with unusual datatype */
-    if (H5T_is_numeric_with_unusual_unused_bits(type) &&
-        !(H5O_has_chksum(oh) || (H5F_RFIC_FLAGS(file) & H5F_RFIC_UNUSUAL_NUM_UNUSED_NUMERIC_BITS)))
+    if (!(H5O_has_chksum(oh) || (H5F_RFIC_FLAGS(file) & H5F_RFIC_UNUSUAL_NUM_UNUSED_NUMERIC_BITS)) &&
+        H5T_is_numeric_with_unusual_unused_bits(type))
         HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL,
                     "creating dataset with unusual datatype, see documentation for "
                     "H5Pset_relax_file_integrity_checks for details.");

diff --git a/src/H5Oattribute.c b/src/H5Oattribute.c
@@ -212,8 +212,8 @@ H5O__attr_create(const H5O_loc_t *loc, H5A_t *attr)
         HGOTO_ERROR(H5E_ATTR, H5E_CANTPIN, FAIL, "unable to pin object header");
 
     /* Check for creating attribute with unusual datatype */
-    if (H5T_is_numeric_with_unusual_unused_bits(attr->shared->dt) &&
-        !(H5O_has_chksum(oh) || (H5F_RFIC_FLAGS(loc->file) & H5F_RFIC_UNUSUAL_NUM_UNUSED_NUMERIC_BITS)))
+    if (!(H5O_has_chksum(oh) || (H5F_RFIC_FLAGS(loc->file) & H5F_RFIC_UNUSUAL_NUM_UNUSED_NUMERIC_BITS)) &&
+        H5T_is_numeric_with_unusual_unused_bits(attr->shared->dt))
         HGOTO_ERROR(H5E_ATTR, H5E_CANTINIT, FAIL,
                     "creating attribute with unusual datatype, see documentation for "
                     "H5Pset_relax_file_integrity_checks for details.");

diff --git a/src/H5Tcommit.c b/src/H5Tcommit.c
@@ -490,8 +490,8 @@ H5T__commit(H5F_t *file, H5T_t *type, hid_t tcpl_id)
         HGOTO_ERROR(H5E_ATTR, H5E_CANTPIN, FAIL, "unable to pin object header");
 
     /* Check for creating committed datatype with unusual datatype */
-    if (H5T_is_numeric_with_unusual_unused_bits(type) &&
-        !(H5O_has_chksum(oh) || (H5F_RFIC_FLAGS(file) & H5F_RFIC_UNUSUAL_NUM_UNUSED_NUMERIC_BITS)))
+    if (!(H5O_has_chksum(oh) || (H5F_RFIC_FLAGS(file) & H5F_RFIC_UNUSUAL_NUM_UNUSED_NUMERIC_BITS)) &&
+        H5T_is_numeric_with_unusual_unused_bits(type))
         HGOTO_ERROR(H5E_DATATYPE, H5E_CANTINIT, FAIL,
                     "creating committed datatype with unusual datatype, see documentation for "
                     "H5Pset_relax_file_integrity_checks for details.");

diff --git a/src/H5private.h b/src/H5private.h
@@ -352,12 +352,21 @@
  * For the time being, these can be suppressed with
  * H5_GCC_CLANG_DIAG_OFF("type-limits")/H5_GCC_CLANG_DIAG_ON("type-limits")
  */
+/* clang-format off */
 #define H5_IS_BUFFER_OVERFLOW(ptr, size, buffer_end)                                                         \
-    (((ptr) > (buffer_end)) || /* Bad precondition */                                                        \
-     (((size_t)(size) <= PTRDIFF_MAX) &&                                                                     \
-      ((ptrdiff_t)(size) < 0)) || /* Account for (likely unintentional) negative 'size' */                   \
-     ((size_t)(size) >                                                                                       \
-      (size_t)((((const uint8_t *)buffer_end) - ((const uint8_t *)ptr)) + 1))) /* Typical overflow */
+    (                                                                                                        \
+      /* Trivial case */                                                                                     \
+      ((size) != 0) &&                                                                                       \
+      (                                                                                                      \
+        /* Bad precondition */                                                                               \
+        ((ptr) > (buffer_end)) ||                                                                            \
+        /* Account for (likely unintentional) negative 'size' */                                             \
+        (((size_t)(size) <= PTRDIFF_MAX) && ((ptrdiff_t)(size) < 0)) ||                                      \
+        /* Typical overflow */                                                                               \
+        ((size_t)(size) > (size_t)((((const uint8_t *)buffer_end) - ((const uint8_t *)ptr)) + 1))            \
+      )                                                                                                      \
+    )
+/* clang-format on */
 
 /* Variant of H5_IS_BUFFER_OVERFLOW, used with functions such as H5Tdecode()
  * that don't take a size parameter, where we need to skip the bounds checks.
@@ -366,7 +375,7 @@
  * the entire library.
  */
 #define H5_IS_KNOWN_BUFFER_OVERFLOW(skip, ptr, size, buffer_end)                                             \
-    (skip ? false : ((ptr) + (size)-1) > (buffer_end))
+    (skip ? false : H5_IS_BUFFER_OVERFLOW(ptr, size, buffer_end))
 
 /*
  * HDF Boolean type.