Fix out of bounds in `hdf5/src/H5Fint.c:2859` (#2691)

4 files changed, 84 insertions, 7 deletions

diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt
index 2dcb057..63f5a36 100644
--- a/release_docs/RELEASE.txt
+++ b/release_docs/RELEASE.txt
@@ -291,6 +291,18 @@ Bug Fixes since HDF5-1.14.0 release
 
       (JTH - 2023/02/16, GH #2433)
 
+    - Fixed buffer overflow error in image decoding function.
+
+      The error occurred in the function for decoding address from the specified
+      buffer, which is called many times from the function responsible for image
+      decoding. The length of the buffer is known in the image decoding function,
+      but no checks are produced, so the buffer overflow can occur in many places,
+      including callee functions for address decoding. 
+
+      The error was fixed by inserting corresponding checks for buffer overflow.
+
+      (KE - 2023/02/07 GH #2432)
+
 
     Java Library
     ------------
diff --git a/src/H5Fsuper_cache.c b/src/H5Fsuper_cache.c
index 5d9b628..467e287 100644
--- a/src/H5Fsuper_cache.c
+++ b/src/H5Fsuper_cache.c
@@ -433,6 +433,8 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS
     if (H5F__superblock_prefix_decode(sblock, &image, udata, FALSE) < 0)
         HGOTO_ERROR(H5E_FILE, H5E_CANTDECODE, NULL, "can't decode file superblock prefix")
 
+    const uint8_t *image_end = image + len - 1;
+
     /* Check for older version of superblock format */
     if (sblock->super_vers < HDF5_SUPERBLOCK_VERSION_2) {
         uint32_t status_flags;  /* File status flags	   */
@@ -440,10 +442,18 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS
         unsigned snode_btree_k; /* B-tree symbol table internal node 'K' value */
         unsigned chunk_btree_k; /* B-tree chunk internal node 'K' value */
 
+        /* Check whether the image pointer is out of bounds */
+        if (H5_IS_BUFFER_OVERFLOW(image, 1, image_end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
+
         /* Freespace version (hard-wired) */
         if (HDF5_FREESPACE_VERSION != *image++)
             HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad free space version number")
 
+        /* Check whether the image pointer is out of bounds */
+        if (H5_IS_BUFFER_OVERFLOW(image, 1, image_end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
+
         /* Root group version number (hard-wired) */
         if (HDF5_OBJECTDIR_VERSION != *image++)
             HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad object directory version number")
@@ -451,6 +461,10 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS
         /* Skip over reserved byte */
         image++;
 
+        /* Check whether the image pointer is out of bounds */
+        if (H5_IS_BUFFER_OVERFLOW(image, 1, image_end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
+
         /* Shared header version number (hard-wired) */
         if (HDF5_SHAREDHEADER_VERSION != *image++)
             HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad shared-header format version number")
@@ -466,12 +480,20 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS
         /* Skip over reserved byte */
         image++;
 
+        /* Check whether the image pointer is out of bounds */
+        if (H5_IS_BUFFER_OVERFLOW(image, sizeof(uint16_t), image_end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
+
         /* Various B-tree sizes */
         UINT16DECODE(image, sym_leaf_k);
         if (sym_leaf_k == 0)
             HGOTO_ERROR(H5E_FILE, H5E_BADRANGE, NULL, "bad symbol table leaf node 1/2 rank")
         udata->sym_leaf_k = sym_leaf_k; /* Keep a local copy also */
 
+        /* Check whether the image pointer is out of bounds */
+        if (H5_IS_BUFFER_OVERFLOW(image, sizeof(uint16_t), image_end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
+
         /* Need 'get' call to set other array values */
         UINT16DECODE(image, snode_btree_k);
         if (snode_btree_k == 0)
@@ -483,6 +505,10 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS
          * for the indexed storage B-tree internal 'K' value later.
          */
 
+        /* Check whether the image pointer is out of bounds */
+        if (H5_IS_BUFFER_OVERFLOW(image, sizeof(uint32_t), image_end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
+
         /* File status flags (not really used yet) */
         UINT32DECODE(image, status_flags);
         HDassert(status_flags <= 255);
@@ -495,16 +521,29 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS
          * storage B-tree internal 'K' value
          */
         if (sblock->super_vers > HDF5_SUPERBLOCK_VERSION_DEF) {
+            /* Check whether the image pointer is out of bounds */
+            if (H5_IS_BUFFER_OVERFLOW(image, sizeof(uint16_t), image_end))
+                HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
+
             UINT16DECODE(image, chunk_btree_k);
 
             /* Reserved bytes are present only in version 1 */
-            if (sblock->super_vers == HDF5_SUPERBLOCK_VERSION_1)
+            if (sblock->super_vers == HDF5_SUPERBLOCK_VERSION_1) {
                 image += 2; /* reserved */
-        }                   /* end if */
+
+                /* Check whether the image pointer is out of bounds */
+                if (H5_IS_BUFFER_OVERFLOW(image, 1, image_end))
+                    HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
+            }
+        } /* end if */
         else
             chunk_btree_k = HDF5_BTREE_CHUNK_IK_DEF;
         udata->btree_k[H5B_CHUNK_ID] = chunk_btree_k;
 
+        /* Check whether the image pointer will be out of bounds */
+        if (H5_IS_BUFFER_OVERFLOW(image, H5F_SIZEOF_ADDR(udata->f) * 4, image_end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
+
         /* Remainder of "variable-sized" portion of superblock */
         H5F_addr_decode(udata->f, (const uint8_t **)&image, &sblock->base_addr /*out*/);
         H5F_addr_decode(udata->f, (const uint8_t **)&image, &sblock->ext_addr /*out*/);
@@ -518,7 +557,7 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS
                         "can't allocate space for root group symbol table entry")
 
         /* decode the root group symbol table entry */
-        if (H5G_ent_decode(udata->f, (const uint8_t **)&image, sblock->root_ent) < 0)
+        if (H5G_ent_decode(udata->f, (const uint8_t **)&image, sblock->root_ent, image_end) < 0)
             HGOTO_ERROR(H5E_FILE, H5E_CANTDECODE, NULL, "can't decode root group symbol table entry")
 
         /* Set the root group address to the correct value */
@@ -544,16 +583,23 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS
         /* Skip over size of file addresses (already decoded) */
         image++;
         udata->f->shared->sizeof_addr = sblock->sizeof_addr; /* Keep a local copy also */
-
         /* Skip over size of file sizes (already decoded) */
         image++;
         udata->f->shared->sizeof_size = sblock->sizeof_size; /* Keep a local copy also */
 
+        /* Check whether the image pointer is out of bounds */
+        if (H5_IS_BUFFER_OVERFLOW(image, 1, image_end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
+
         /* File status flags (not really used yet) */
         sblock->status_flags = *image++;
         if (sblock->status_flags & ~H5F_SUPER_ALL_FLAGS)
             HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad flag value for superblock")
 
+        /* Check whether the image pointer will be out of bounds */
+        if (H5_IS_BUFFER_OVERFLOW(image, H5F_SIZEOF_ADDR(udata->f) * 4, image_end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
+
         /* Base, superblock extension, end of file & root group object header addresses */
         H5F_addr_decode(udata->f, (const uint8_t **)&image, &sblock->base_addr /*out*/);
         H5F_addr_decode(udata->f, (const uint8_t **)&image, &sblock->ext_addr /*out*/);
@@ -562,6 +608,10 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS
 
         /* checksum verification already done in verify_chksum cb */
 
+        /* Check whether the image pointer will be out of bounds */
+        if (H5_IS_BUFFER_OVERFLOW(image, sizeof(uint32_t), image_end))
+            HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds")
+
         /* Decode checksum */
         UINT32DECODE(image, read_chksum);
 
diff --git a/src/H5Gent.c b/src/H5Gent.c
index f58ef5c..096e13e 100644
--- a/src/H5Gent.c
+++ b/src/H5Gent.c
@@ -93,7 +93,7 @@ H5G__ent_decode_vec(const H5F_t *f, const uint8_t **pp, const uint8_t *p_end, H5
     for (u = 0; u < n; u++) {
         if (*pp > p_end)
             HGOTO_ERROR(H5E_SYM, H5E_CANTDECODE, FAIL, "ran off the end of the image buffer")
-        if (H5G_ent_decode(f, pp, ent + u) < 0)
+        if (H5G_ent_decode(f, pp, ent + u, p_end) < 0)
             HGOTO_ERROR(H5E_SYM, H5E_CANTDECODE, FAIL, "can't decode")
     }
 
@@ -117,7 +117,7 @@ done:
  *-------------------------------------------------------------------------
  */
 herr_t
-H5G_ent_decode(const H5F_t *f, const uint8_t **pp, H5G_entry_t *ent)
+H5G_ent_decode(const H5F_t *f, const uint8_t **pp, H5G_entry_t *ent, const uint8_t *p_end)
 {
     const uint8_t *p_ret = *pp;
     uint32_t       tmp;
@@ -130,11 +130,22 @@ H5G_ent_decode(const H5F_t *f, const uint8_t **pp, H5G_entry_t *ent)
     HDassert(pp);
     HDassert(ent);
 
+    if (H5_IS_BUFFER_OVERFLOW(*pp, ent->name_off, p_end))
+        HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, FAIL, "image pointer is out of bounds")
+
     /* decode header */
     H5F_DECODE_LENGTH(f, *pp, ent->name_off);
+
+    if (H5_IS_BUFFER_OVERFLOW(*pp, H5F_SIZEOF_ADDR(f) + sizeof(uint32_t), p_end))
+        HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, FAIL, "image pointer is out of bounds")
+
     H5F_addr_decode(f, pp, &(ent->header));
     UINT32DECODE(*pp, tmp);
     *pp += 4; /*reserved*/
+
+    if (H5_IS_BUFFER_OVERFLOW(*pp, 1, p_end))
+        HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, FAIL, "image pointer is out of bounds")
+
     ent->type = (H5G_cache_type_t)tmp;
 
     /* decode scratch-pad */
@@ -144,11 +155,15 @@ H5G_ent_decode(const H5F_t *f, const uint8_t **pp, H5G_entry_t *ent)
 
         case H5G_CACHED_STAB:
             HDassert(2 * H5F_SIZEOF_ADDR(f) <= H5G_SIZEOF_SCRATCH);
+            if (H5_IS_BUFFER_OVERFLOW(*pp, H5F_SIZEOF_ADDR(f) * 2, p_end))
+                HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, FAIL, "image pointer is out of bounds")
             H5F_addr_decode(f, pp, &(ent->cache.stab.btree_addr));
             H5F_addr_decode(f, pp, &(ent->cache.stab.heap_addr));
             break;
 
         case H5G_CACHED_SLINK:
+            if (H5_IS_BUFFER_OVERFLOW(*pp, sizeof(uint32_t), p_end))
+                HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, FAIL, "image pointer is out of bounds")
             UINT32DECODE(*pp, ent->cache.slink.lval_offset);
             break;
 
diff --git a/src/H5Gprivate.h b/src/H5Gprivate.h
index 0042fb2..2819e4f 100644
--- a/src/H5Gprivate.h
+++ b/src/H5Gprivate.h
@@ -248,7 +248,7 @@ H5_DLL herr_t H5G_node_debug(H5F_t *f, haddr_t addr, FILE *stream, int indent, i
  * These functions operate on group object locations.
  */
 H5_DLL herr_t H5G_ent_encode(const H5F_t *f, uint8_t **pp, const H5G_entry_t *ent);
-H5_DLL herr_t H5G_ent_decode(const H5F_t *f, const uint8_t **pp, H5G_entry_t *ent);
+H5_DLL herr_t H5G_ent_decode(const H5F_t *f, const uint8_t **pp, H5G_entry_t *ent, const uint8_t *p_end);
 
 /*
  * These functions operate on group hierarchy names.