From 81bb90e77b5a16548583015c6ffca6a0b89284bc Mon Sep 17 00:00:00 2001 From: Dana Robinson <43805+derobins@users.noreply.github.com> Date: Wed, 26 Apr 2023 15:54:23 -0700 Subject: Harden H5G cache deserialization (#2810) --- src/H5Gcache.c | 113 +++++++++++++++++++++++---------------------------------- 1 file changed, 45 insertions(+), 68 deletions(-) diff --git a/src/H5Gcache.c b/src/H5Gcache.c index b6c6a85..e088fd8 100644 --- a/src/H5Gcache.c +++ b/src/H5Gcache.c @@ -13,10 +13,8 @@ /*------------------------------------------------------------------------- * * Created: H5Gcache.c - * Feb 5 2008 - * Quincey Koziol * - * Purpose: Implement group metadata cache methods. + * Purpose: Implement group metadata cache methods * *------------------------------------------------------------------------- */ @@ -101,15 +99,10 @@ H5FL_SEQ_EXTERN(H5G_entry_t); /*------------------------------------------------------------------------- * Function: H5G__cache_node_get_initial_load_size() * - * Purpose: Determine the size of the on-disk image of the node, and - * return this value in *image_len. - * - * Return: Success: SUCCEED - * Failure: FAIL - * - * Programmer: John Mainzer - * 7/21/14 + * Purpose: Determine the size of the on-disk image of the node, and + * return this value in *image_len. * + * Return: SUCCEED/FAIL *------------------------------------------------------------------------- */ static herr_t @@ -119,7 +112,6 @@ H5G__cache_node_get_initial_load_size(void *_udata, size_t *image_len) FUNC_ENTER_PACKAGE_NOERR - /* Sanity checks */ HDassert(f); HDassert(image_len); @@ -132,22 +124,18 @@ H5G__cache_node_get_initial_load_size(void *_udata, size_t *image_len) /*------------------------------------------------------------------------- * Function: H5G__cache_node_deserialize * - * Purpose: Given a buffer containing the on disk image of a symbol table - * node, allocate an instance of H5G_node_t, load the contents of the - * image into it, and return a pointer to the instance. + * Purpose: Given a buffer containing the on disk image of a symbol table + * node, allocate an instance of H5G_node_t, load the contents of the + * image into it, and return a pointer to the instance. * - * Note that deserializing the image requires access to the file - * pointer, which is not included in the parameter list for this - * callback. Finesse this issue by passing in the file pointer - * twice to the H5AC_protect() call -- once as the file pointer - * proper, and again as the user data + * Note that deserializing the image requires access to the file + * pointer, which is not included in the parameter list for this + * callback. Finesse this issue by passing in the file pointer + * twice to the H5AC_protect() call -- once as the file pointer + * proper, and again as the user data * * Return: Success: Pointer to in core representation * Failure: NULL - * - * Programmer: John Mainzer - * 6/21/14 - * *------------------------------------------------------------------------- */ static void * @@ -157,11 +145,10 @@ H5G__cache_node_deserialize(const void *_image, size_t len, void *_udata, hbool_ H5G_node_t *sym = NULL; /* Symbol table node created */ const uint8_t *image = (const uint8_t *)_image; /* Pointer to image to deserialize */ const uint8_t *image_end = image + len - 1; /* Pointer to end of image buffer */ - void *ret_value = NULL; /* Return value */ + void *ret_value = NULL; FUNC_ENTER_PACKAGE - /* Sanity checks */ HDassert(image); HDassert(len > 0); HDassert(f); @@ -174,22 +161,30 @@ H5G__cache_node_deserialize(const void *_image, size_t len, void *_udata, hbool_ if (NULL == (sym->entry = H5FL_SEQ_CALLOC(H5G_entry_t, (size_t)(2 * H5F_SYM_LEAF_K(f))))) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed") - /* magic */ + /* Magic */ + if (H5_IS_BUFFER_OVERFLOW(image, H5_SIZEOF_MAGIC, image_end)) + HGOTO_ERROR(H5E_SYM, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); if (HDmemcmp(image, H5G_NODE_MAGIC, (size_t)H5_SIZEOF_MAGIC) != 0) HGOTO_ERROR(H5E_SYM, H5E_BADVALUE, NULL, "bad symbol table node signature") image += H5_SIZEOF_MAGIC; - /* version */ + /* Version */ + if (H5_IS_BUFFER_OVERFLOW(image, 1, image_end)) + HGOTO_ERROR(H5E_SYM, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); if (H5G_NODE_VERS != *image++) HGOTO_ERROR(H5E_SYM, H5E_VERSION, NULL, "bad symbol table node version") - /* reserved */ + /* Reserved */ + if (H5_IS_BUFFER_OVERFLOW(image, 1, image_end)) + HGOTO_ERROR(H5E_SYM, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); image++; - /* number of symbols */ + /* Number of symbols */ + if (H5_IS_BUFFER_OVERFLOW(image, 2, image_end)) + HGOTO_ERROR(H5E_SYM, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding"); UINT16DECODE(image, sym->nsyms); - /* entries */ + /* Entries */ if (H5G__ent_decode_vec(f, &image, image_end, sym->entry, sym->nsyms) < 0) HGOTO_ERROR(H5E_SYM, H5E_CANTLOAD, NULL, "unable to decode symbol table entries") @@ -208,14 +203,9 @@ done: * Function: H5G__cache_node_image_len * * Purpose: Compute the size of the data structure on disk and return - * it in *image_len. - * - * Return: Success: SUCCEED - * Failure: FAIL - * - * Programmer: John Mainzer - * 6/21/14 + * it in *image_len * + * Return: SUCCEED/FAIL *------------------------------------------------------------------------- */ static herr_t @@ -225,7 +215,6 @@ H5G__cache_node_image_len(const void *_thing, size_t *image_len) FUNC_ENTER_PACKAGE_NOERR - /* Sanity checks */ HDassert(sym); HDassert(sym->cache_info.magic == H5C__H5C_CACHE_ENTRY_T_MAGIC); HDassert(sym->cache_info.type == H5AC_SNODE); @@ -239,17 +228,12 @@ H5G__cache_node_image_len(const void *_thing, size_t *image_len) /*------------------------------------------------------------------------- * Function: H5G__cache_node_serialize * - * Purpose: Given a correctly sized buffer and an instance of H5G_node_t, - * serialize the contents of the instance of H5G_node_t, and write - * this data into the supplied buffer. This buffer will be written - * to disk. - * - * Return: Success: SUCCEED - * Failure: FAIL - * - * Programmer: John Mainzer - * 7/21/14 + * Purpose: Given a correctly sized buffer and an instance of H5G_node_t, + * serialize the contents of the instance of H5G_node_t, and write + * this data into the supplied buffer. This buffer will be written + * to disk. * + * Return: SUCCEED/FAIL *------------------------------------------------------------------------- */ static herr_t @@ -257,11 +241,10 @@ H5G__cache_node_serialize(const H5F_t *f, void *_image, size_t len, void *_thing { H5G_node_t *sym = (H5G_node_t *)_thing; /* Pointer to object */ uint8_t *image = (uint8_t *)_image; /* Pointer into raw data buffer */ - herr_t ret_value = SUCCEED; /* Return value */ + herr_t ret_value = SUCCEED; FUNC_ENTER_PACKAGE - /* Sanity checks */ HDassert(f); HDassert(image); HDassert(sym); @@ -269,20 +252,20 @@ H5G__cache_node_serialize(const H5F_t *f, void *_image, size_t len, void *_thing HDassert(sym->cache_info.type == H5AC_SNODE); HDassert(len == sym->node_size); - /* magic number */ + /* Magic number */ H5MM_memcpy(image, H5G_NODE_MAGIC, (size_t)H5_SIZEOF_MAGIC); image += H5_SIZEOF_MAGIC; - /* version number */ + /* Version number */ *image++ = H5G_NODE_VERS; - /* reserved */ + /* Reserved */ *image++ = 0; - /* number of symbols */ + /* Number of symbols */ UINT16ENCODE(image, sym->nsyms); - /* entries */ + /* Entries */ if (H5G__ent_encode_vec(f, &image, sym->entry, sym->nsyms) < 0) HGOTO_ERROR(H5E_SYM, H5E_CANTENCODE, FAIL, "can't serialize") @@ -296,29 +279,23 @@ done: /*------------------------------------------------------------------------- * Function: H5G__cache_node_free_icr * - * Purpose: Destroys a symbol table node in memory. - * - * Note: The metadata cache sets the object's cache_info.magic to - * H5C__H5C_CACHE_ENTRY_T_BAD_MAGIC before calling a free_icr - * callback (checked in assert). - * - * Return: Success: SUCCEED - * Failure: FAIL + * Purpose: Destroy a symbol table node in memory * - * Programmer: John Mainzer - * 6/21/14 + * Note: The metadata cache sets the object's cache_info.magic to + * H5C__H5C_CACHE_ENTRY_T_BAD_MAGIC before calling a free_icr + * callback (checked in assert). * + * Return: SUCCEED/FAIL *------------------------------------------------------------------------- */ static herr_t H5G__cache_node_free_icr(void *_thing) { H5G_node_t *sym = (H5G_node_t *)_thing; /* Pointer to the object */ - herr_t ret_value = SUCCEED; /* Return value */ + herr_t ret_value = SUCCEED; FUNC_ENTER_PACKAGE - /* Sanity checks */ HDassert(sym); HDassert(sym->cache_info.magic == H5C__H5C_CACHE_ENTRY_T_BAD_MAGIC); HDassert(sym->cache_info.type == H5AC_SNODE); -- cgit v0.12