From 89e3e43aa0f64a3bbd253bef658846d9ff030bdd Mon Sep 17 00:00:00 2001 From: bmribler <39579120+bmribler@users.noreply.github.com> Date: Thu, 25 Sep 2025 22:17:14 -0400 Subject: [PATCH] Fixed CVE-2025-6269 (#5850) The GitHub issue #5579 included several security vulnerabilities in function H5C__reconstruct_cache_entry(). This PR addressed them by: - adding buffer size argument to the function - adding buffer overflow checks - adding input validations - releasing allocated resource on failure These changes addressed the crashes reported. However, there is a skiplist crash during the unwinding process that has to be investigated. CVE: CVE-2025-6269 CVE-2025-6270 CVE-2025-6516 Upstream-Status: Backport [https://github.com/HDFGroup/hdf5/commit/3914bb7f7ec7105d8bfbeb3aebd92e867cff5b70] (cherry picked from commit 3914bb7f7ec7105d8bfbeb3aebd92e867cff5b70) Signed-off-by: Ankur Tyagi Signed-off-by: Deepak Rathore --- src/H5Cimage.c | 84 ++++++++++++++++++++++++++++++++++++++------------ src/H5Ocont.c | 5 +-- 2 files changed, 68 insertions(+), 21 deletions(-) diff --git a/src/H5Cimage.c b/src/H5Cimage.c index 72dc52dafb..b97be228ed 100644 --- a/src/H5Cimage.c +++ b/src/H5Cimage.c @@ -132,7 +132,8 @@ static void H5C__prep_for_file_close__compute_fd_heights_real(H5C_cache_entry_ static herr_t H5C__prep_for_file_close__setup_image_entries_array(H5C_t *cache_ptr); static herr_t H5C__prep_for_file_close__scan_entries(const H5F_t *f, H5C_t *cache_ptr); static herr_t H5C__reconstruct_cache_contents(H5F_t *f, H5C_t *cache_ptr); -static H5C_cache_entry_t *H5C__reconstruct_cache_entry(const H5F_t *f, H5C_t *cache_ptr, const uint8_t **buf); +static H5C_cache_entry_t *H5C__reconstruct_cache_entry(const H5F_t *f, H5C_t *cache_ptr, hsize_t *buf_size, + const uint8_t **buf); static herr_t H5C__write_cache_image_superblock_msg(H5F_t *f, bool create); static herr_t H5C__read_cache_image(H5F_t *f, H5C_t *cache_ptr); static herr_t H5C__write_cache_image(H5F_t *f, const H5C_t *cache_ptr); @@ -2377,6 +2378,7 @@ H5C__reconstruct_cache_contents(H5F_t *f, H5C_t *cache_ptr) { H5C_cache_entry_t *pf_entry_ptr; /* Pointer to prefetched entry */ H5C_cache_entry_t *parent_ptr; /* Pointer to parent of prefetched entry */ + hsize_t image_len; /* Image length */ const uint8_t *p; /* Pointer into image buffer */ unsigned u, v; /* Local index variable */ herr_t ret_value = SUCCEED; /* Return value */ @@ -2392,10 +2394,11 @@ 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; - if (H5C__decode_cache_image_header(f, cache_ptr, &p, cache_ptr->image_len + 1) < 0) + 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) 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); + assert((size_t)(p - (uint8_t *)cache_ptr->image_buffer) < image_len); /* The image_data_len and # of entries should be defined now */ assert(cache_ptr->image_data_len > 0); @@ -2407,7 +2410,7 @@ H5C__reconstruct_cache_contents(H5F_t *f, H5C_t *cache_ptr) /* Create the prefetched entry described by the ith * entry in cache_ptr->image_entrise. */ - if (NULL == (pf_entry_ptr = H5C__reconstruct_cache_entry(f, cache_ptr, &p))) + if (NULL == (pf_entry_ptr = H5C__reconstruct_cache_entry(f, cache_ptr, &image_len, &p))) HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "reconstruction of cache entry failed"); /* Note that we make no checks on available cache space before @@ -2563,19 +2566,21 @@ done: *------------------------------------------------------------------------- */ static H5C_cache_entry_t * -H5C__reconstruct_cache_entry(const H5F_t *f, H5C_t *cache_ptr, const uint8_t **buf) +H5C__reconstruct_cache_entry(const H5F_t *f, H5C_t *cache_ptr, hsize_t *buf_size, const uint8_t **buf) { H5C_cache_entry_t *pf_entry_ptr = NULL; /* Reconstructed cache entry */ uint8_t flags = 0; bool is_dirty = false; + haddr_t eoa; + bool is_fd_parent = false; #ifndef NDEBUG /* only used in assertions */ - bool in_lru = false; - bool is_fd_parent = false; - bool is_fd_child = false; + bool in_lru = false; + bool is_fd_child = false; #endif - const uint8_t *p; bool file_is_rw; - H5C_cache_entry_t *ret_value = NULL; /* Return value */ + const uint8_t *p; + const uint8_t *p_end = *buf + *buf_size - 1; /* Pointer to last valid byte in buffer */ + H5C_cache_entry_t *ret_value = NULL; /* Return value */ FUNC_ENTER_PACKAGE @@ -2595,9 +2600,15 @@ H5C__reconstruct_cache_entry(const H5F_t *f, H5C_t *cache_ptr, const uint8_t **b p = *buf; /* Decode type id */ + if (H5_IS_BUFFER_OVERFLOW(p, 1, p_end)) + HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); pf_entry_ptr->prefetch_type_id = *p++; + if (pf_entry_ptr->prefetch_type_id < H5AC_BT_ID || pf_entry_ptr->prefetch_type_id >= H5AC_NTYPES) + HGOTO_ERROR(H5E_CACHE, H5E_BADVALUE, NULL, "type id is out of valid range"); /* 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_ENTRY_DIRTY_FLAG) is_dirty = true; @@ -2625,19 +2636,31 @@ H5C__reconstruct_cache_entry(const H5F_t *f, H5C_t *cache_ptr, const uint8_t **b pf_entry_ptr->is_dirty = (is_dirty && file_is_rw); /* Decode ring */ + if (H5_IS_BUFFER_OVERFLOW(p, 1, p_end)) + HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); pf_entry_ptr->ring = *p++; - assert(pf_entry_ptr->ring > (uint8_t)(H5C_RING_UNDEFINED)); - assert(pf_entry_ptr->ring < (uint8_t)(H5C_RING_NTYPES)); + if (pf_entry_ptr->ring >= (uint8_t)(H5C_RING_NTYPES)) + HGOTO_ERROR(H5E_CACHE, H5E_BADVALUE, NULL, "ring is out of valid range"); /* Decode age */ + if (H5_IS_BUFFER_OVERFLOW(p, 1, p_end)) + HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); pf_entry_ptr->age = *p++; + if (pf_entry_ptr->age > H5AC__CACHE_IMAGE__ENTRY_AGEOUT__MAX) + HGOTO_ERROR(H5E_CACHE, H5E_BADVALUE, NULL, "entry age is out of policy range"); /* Decode dependency child count */ + if (H5_IS_BUFFER_OVERFLOW(p, 2, p_end)) + HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); UINT16DECODE(p, pf_entry_ptr->fd_child_count); - assert((is_fd_parent && pf_entry_ptr->fd_child_count > 0) || - (!is_fd_parent && pf_entry_ptr->fd_child_count == 0)); + if (is_fd_parent && pf_entry_ptr->fd_child_count <= 0) + HGOTO_ERROR(H5E_CACHE, H5E_BADVALUE, NULL, "parent entry has no children"); + else if (!is_fd_parent && pf_entry_ptr->fd_child_count != 0) + HGOTO_ERROR(H5E_CACHE, H5E_BADVALUE, NULL, "non-parent entry has children"); /* Decode dirty dependency child count */ + if (H5_IS_BUFFER_OVERFLOW(p, 2, p_end)) + HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); UINT16DECODE(p, pf_entry_ptr->fd_dirty_child_count); if (!file_is_rw) pf_entry_ptr->fd_dirty_child_count = 0; @@ -2645,20 +2668,32 @@ H5C__reconstruct_cache_entry(const H5F_t *f, H5C_t *cache_ptr, const uint8_t **b HGOTO_ERROR(H5E_CACHE, H5E_BADVALUE, NULL, "invalid dirty flush dependency child count"); /* Decode dependency parent count */ + if (H5_IS_BUFFER_OVERFLOW(p, 2, p_end)) + HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); UINT16DECODE(p, pf_entry_ptr->fd_parent_count); assert((is_fd_child && pf_entry_ptr->fd_parent_count > 0) || (!is_fd_child && pf_entry_ptr->fd_parent_count == 0)); /* Decode index in LRU */ + if (H5_IS_BUFFER_OVERFLOW(p, 4, p_end)) + HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); INT32DECODE(p, pf_entry_ptr->lru_rank); assert((in_lru && pf_entry_ptr->lru_rank >= 0) || (!in_lru && pf_entry_ptr->lru_rank == -1)); /* Decode entry offset */ + if (H5_IS_BUFFER_OVERFLOW(p, H5F_SIZEOF_ADDR(f), p_end)) + HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); H5F_addr_decode(f, &p, &pf_entry_ptr->addr); - if (!H5_addr_defined(pf_entry_ptr->addr)) - HGOTO_ERROR(H5E_CACHE, H5E_BADVALUE, NULL, "invalid entry offset"); + + /* Validate address range */ + eoa = H5F_get_eoa(f, H5FD_MEM_DEFAULT); + if (!H5_addr_defined(pf_entry_ptr->addr) || H5_addr_overflow(pf_entry_ptr->addr, pf_entry_ptr->size) || + H5_addr_ge(pf_entry_ptr->addr + pf_entry_ptr->size, eoa)) + HGOTO_ERROR(H5E_CACHE, H5E_BADVALUE, NULL, "invalid entry address range"); /* Decode entry length */ + if (H5_IS_BUFFER_OVERFLOW(p, H5F_SIZEOF_SIZE(f), p_end)) + HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); H5F_DECODE_LENGTH(f, p, pf_entry_ptr->size); if (pf_entry_ptr->size == 0) HGOTO_ERROR(H5E_CACHE, H5E_BADVALUE, NULL, "invalid entry size"); @@ -2679,6 +2714,9 @@ H5C__reconstruct_cache_entry(const H5F_t *f, H5C_t *cache_ptr, const uint8_t **b "memory allocation failed for fd parent addrs buffer"); for (u = 0; u < pf_entry_ptr->fd_parent_count; u++) { + + if (H5_IS_BUFFER_OVERFLOW(p, H5F_SIZEOF_ADDR(f), p_end)) + HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); H5F_addr_decode(f, &p, &(pf_entry_ptr->fd_parent_addrs[u])); if (!H5_addr_defined(pf_entry_ptr->fd_parent_addrs[u])) HGOTO_ERROR(H5E_CACHE, H5E_BADVALUE, NULL, "invalid flush dependency parent offset"); @@ -2694,6 +2732,8 @@ H5C__reconstruct_cache_entry(const H5F_t *f, H5C_t *cache_ptr, const uint8_t **b #endif /* H5C_DO_MEMORY_SANITY_CHECKS */ /* Copy the entry image from the cache image block */ + if (H5_IS_BUFFER_OVERFLOW(p, pf_entry_ptr->size, p_end)) + HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); H5MM_memcpy(pf_entry_ptr->image_ptr, p, pf_entry_ptr->size); p += pf_entry_ptr->size; @@ -2708,14 +2748,20 @@ H5C__reconstruct_cache_entry(const H5F_t *f, H5C_t *cache_ptr, const uint8_t **b /* Sanity checks */ assert(pf_entry_ptr->size > 0 && pf_entry_ptr->size < H5C_MAX_ENTRY_SIZE); - /* Update buffer pointer */ + /* Update buffer pointer and buffer len */ + *buf_size -= (hsize_t)(p - *buf); *buf = p; ret_value = pf_entry_ptr; done: - if (NULL == ret_value && pf_entry_ptr) + if (NULL == ret_value && pf_entry_ptr) { + if (pf_entry_ptr->image_ptr) + H5MM_xfree(pf_entry_ptr->image_ptr); + if (pf_entry_ptr->fd_parent_count > 0 && pf_entry_ptr->fd_parent_addrs) + H5MM_xfree(pf_entry_ptr->fd_parent_addrs); pf_entry_ptr = H5FL_FREE(H5C_cache_entry_t, pf_entry_ptr); + } FUNC_LEAVE_NOAPI(ret_value) } /* H5C__reconstruct_cache_entry() */ diff --git a/src/H5Ocont.c b/src/H5Ocont.c index c03f4dd1e9..4b1840448a 100644 --- a/src/H5Ocont.c +++ b/src/H5Ocont.c @@ -93,6 +93,9 @@ H5O__cont_decode(H5F_t *f, H5O_t H5_ATTR_UNUSED *open_oh, unsigned H5_ATTR_UNUSE HGOTO_ERROR(H5E_OHDR, H5E_NOSPACE, NULL, "memory allocation failed"); /* Decode */ + + cont->chunkno = 0; + 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)); @@ -103,8 +106,6 @@ H5O__cont_decode(H5F_t *f, H5O_t H5_ATTR_UNUSED *open_oh, unsigned H5_ATTR_UNUSE if (cont->size == 0) HGOTO_ERROR(H5E_OHDR, H5E_BADVALUE, NULL, "invalid continuation chunk size (0)"); - cont->chunkno = 0; - /* Set return value */ ret_value = cont;