Fix memory leaks when processing OH cont messages (#2723)

Malformed object header continuation messages can result in a too-small
buffer being passed to the decode function, which could lead to reading
past the end of the buffer. Additionally, errors in processing these
malformed messages can lead to allocated memory not being cleaned up.

This fix adds bounds checking and cleanup code to the object header
continuation message processing.

Fixes #2604

3 files changed, 30 insertions, 13 deletions

diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt
index 63f5a36..7e53322 100644
--- a/release_docs/RELEASE.txt
+++ b/release_docs/RELEASE.txt
@@ -151,6 +151,18 @@ Bug Fixes since HDF5-1.14.0 release
 ===================================
     Library
     -------
+    - Fixed memory leaks when processing malformed object header continuation messages
+
+      Malformed object header continuation messages can result in a too-small
+      buffer being passed to the decode function, which could lead to reading
+      past the end of the buffer. Additionally, errors in processing these
+      malformed messages can lead to allocated memory not being cleaned up.
+
+      This fix adds bounds checking and cleanup code to the object header
+      continuation message processing.
+
+      (DER - 2023/04/13 GH-2604)
+
     - Fixed memory leaks, aborts, and overflows in H5O EFL decode
 
       The external file list code could call assert(), read past buffer
@@ -196,7 +208,8 @@ Bug Fixes since HDF5-1.14.0 release
 
     - Fixed potential heap buffer overrun in group info header decoding from malformed file
 
-      H5O__ginfo_decode could sometimes read past allocated memory when parsing a group info message from the header of a malformed file.
+      H5O__ginfo_decode could sometimes read past allocated memory when parsing a
+      group info message from the header of a malformed file.
   
       It now checks buffer size before each read to properly throw an error in these cases.
   
diff --git a/src/H5Ocache.c b/src/H5Ocache.c
index 0851493..42d8f35 100644
--- a/src/H5Ocache.c
+++ b/src/H5Ocache.c
@@ -1510,8 +1510,9 @@ H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t chunk_size, const uint8_t
                 H5O_cont_t *cont;
 
                 /* Decode continuation message */
-                cont = (H5O_cont_t *)(H5O_MSG_CONT->decode)(udata->f, NULL, 0, &ioflags, mesg->raw_size,
-                                                            mesg->raw);
+                if (NULL == (cont = (H5O_cont_t *)(H5O_MSG_CONT->decode)(udata->f, NULL, 0, &ioflags,
+                                                                         mesg->raw_size, mesg->raw)))
+                    HGOTO_ERROR(H5E_OHDR, H5E_BADMESG, FAIL, "bad continuation message found")
                 H5_CHECKED_ASSIGN(cont->chunkno, unsigned, udata->cont_msg_info->nmsgs + 1,
                                   size_t); /* the next continuation message/chunk */
 
diff --git a/src/H5Ocont.c b/src/H5Ocont.c
index 8919ced..bbf233d 100644
--- a/src/H5Ocont.c
+++ b/src/H5Ocont.c
@@ -74,40 +74,43 @@ H5FL_DEFINE(H5O_cont_t);
  * Purpose:     Decode the raw header continuation message.
  *
  * Return:      Success:        Ptr to the new native message
- *
  *              Failure:        NULL
- *
- * Programmer:  Robb Matzke
- *              Aug  6 1997
- *
  *-------------------------------------------------------------------------
  */
 static void *
 H5O__cont_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_cont_t *cont      = NULL;
-    void       *ret_value = NULL; /* Return value */
+    H5O_cont_t    *cont      = NULL;
+    const uint8_t *p_end     = p + p_size - 1;
+    void          *ret_value = NULL;
 
     FUNC_ENTER_PACKAGE
 
-    /* check args */
     HDassert(f);
     HDassert(p);
 
     /* Allocate space for the message */
     if (NULL == (cont = H5FL_MALLOC(H5O_cont_t)))
-        HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed");
+        HGOTO_ERROR(H5E_OHDR, H5E_NOSPACE, NULL, "memory allocation failed");
 
     /* Decode */
+    if (H5_IS_BUFFER_OVERFLOW(p, H5F_sizeof_addr(f), p_end))
+        HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding");
     H5F_addr_decode(f, &p, &(cont->addr));
+
+    if (H5_IS_BUFFER_OVERFLOW(p, H5F_sizeof_size(f), p_end))
+        HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding");
     H5F_DECODE_LENGTH(f, p, cont->size);
+
     cont->chunkno = 0;
 
     /* Set return value */
     ret_value = cont;
 
 done:
+    if (NULL == ret_value && NULL != cont)
+        H5FL_FREE(H5O_cont_t, cont);
     FUNC_LEAVE_NOAPI(ret_value)
 } /* end H5O__cont_decode() */