From f6d805d772ef314b7bb09d0d444f2ad5d657f5cc Mon Sep 17 00:00:00 2001 From: Jordan Henderson Date: Wed, 8 Jul 2026 14:48:48 -0500 Subject: [PATCH] Fix heap buffer overflow in H5VM_array_fill() Fixes a heap buffer overflow that occurs due to a decoded fill value message's size not matching the size of the fill value's datatype. This results in a buffer being allocated with an incorrect size that is later overflowed when reading the fill value for chunks that haven't been allocated yet. Similar logic existed for decoding 'old' format fill value messages, but was not added for decoding 'new' format fill value messages --- src/H5Dfill.c | 11 ++++++++--- src/H5Ofill.c | 32 +++++++++++++++++++++++++------- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/src/H5Dfill.c b/src/H5Dfill.c index 093039987b8..23599c28c1a 100644 --- a/src/H5Dfill.c +++ b/src/H5Dfill.c @@ -335,7 +335,7 @@ H5D__fill_init(H5D_fill_buf_info_t *fb_info, void *caller_fill_buf, H5MM_allocat if (total_nelmts > 0) fb_info->elmts_per_buf = MIN(total_nelmts, MAX(1, (max_buf_size / fb_info->max_elmt_size))); else - fb_info->elmts_per_buf = max_buf_size / fb_info->max_elmt_size; + fb_info->elmts_per_buf = MAX(1, (max_buf_size / fb_info->max_elmt_size)); assert(fb_info->elmts_per_buf > 0); /* Compute the buffer size to use */ @@ -388,13 +388,18 @@ H5D__fill_init(H5D_fill_buf_info_t *fb_info, void *caller_fill_buf, H5MM_allocat else { /* If fill value is not library default, use it to set the element size */ assert(fill->size >= 0); + + if (H5T_get_size(dset_type) != (size_t)fill->size) + HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, FAIL, + "fill value type size doesn't match file type size"); + fb_info->max_elmt_size = fb_info->file_elmt_size = fb_info->mem_elmt_size = (size_t)fill->size; /* Compute the number of elements that fit within a buffer to write */ if (total_nelmts > 0) fb_info->elmts_per_buf = MIN(total_nelmts, MAX(1, (max_buf_size / fb_info->max_elmt_size))); else - fb_info->elmts_per_buf = max_buf_size / fb_info->max_elmt_size; + fb_info->elmts_per_buf = MAX(1, (max_buf_size / fb_info->max_elmt_size)); assert(fb_info->elmts_per_buf > 0); /* Compute the buffer size to use */ @@ -433,7 +438,7 @@ H5D__fill_init(H5D_fill_buf_info_t *fb_info, void *caller_fill_buf, H5MM_allocat if (total_nelmts > 0) fb_info->elmts_per_buf = MIN(total_nelmts, MAX(1, (max_buf_size / fb_info->max_elmt_size))); else - fb_info->elmts_per_buf = max_buf_size / fb_info->max_elmt_size; + fb_info->elmts_per_buf = MAX(1, (max_buf_size / fb_info->max_elmt_size)); assert(fb_info->elmts_per_buf > 0); /* Compute the buffer size to use */ diff --git a/src/H5Ofill.c b/src/H5Ofill.c index 7f111bfbf66..27b9c3a8e18 100644 --- a/src/H5Ofill.c +++ b/src/H5Ofill.c @@ -186,13 +186,13 @@ H5FL_BLK_EXTERN(type_conv); *------------------------------------------------------------------------- */ static void * -H5O__fill_new_decode(H5F_t H5_ATTR_UNUSED *f, H5O_t H5_ATTR_UNUSED *open_oh, - unsigned H5_ATTR_UNUSED mesg_flags, unsigned H5_ATTR_UNUSED *ioflags, size_t p_size, - const uint8_t *p) +H5O__fill_new_decode(H5F_t H5_ATTR_UNUSED *f, H5O_t *open_oh, unsigned H5_ATTR_UNUSED mesg_flags, + unsigned H5_ATTR_UNUSED *ioflags, size_t p_size, const uint8_t *p) { H5O_fill_t *fill = NULL; const uint8_t *p_end = p + p_size - 1; /* End of the p buffer */ - void *ret_value = NULL; /* Return value */ + H5T_t *dt = NULL; + void *ret_value = NULL; /* Return value */ FUNC_ENTER_PACKAGE @@ -303,10 +303,28 @@ H5O__fill_new_decode(H5F_t H5_ATTR_UNUSED *f, H5O_t H5_ATTR_UNUSED *open_oh, fill->fill_defined = true; } + /* Verify fill value size against datatype size */ + if (fill->fill_defined && fill->size > 0) { + htri_t msg_exists = false; + + if ((msg_exists = H5O_msg_exists_oh(open_oh, H5O_DTYPE_ID)) < 0) + HGOTO_ERROR(H5E_OHDR, H5E_NOTFOUND, NULL, "unable to read object header"); + if (msg_exists) { + if (NULL == (dt = H5O_msg_read_oh(f, open_oh, H5O_DTYPE_ID, NULL))) + HGOTO_ERROR(H5E_OHDR, H5E_CANTGET, NULL, "can't read datatype message"); + + if ((size_t)fill->size != H5T_GET_SIZE(dt)) + HGOTO_ERROR(H5E_OHDR, H5E_BADVALUE, NULL, "inconsistent fill value size"); + } + } + /* Set return value */ ret_value = (void *)fill; done: + if (dt) + H5O_msg_free(H5O_DTYPE_ID, dt); + if (!ret_value && fill) { H5MM_xfree(fill->buf); fill = H5FL_FREE(H5O_fill_t, fill); @@ -362,13 +380,13 @@ H5O__fill_old_decode(H5F_t *f, H5O_t *open_oh, unsigned H5_ATTR_UNUSED mesg_flag /* Get the datatype message */ if ((exists = H5O_msg_exists_oh(open_oh, H5O_DTYPE_ID)) < 0) - HGOTO_ERROR(H5E_SYM, H5E_NOTFOUND, NULL, "unable to read object header"); + HGOTO_ERROR(H5E_OHDR, H5E_NOTFOUND, NULL, "unable to read object header"); if (exists) { if (NULL == (dt = (H5T_t *)H5O_msg_read_oh(f, open_oh, H5O_DTYPE_ID, NULL))) - HGOTO_ERROR(H5E_SYM, H5E_CANTGET, NULL, "can't read DTYPE message"); + HGOTO_ERROR(H5E_OHDR, H5E_CANTGET, NULL, "can't read DTYPE message"); /* Verify size */ if (fill->size != (ssize_t)H5T_GET_SIZE(dt)) - HGOTO_ERROR(H5E_SYM, H5E_CANTGET, NULL, "inconsistent fill value size"); + HGOTO_ERROR(H5E_OHDR, H5E_BADVALUE, NULL, "inconsistent fill value size"); } if (NULL == (fill->buf = H5MM_malloc((size_t)fill->size)))