From d5f05f38ad985f9437eba782877ea4a16e463496 Mon Sep 17 00:00:00 2001 From: Binh-Minh Date: Sat, 27 Sep 2025 01:36:23 -0400 Subject: [PATCH 1/4] Harden H5C__decode_cache_image_header() by adding buffer overflow checks --- src/H5Cimage.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/H5Cimage.c b/src/H5Cimage.c index ded37100bb2..6bea76bb7ef 100644 --- a/src/H5Cimage.c +++ b/src/H5Cimage.c @@ -1277,6 +1277,7 @@ H5C__decode_cache_image_header(const H5F_t *f, H5C_t *cache_ptr, const uint8_t * size_t actual_header_len; size_t expected_header_len; const uint8_t *p; + const uint8_t *p_end = p + buf_size - 1; /* End of the p buffer */ herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_PACKAGE @@ -1299,11 +1300,15 @@ H5C__decode_cache_image_header(const H5F_t *f, H5C_t *cache_ptr, const uint8_t * p += H5C__MDCI_BLOCK_SIGNATURE_LEN; /* Check version */ + if (H5_IS_BUFFER_OVERFLOW(p, 1, p_end)) + HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); version = *p++; if (version != (uint8_t)H5C__MDCI_BLOCK_VERSION_0) HGOTO_ERROR(H5E_CACHE, H5E_BADVALUE, FAIL, "Bad metadata cache image version"); /* Decode flags */ + if (H5_IS_BUFFER_OVERFLOW(p, 1, p_end)) + HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); flags = *p++; if (flags & H5C__MDCI_HEADER_HAVE_RESIZE_STATUS) have_resize_status = true; @@ -1311,6 +1316,8 @@ H5C__decode_cache_image_header(const H5F_t *f, H5C_t *cache_ptr, const uint8_t * HGOTO_ERROR(H5E_CACHE, H5E_BADVALUE, FAIL, "MDC resize status not yet supported"); /* Read image data length */ + 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, cache_ptr->image_data_len); /* For now -- will become <= eventually */ @@ -1318,6 +1325,8 @@ H5C__decode_cache_image_header(const H5F_t *f, H5C_t *cache_ptr, const uint8_t * HGOTO_ERROR(H5E_CACHE, H5E_BADVALUE, FAIL, "Bad metadata cache image data length"); /* Read num entries */ + if (H5_IS_BUFFER_OVERFLOW(p, 4, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); UINT32DECODE(p, cache_ptr->num_entries_in_image); if (cache_ptr->num_entries_in_image == 0) HGOTO_ERROR(H5E_CACHE, H5E_BADVALUE, FAIL, "Bad metadata cache entry count"); @@ -2393,10 +2402,9 @@ H5C__reconstruct_cache_contents(H5F_t *f, H5C_t *cache_ptr) /* Decode metadata cache image header */ p = (uint8_t *)cache_ptr->image_buffer; - image_len = cache_ptr->image_len; - if (H5C__decode_cache_image_header(f, cache_ptr, &p, image_len + 1) < 0) + if (H5C__decode_cache_image_header(f, cache_ptr, &p, cache_ptr->image_len + 1) < 0) HGOTO_ERROR(H5E_CACHE, H5E_CANTDECODE, FAIL, "cache image header decode failed"); - assert((size_t)(p - (uint8_t *)cache_ptr->image_buffer) < image_len); + assert((size_t)(p - (uint8_t *)cache_ptr->image_buffer) < cache_ptr->image_len); /* The image_data_len and # of entries should be defined now */ assert(cache_ptr->image_data_len > 0); @@ -2404,6 +2412,7 @@ H5C__reconstruct_cache_contents(H5F_t *f, H5C_t *cache_ptr) assert(cache_ptr->num_entries_in_image > 0); /* Reconstruct entries in image */ + image_len = cache_ptr->image_len; for (u = 0; u < cache_ptr->num_entries_in_image; u++) { /* Create the prefetched entry described by the ith * entry in cache_ptr->image_entrise. From df86e9da4995f987ea47a40091951c5237674e7d Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 27 Sep 2025 05:38:40 +0000 Subject: [PATCH 2/4] Committing clang-format changes --- src/H5Cimage.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/H5Cimage.c b/src/H5Cimage.c index 6bea76bb7ef..54d7c4aae16 100644 --- a/src/H5Cimage.c +++ b/src/H5Cimage.c @@ -1278,7 +1278,7 @@ H5C__decode_cache_image_header(const H5F_t *f, H5C_t *cache_ptr, const uint8_t * size_t expected_header_len; const uint8_t *p; const uint8_t *p_end = p + buf_size - 1; /* End of the p buffer */ - herr_t ret_value = SUCCEED; /* Return value */ + herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_PACKAGE @@ -2401,7 +2401,7 @@ H5C__reconstruct_cache_contents(H5F_t *f, H5C_t *cache_ptr) assert(cache_ptr->image_len > 0); /* Decode metadata cache image header */ - p = (uint8_t *)cache_ptr->image_buffer; + p = (uint8_t *)cache_ptr->image_buffer; if (H5C__decode_cache_image_header(f, cache_ptr, &p, cache_ptr->image_len + 1) < 0) HGOTO_ERROR(H5E_CACHE, H5E_CANTDECODE, FAIL, "cache image header decode failed"); assert((size_t)(p - (uint8_t *)cache_ptr->image_buffer) < cache_ptr->image_len); From 9e9f28db4a9f70963d8b9ce568be98fcaf2cee33 Mon Sep 17 00:00:00 2001 From: Binh-Minh Date: Sun, 28 Sep 2025 17:29:27 -0400 Subject: [PATCH 3/4] Fix typos --- src/H5Cimage.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/H5Cimage.c b/src/H5Cimage.c index 54d7c4aae16..5664f441006 100644 --- a/src/H5Cimage.c +++ b/src/H5Cimage.c @@ -1277,8 +1277,8 @@ H5C__decode_cache_image_header(const H5F_t *f, H5C_t *cache_ptr, const uint8_t * size_t actual_header_len; size_t expected_header_len; const uint8_t *p; - const uint8_t *p_end = p + buf_size - 1; /* End of the p buffer */ - herr_t ret_value = SUCCEED; /* Return value */ + const uint8_t *p_end = *buf + buf_size - 1; /* End of the p buffer */ + herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_PACKAGE @@ -1301,14 +1301,14 @@ H5C__decode_cache_image_header(const H5F_t *f, H5C_t *cache_ptr, const uint8_t * /* Check version */ if (H5_IS_BUFFER_OVERFLOW(p, 1, p_end)) - HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); + HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); version = *p++; if (version != (uint8_t)H5C__MDCI_BLOCK_VERSION_0) HGOTO_ERROR(H5E_CACHE, H5E_BADVALUE, FAIL, "Bad metadata cache image version"); /* Decode flags */ if (H5_IS_BUFFER_OVERFLOW(p, 1, p_end)) - HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); + HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); flags = *p++; if (flags & H5C__MDCI_HEADER_HAVE_RESIZE_STATUS) have_resize_status = true; @@ -1317,7 +1317,7 @@ H5C__decode_cache_image_header(const H5F_t *f, H5C_t *cache_ptr, const uint8_t * /* Read image data length */ 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"); + HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); H5F_DECODE_LENGTH(f, p, cache_ptr->image_data_len); /* For now -- will become <= eventually */ @@ -1326,7 +1326,7 @@ H5C__decode_cache_image_header(const H5F_t *f, H5C_t *cache_ptr, const uint8_t * /* Read num entries */ if (H5_IS_BUFFER_OVERFLOW(p, 4, p_end)) - HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); + HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); UINT32DECODE(p, cache_ptr->num_entries_in_image); if (cache_ptr->num_entries_in_image == 0) HGOTO_ERROR(H5E_CACHE, H5E_BADVALUE, FAIL, "Bad metadata cache entry count"); From 17b13b0c748fa38a6fd412891d0718bd6756cfd2 Mon Sep 17 00:00:00 2001 From: Binh-Minh Date: Fri, 10 Oct 2025 01:39:22 -0400 Subject: [PATCH 4/4] Code improvement --- src/H5Cimage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/H5Cimage.c b/src/H5Cimage.c index 5664f441006..85077ec6d0c 100644 --- a/src/H5Cimage.c +++ b/src/H5Cimage.c @@ -1291,7 +1291,7 @@ H5C__decode_cache_image_header(const H5F_t *f, H5C_t *cache_ptr, const uint8_t * p = *buf; /* Ensure buffer has enough data for signature comparison */ - if (H5_IS_BUFFER_OVERFLOW(p, H5C__MDCI_BLOCK_SIGNATURE_LEN, *buf + buf_size - 1)) + if (H5_IS_BUFFER_OVERFLOW(p, H5C__MDCI_BLOCK_SIGNATURE_LEN, p_end)) HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, FAIL, "Insufficient buffer size for signature"); /* Check signature */