Fixes a bad memory read and unfreed memory in fsinfo code (#893) (#1012)

* Fixes a bad memory read and unfreed memory in fsinfo code

The segfaul from CVE-2020-10810 was fixed some time ago, but the
illegal memory read and unfreed memory were not.

This fix tracks some buffer sizes and errors out gracefully on errors,
ensuring buffers are cleaned up and avoiding the H5FL infinite loop +
abort on library close.

* Committing clang-format changes

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

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

2 files changed, 22 insertions, 14 deletions

diff --git a/src/H5Ocache.c b/src/H5Ocache.c
index 62dc2f2..3aae0cb 100644
--- a/src/H5Ocache.c
+++ b/src/H5Ocache.c
@@ -78,8 +78,8 @@ static herr_t H5O__cache_chk_free_icr(void *thing);
 static herr_t H5O__prefix_deserialize(const uint8_t *image, H5O_cache_ud_t *udata);
 
 /* Chunk routines */
-static herr_t H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t len, const uint8_t *image,
-                                     H5O_common_cache_ud_t *udata, hbool_t *dirty);
+static herr_t H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t chunk_size, const uint8_t *image,
+                                     size_t len, H5O_common_cache_ud_t *udata, hbool_t *dirty);
 static herr_t H5O__chunk_serialize(const H5F_t *f, H5O_t *oh, unsigned chunkno);
 
 /* Misc. routines */
@@ -287,7 +287,7 @@ H5O__cache_verify_chksum(const void *_image, size_t len, void *_udata)
  *-------------------------------------------------------------------------
  */
 static void *
