From f9c16de8a7f50afd6f5ef14df68074552388dae6 Mon Sep 17 00:00:00 2001 From: Dana Robinson <43805+derobins@users.noreply.github.com> Date: Fri, 14 Apr 2023 15:17:24 -0700 Subject: 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 --- release_docs/RELEASE.txt | 15 ++++++++++++++- src/H5Ocache.c | 5 +++-- src/H5Ocont.c | 23 +++++++++++++---------- 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() */ -- cgit v0.12