From 2636f401ba236e99adda4cc50fb89bebbe0b73fd Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Fri, 11 May 2018 16:51:03 -0700 Subject: Moved a fix for HDFFV-10358 (CVE-2017-17509) from develop to 1.8. This was done manually due to the cache differences between 1.8 and develop. --- release_docs/RELEASE.txt | 17 ++++++++++++++++ src/H5Gcache.c | 53 ++++++++++++++++++++++++------------------------ src/H5Gent.c | 8 ++++++-- src/H5Gpkg.h | 6 +++--- 4 files changed, 52 insertions(+), 32 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 30b148a..ad06d40 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -209,6 +209,23 @@ Bug Fixes since HDF5-1.8.20 (DER - 2018/02/26, HDFFV-10357) + - If an HDF5 file contains a malformed symbol table node that declares + it contains more symbols than it actually contains, the library + can run off the end of the metadata cache buffer while processing + the symbol table node. + + This issue was reported to The HDF Group as issue #CVE-2017-17509. + + NOTE: The HDF5 C library cannot produce such a file. This condition + should only occur in a corrupt (or deliberately altered) file + or a file created by third-party software. + + Performing bounds checks on the buffer while processing fixes the + problem. Instead of the segmentation fault, the normal HDF5 error + handling is invoked. + + (DER - 2018/03/12, HDFFV-10358) + Configuration ------------- - CMake diff --git a/src/H5Gcache.c b/src/H5Gcache.c index 1ca80df..994c5bf 100644 --- a/src/H5Gcache.c +++ b/src/H5Gcache.c @@ -103,49 +103,43 @@ H5FL_SEQ_EXTERN(H5G_entry_t); /*------------------------------------------------------------------------- - * Function: H5G_node_load + * Function: H5G_node_load * - * Purpose: Loads a symbol table node from the file. + * Purpose: Loads a symbol table node from the file. * - * Return: Success: Ptr to the new table. + * Return: Success: Ptr to the new table. + * Failure: NULL * - * Failure: NULL - * - * Programmer: Robb Matzke - * matzke@llnl.gov - * Jun 23 1997 + * Programmer: Robb Matzke + * matzke@llnl.gov + * Jun 23 1997 * *------------------------------------------------------------------------- */ static H5G_node_t * H5G_node_load(H5F_t *f, hid_t dxpl_id, haddr_t addr, void *udata) { - H5G_node_t *sym = NULL; - H5WB_t *wb = NULL; /* Wrapped buffer for node data */ - uint8_t node_buf[H5G_NODE_BUF_SIZE]; /* Buffer for node */ - uint8_t *node; /* Pointer to node buffer */ + H5G_node_t *sym = NULL; + H5WB_t *wb = NULL; /* Wrapped buffer for node data */ + uint8_t node_buf[H5G_NODE_BUF_SIZE]; /* Buffer for node */ + uint8_t *node; /* Pointer to node buffer */ const uint8_t *p; - H5G_node_t *ret_value; /*for error handling */ + const uint8_t *p_end; + H5G_node_t *ret_value; /* Return value */ FUNC_ENTER_NOAPI_NOINIT - /* - * Check arguments. - */ + /* Sanity checks */ HDassert(f); HDassert(H5F_addr_defined(addr)); HDassert(udata); - /* - * Initialize variables. - */ - /* Allocate symbol table data structures */ if(NULL == (sym = H5FL_CALLOC(H5G_node_t))) - HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed") + HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed") sym->node_size = H5G_NODE_SIZE(f); 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") + HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed") /* Wrap the local buffer for serialized node info */ if(NULL == (wb = H5WB_wrap(node_buf, sizeof(node_buf)))) @@ -157,19 +151,24 @@ H5G_node_load(H5F_t *f, hid_t dxpl_id, haddr_t addr, void *udata) /* Read the serialized symbol table node. */ if(H5F_block_read(f, H5FD_MEM_BTREE, addr, sym->node_size, dxpl_id, node) < 0) - HGOTO_ERROR(H5E_SYM, H5E_READERROR, NULL, "unable to read symbol table node") + HGOTO_ERROR(H5E_SYM, H5E_READERROR, NULL, "unable to read symbol table node") /* Get temporary pointer to serialized node */ p = node; + /* Get a pointer to the end of the node. This ensures we don't run off + * the end of the buffer if the file is corrupt. + */ + p_end = p + sym->node_size - 1; + /* magic */ if(HDmemcmp(p, H5G_NODE_MAGIC, (size_t)H5_SIZEOF_MAGIC)) - HGOTO_ERROR(H5E_SYM, H5E_CANTLOAD, NULL, "bad symbol table node signature") + HGOTO_ERROR(H5E_SYM, H5E_CANTLOAD, NULL, "bad symbol table node signature") p += 4; /* version */ if(H5G_NODE_VERS != *p++) - HGOTO_ERROR(H5E_SYM, H5E_CANTLOAD, NULL, "bad symbol table node version") + HGOTO_ERROR(H5E_SYM, H5E_CANTLOAD, NULL, "bad symbol table node version") /* reserved */ p++; @@ -178,8 +177,8 @@ H5G_node_load(H5F_t *f, hid_t dxpl_id, haddr_t addr, void *udata) UINT16DECODE(p, sym->nsyms); /* entries */ - if(H5G__ent_decode_vec(f, &p, sym->entry, sym->nsyms) < 0) - HGOTO_ERROR(H5E_SYM, H5E_CANTLOAD, NULL, "unable to decode symbol table entries") + if(H5G__ent_decode_vec(f, &p, p_end, sym->entry, sym->nsyms) < 0) + HGOTO_ERROR(H5E_SYM, H5E_CANTLOAD, NULL, "unable to decode symbol table entries") /* Set return value */ ret_value = sym; diff --git a/src/H5Gent.c b/src/H5Gent.c index bb6aa38..1bd8e63 100644 --- a/src/H5Gent.c +++ b/src/H5Gent.c @@ -91,7 +91,8 @@ H5FL_BLK_EXTERN(str_buf); *------------------------------------------------------------------------- */ herr_t -H5G__ent_decode_vec(const H5F_t *f, const uint8_t **pp, H5G_entry_t *ent, unsigned n) +H5G__ent_decode_vec(const H5F_t *f, const uint8_t **pp, const uint8_t *p_end, + H5G_entry_t *ent, unsigned n) { unsigned u; /* Local index variable */ herr_t ret_value = SUCCEED; /* Return value */ @@ -104,9 +105,12 @@ H5G__ent_decode_vec(const H5F_t *f, const uint8_t **pp, H5G_entry_t *ent, unsign HDassert(ent); /* decode entries */ - for(u = 0; u < n; u++) + for(u = 0; u < n; u++) { + if(*pp > p_end) + HGOTO_ERROR(H5E_SYM, H5E_CANTDECODE, FAIL, "ran off the end of the buffer") if(H5G_ent_decode(f, pp, ent + u) < 0) HGOTO_ERROR(H5E_SYM, H5E_CANTDECODE, FAIL, "can't decode") + } done: FUNC_LEAVE_NOAPI(ret_value) diff --git a/src/H5Gpkg.h b/src/H5Gpkg.h index 07aff02..f49ef26 100644 --- a/src/H5Gpkg.h +++ b/src/H5Gpkg.h @@ -1,4 +1,4 @@ -/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * Copyright by The HDF Group. * * Copyright by the Board of Trustees of the University of Illinois. * * All rights reserved. * @@ -400,9 +400,9 @@ H5_DLL void H5G__ent_copy(H5G_entry_t *dst, const H5G_entry_t *src, H5_copy_depth_t depth); H5_DLL void H5G__ent_reset(H5G_entry_t *ent); H5_DLL herr_t H5G__ent_decode_vec(const H5F_t *f, const uint8_t **pp, - H5G_entry_t *ent, unsigned n); + const uint8_t *p_end, H5G_entry_t *ent, unsigned n); H5_DLL herr_t H5G__ent_encode_vec(const H5F_t *f, uint8_t **pp, - const H5G_entry_t *ent, unsigned n); + const H5G_entry_t *ent, unsigned n); H5_DLL herr_t H5G__ent_convert(H5F_t *f, hid_t dxpl_id, H5HL_t *heap, const char *name, const H5O_link_t *lnk, H5O_type_t obj_type, const void *crt_info, H5G_entry_t *ent); -- cgit v0.12