-H5O__cache_deserialize(const void *image, size_t H5_ATTR_NDEBUG_UNUSED len, void *_udata, hbool_t *dirty)
+H5O__cache_deserialize(const void *image, size_t len, void *_udata, hbool_t *dirty)
 {
     H5O_t *         oh        = NULL;                     /* Object header read in */
     H5O_cache_ud_t *udata     = (H5O_cache_ud_t *)_udata; /* User data for callback */
@@ -333,7 +333,7 @@ H5O__cache_deserialize(const void *image, size_t H5_ATTR_NDEBUG_UNUSED len, void
         oh->proxy = NULL;
 
     /* Parse the first chunk */
-    if (H5O__chunk_deserialize(oh, udata->common.addr, udata->chunk0_size, (const uint8_t *)image,
+    if (H5O__chunk_deserialize(oh, udata->common.addr, udata->chunk0_size, (const uint8_t *)image, len,
                                &(udata->common), dirty) < 0)
         HGOTO_ERROR(H5E_OHDR, H5E_CANTINIT, NULL, "can't deserialize first object header chunk")
 
@@ -736,7 +736,7 @@ H5O__cache_chk_verify_chksum(const void *_image, size_t len, void *_udata)
  *-------------------------------------------------------------------------
  */
 static void *
-H5O__cache_chk_deserialize(const void *image, size_t H5_ATTR_NDEBUG_UNUSED len, void *_udata, hbool_t *dirty)
+H5O__cache_chk_deserialize(const void *image, size_t len, void *_udata, hbool_t *dirty)
 {
     H5O_chunk_proxy_t * chk_proxy = NULL;                         /* Chunk proxy object */
     H5O_chk_cache_ud_t *udata     = (H5O_chk_cache_ud_t *)_udata; /* User data for callback */
@@ -763,7 +763,7 @@ H5O__cache_chk_deserialize(const void *image, size_t H5_ATTR_NDEBUG_UNUSED len,
         HDassert(udata->common.cont_msg_info);
 
         /* Parse the chunk */
-        if (H5O__chunk_deserialize(udata->oh, udata->common.addr, udata->size, (const uint8_t *)image,
+        if (H5O__chunk_deserialize(udata->oh, udata->common.addr, udata->size, (const uint8_t *)image, len,
                                    &(udata->common), dirty) < 0)
             HGOTO_ERROR(H5E_OHDR, H5E_CANTINIT, NULL, "can't deserialize object header chunk")
 
@@ -1275,7 +1275,7 @@ done:
  *-------------------------------------------------------------------------
  */
 static herr_t
-H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t len, const uint8_t *image,
+H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t chunk_size, const uint8_t *image, size_t len,
                        H5O_common_cache_ud_t *udata, hbool_t *dirty)
 {
     const uint8_t *chunk_image;          /* Pointer into buffer to decode */
@@ -1295,6 +1295,7 @@ H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t len, const uint8_t *image
     HDassert(oh);
     HDassert(H5F_addr_defined(addr));
     HDassert(image);
+    HDassert(len);
     HDassert(udata->f);
     HDassert(udata->cont_msg_info);
 
@@ -1315,14 +1316,16 @@ H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t len, const uint8_t *image
     oh->chunk[chunkno].addr = addr;
     if (chunkno == 0)
         /* First chunk's 'image' includes room for the object header prefix */
-        oh->chunk[0].size = len + (size_t)H5O_SIZEOF_HDR(oh);
+        oh->chunk[0].size = chunk_size + (size_t)H5O_SIZEOF_HDR(oh);
     else
-        oh->chunk[chunkno].size = len;
+        oh->chunk[chunkno].size = chunk_size;
     if (NULL == (oh->chunk[chunkno].image = H5FL_BLK_MALLOC(chunk_image, oh->chunk[chunkno].size)))
         HGOTO_ERROR(H5E_OHDR, H5E_CANTALLOC, FAIL, "memory allocation failed")
     oh->chunk[chunkno].chunk_proxy = NULL;
 
     /* Copy disk image into chunk's image */
+    if (len < oh->chunk[chunkno].size)
+        HGOTO_ERROR(H5E_OHDR, H5E_CANTCOPY, FAIL, "attempted to copy too many disk image bytes into buffer")
     H5MM_memcpy(oh->chunk[chunkno].image, image, oh->chunk[chunkno].size);
 
     /* Point into chunk image to decode */
diff --git a/src/H5Ofsinfo.c b/src/H5Ofsinfo.c
index b9dc4fe..0ab5abb 100644
--- a/src/H5Ofsinfo.c
+++ b/src/H5Ofsinfo.c
@@ -90,11 +90,12 @@ H5FL_DEFINE_STATIC(H5O_fsinfo_t);
  */
 static void *
 H5O__fsinfo_decode(H5F_t *f, H5O_t H5_ATTR_UNUSED *open_oh, unsigned H5_ATTR_UNUSED mesg_flags,
-                   unsigned H5_ATTR_UNUSED *ioflags, size_t H5_ATTR_UNUSED p_size, const uint8_t *p)
+                   unsigned H5_ATTR_UNUSED *ioflags, size_t p_size, const uint8_t *p)
 {
-    H5O_fsinfo_t * fsinfo = NULL;    /* File space info message */
-    H5F_mem_page_t ptype;            /* Memory type for iteration */
-    unsigned       vers;             /* message version */
+    H5O_fsinfo_t * fsinfo = NULL; /* File space info message */
+    H5F_mem_page_t ptype;         /* Memory type for iteration */
+    unsigned       vers;          /* message version */
+    const uint8_t *p_end     = p + p_size;
     void *         ret_value = NULL; /* Return value */
 
     FUNC_ENTER_STATIC
@@ -135,8 +136,12 @@ H5O__fsinfo_decode(H5F_t *f, H5O_t H5_ATTR_UNUSED *open_oh, unsigned H5_ATTR_UNU
                 fsinfo->threshold = threshold;
                 if (HADDR_UNDEF == (fsinfo->eoa_pre_fsm_fsalloc = H5F_get_eoa(f, H5FD_MEM_DEFAULT)))
                     HGOTO_ERROR(H5E_FILE, H5E_CANTGET, NULL, "unable to get file size")
-                for (type = H5FD_MEM_SUPER; type < H5FD_MEM_NTYPES; type++)
+                for (type = H5FD_MEM_SUPER; type < H5FD_MEM_NTYPES; type++) {
+                    if (p + H5_SIZEOF_HADDR_T > p_end)
+                        HGOTO_ERROR(H5E_FILE, H5E_CANTDECODE, NULL,
+                                    "ran off end of input buffer while decoding")
                     H5F_addr_decode(f, &p, &(fsinfo->fs_addr[type - 1]));
+                }
                 break;
 
             case H5F_FILE_SPACE_ALL: