From: Egbert Eich Date: Sat Oct 1 15:13:52 2022 +0200 Subject: Pass compact chunk size info to ensure requested elements are within bounds Patch-mainline: Not yet Git-repo: ssh://eich@192.168.122.1:/home/eich/sources/HPC/hdf5 Git-commit: 18300944261a9fa8f0087f99d9176f3757b1ec38 References: To avoid reading/writing elements out of bounds of a compact chunk, pass size info and check whether all elements are within the size before attempting to read/write these elements. Such accesses can occur when accessing malformed hdf5 files. This fixes CVE-2018-11205 Signed-off-by: Egbert Eich Signed-off-by: Egbert Eich --- src/H5Dchunk.c | 34 +++++++++++++++++++++++++++------- src/H5Dcompact.c | 5 +++++ src/H5Dpkg.h | 1 + 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/src/H5Dchunk.c b/src/H5Dchunk.c index e6bf26ce89..94ad392cb7 100644 --- a/src/H5Dchunk.c +++ b/src/H5Dchunk.c @@ -128,6 +128,7 @@ typedef struct H5D_rdcc_ent_t { H5F_block_t chunk_block; /*offset/length of chunk in file */ hsize_t chunk_idx; /*index of chunk in dataset */ uint8_t * chunk; /*the unfiltered chunk data */ + size_t size; /*size of chunk */ unsigned idx; /*index in hash table */ struct H5D_rdcc_ent_t *next; /*next item in doubly-linked list */ struct H5D_rdcc_ent_t *prev; /*previous item in doubly-linked list */ @@ -303,7 +304,7 @@ static unsigned H5D__chunk_hash_val(const H5D_shared_t *shared, const hsize_t *s static herr_t H5D__chunk_flush_entry(const H5D_t *dset, H5D_rdcc_ent_t *ent, hbool_t reset); static herr_t H5D__chunk_cache_evict(const H5D_t *dset, H5D_rdcc_ent_t *ent, hbool_t flush); static void * H5D__chunk_lock(const H5D_io_info_t *io_info, H5D_chunk_ud_t *udata, hbool_t relax, - hbool_t prev_unfilt_chunk); + hbool_t prev_unfilt_chunk, size_t *ret_size); static herr_t H5D__chunk_unlock(const H5D_io_info_t *io_info, const H5D_chunk_ud_t *udata, hbool_t dirty, void *chunk, uint32_t naccessed); static herr_t H5D__chunk_cache_prune(const H5D_t *dset, size_t size); @@ -2480,6 +2481,7 @@ H5D__chunk_read(H5D_io_info_t *io_info, const H5D_type_info_t *type_info, hsize_ uint32_t src_accessed_bytes = 0; /* Total accessed size in a chunk */ hbool_t skip_missing_chunks = FALSE; /* Whether to skip missing chunks */ herr_t ret_value = SUCCEED; /*return value */ + size_t chunk_size = 0; FUNC_ENTER_STATIC @@ -2565,11 +2567,12 @@ H5D__chunk_read(H5D_io_info_t *io_info, const H5D_type_info_t *type_info, hsize_ src_accessed_bytes = chunk_info->chunk_points * (uint32_t)type_info->src_type_size; /* Lock the chunk into the cache */ - if (NULL == (chunk = H5D__chunk_lock(io_info, &udata, FALSE, FALSE))) + if (NULL == (chunk = H5D__chunk_lock(io_info, &udata, FALSE, FALSE, &chunk_size))) HGOTO_ERROR(H5E_IO, H5E_READERROR, FAIL, "unable to read raw data chunk") /* Set up the storage buffer information for this chunk */ cpt_store.compact.buf = chunk; + cpt_store.compact.size = chunk_size; /* Point I/O info at contiguous I/O info for this chunk */ chk_io_info = &cpt_io_info; @@ -2629,6 +2632,7 @@ H5D__chunk_write(H5D_io_info_t *io_info, const H5D_type_info_t *type_info, hsize hbool_t cpt_dirty; /* Temporary placeholder for compact storage "dirty" flag */ uint32_t dst_accessed_bytes = 0; /* Total accessed size in a chunk */ herr_t ret_value = SUCCEED; /* Return value */ + size_t chunk_size; FUNC_ENTER_STATIC @@ -2699,11 +2703,12 @@ H5D__chunk_write(H5D_io_info_t *io_info, const H5D_type_info_t *type_info, hsize entire_chunk = FALSE; /* Lock the chunk into the cache */ - if (NULL == (chunk = H5D__chunk_lock(io_info, &udata, entire_chunk, FALSE))) + if (NULL == (chunk = H5D__chunk_lock(io_info, &udata, entire_chunk, FALSE, &chunk_size))) HGOTO_ERROR(H5E_IO, H5E_READERROR, FAIL, "unable to read raw data chunk") /* Set up the storage buffer information for this chunk */ cpt_store.compact.buf = chunk; + cpt_store.compact.size = chunk_size; /* Point I/O info at main I/O info for this chunk */ chk_io_info = &cpt_io_info; @@ -3714,7 +3719,7 @@ done: *------------------------------------------------------------------------- */ static void * -H5D__chunk_lock(const H5D_io_info_t *io_info, H5D_chunk_ud_t *udata, hbool_t relax, hbool_t prev_unfilt_chunk) +H5D__chunk_lock(const H5D_io_info_t *io_info, H5D_chunk_ud_t *udata, hbool_t relax, hbool_t prev_unfilt_chunk, size_t *ret_size) { const H5D_t * dset = io_info->dset; /* Local pointer to the dataset info */ const H5O_pline_t *pline = @@ -3731,6 +3736,7 @@ H5D__chunk_lock(const H5D_io_info_t *io_info, H5D_chunk_ud_t *udata, hbool_t rel hbool_t disable_filters = FALSE; /* Whether to disable filters (when adding to cache) */ void * chunk = NULL; /*the file chunk */ void * ret_value = NULL; /* Return value */ + size_t chunk_size_ret = 0; FUNC_ENTER_STATIC @@ -3796,6 +3802,7 @@ H5D__chunk_lock(const H5D_io_info_t *io_info, H5D_chunk_ud_t *udata, hbool_t rel ent->chunk = (uint8_t *)H5D__chunk_mem_xfree(ent->chunk, old_pline); ent->chunk = (uint8_t *)chunk; chunk = NULL; + ent->size = chunk_size; /* Mark the chunk as having filters disabled as well as "newly * disabled" so it is inserted on flush */ @@ -3823,6 +3830,7 @@ H5D__chunk_lock(const H5D_io_info_t *io_info, H5D_chunk_ud_t *udata, hbool_t rel ent->chunk = (uint8_t *)H5D__chunk_mem_xfree(ent->chunk, old_pline); ent->chunk = (uint8_t *)chunk; chunk = NULL; + ent->size = chunk_size; /* Mark the chunk as having filters enabled */ ent->edge_chunk_state &= ~(H5D_RDCC_DISABLE_FILTERS | H5D_RDCC_NEWLY_DISABLED_FILTERS); @@ -3902,6 +3910,7 @@ H5D__chunk_lock(const H5D_io_info_t *io_info, H5D_chunk_ud_t *udata, hbool_t rel /* In the case that some dataset functions look through this data, * clear it to all 0s. */ HDmemset(chunk, 0, chunk_size); + chunk_size_ret = chunk_size; } /* end if */ else { /* @@ -3924,6 +3933,7 @@ H5D__chunk_lock(const H5D_io_info_t *io_info, H5D_chunk_ud_t *udata, hbool_t rel my_chunk_alloc, chunk) < 0) HGOTO_ERROR(H5E_IO, H5E_READERROR, NULL, "unable to read raw data chunk") + chunk_size_ret = my_chunk_alloc; if (old_pline && old_pline->nused) { H5Z_EDC_t err_detect; /* Error detection info */ H5Z_cb_t filter_cb; /* I/O filter callback function */ @@ -3937,6 +3947,7 @@ H5D__chunk_lock(const H5D_io_info_t *io_info, H5D_chunk_ud_t *udata, hbool_t rel if (H5Z_pipeline(old_pline, H5Z_FLAG_REVERSE, &(udata->filter_mask), err_detect, filter_cb, &my_chunk_alloc, &buf_alloc, &chunk) < 0) HGOTO_ERROR(H5E_DATASET, H5E_CANTFILTER, NULL, "data pipeline read failed") + chunk_size_ret = buf_alloc; /* Reallocate chunk if necessary */ if (udata->new_unfilt_chunk) { @@ -3947,6 +3958,7 @@ H5D__chunk_lock(const H5D_io_info_t *io_info, H5D_chunk_ud_t *udata, hbool_t rel HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed for raw data chunk") } /* end if */ + chunk_size_ret = my_chunk_alloc; H5MM_memcpy(chunk, tmp_chunk, chunk_size); (void)H5D__chunk_mem_xfree(tmp_chunk, old_pline); } /* end if */ @@ -3967,6 +3979,7 @@ H5D__chunk_lock(const H5D_io_info_t *io_info, H5D_chunk_ud_t *udata, hbool_t rel HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed for raw data chunk") + chunk_size_ret = chunk_size; if (H5P_is_fill_value_defined(fill, &fill_status) < 0) HGOTO_ERROR(H5E_PLIST, H5E_CANTGET, NULL, "can't tell if fill value defined") @@ -4032,6 +4045,7 @@ H5D__chunk_lock(const H5D_io_info_t *io_info, H5D_chunk_ud_t *udata, hbool_t rel H5_CHECKED_ASSIGN(ent->rd_count, uint32_t, chunk_size, size_t); H5_CHECKED_ASSIGN(ent->wr_count, uint32_t, chunk_size, size_t); ent->chunk = (uint8_t *)chunk; + ent->size = chunk_size_ret; /* Add it to the cache */ HDassert(NULL == rdcc->slot[udata->idx_hint]); @@ -4065,6 +4079,7 @@ H5D__chunk_lock(const H5D_io_info_t *io_info, H5D_chunk_ud_t *udata, hbool_t rel HDassert(!ent->locked); ent->locked = TRUE; chunk = ent->chunk; + chunk_size_ret = ent->size; } /* end if */ else /* @@ -4076,6 +4091,8 @@ H5D__chunk_lock(const H5D_io_info_t *io_info, H5D_chunk_ud_t *udata, hbool_t rel /* Set return value */ ret_value = chunk; + if (ret_size != NULL) + *ret_size = chunk_size_ret; done: /* Release the fill buffer info, if it's been initialized */ @@ -4084,8 +4101,11 @@ done: /* Release the chunk allocated, on error */ if (!ret_value) - if (chunk) + if (chunk) { chunk = H5D__chunk_mem_xfree(chunk, pline); + if (ret_size != NULL) + *ret_size = 0; + } FUNC_LEAVE_NOAPI(ret_value) } /* end H5D__chunk_lock() */ @@ -4884,7 +4904,7 @@ H5D__chunk_update_old_edge_chunks(H5D_t *dset, hsize_t old_dim[]) if (H5F_addr_defined(chk_udata.chunk_block.offset) || (UINT_MAX != chk_udata.idx_hint)) { /* Lock the chunk into cache. H5D__chunk_lock will take care of * updating the chunk to no longer be an edge chunk. */ - if (NULL == (chunk = (void *)H5D__chunk_lock(&chk_io_info, &chk_udata, FALSE, TRUE))) + if (NULL == (chunk = (void *)H5D__chunk_lock(&chk_io_info, &chk_udata, FALSE, TRUE, NULL))) HGOTO_ERROR(H5E_DATASET, H5E_READERROR, FAIL, "unable to lock raw data chunk") /* Unlock the chunk */ @@ -5274,7 +5294,7 @@ H5D__chunk_prune_fill(H5D_chunk_it_ud1_t *udata, hbool_t new_unfilt_chunk) HGOTO_ERROR(H5E_DATASET, H5E_CANTSELECT, FAIL, "unable to select hyperslab") /* Lock the chunk into the cache, to get a pointer to the chunk buffer */ - if (NULL == (chunk = (void *)H5D__chunk_lock(io_info, &chk_udata, FALSE, FALSE))) + if (NULL == (chunk = (void *)H5D__chunk_lock(io_info, &chk_udata, FALSE, FALSE, NULL))) HGOTO_ERROR(H5E_DATASET, H5E_READERROR, FAIL, "unable to lock raw data chunk") /* Fill the selection in the memory buffer */ diff --git a/src/H5Dcompact.c b/src/H5Dcompact.c index b78693660d..21c37e8a08 100644 --- a/src/H5Dcompact.c +++ b/src/H5Dcompact.c @@ -245,6 +245,7 @@ H5D__compact_io_init(const H5D_io_info_t *io_info, const H5D_type_info_t H5_ATTR FUNC_ENTER_STATIC_NOERR io_info->store->compact.buf = io_info->dset->shared->layout.storage.u.compact.buf; + io_info->store->compact.size = io_info->dset->shared->layout.storage.u.compact.size; io_info->store->compact.dirty = &io_info->dset->shared->layout.storage.u.compact.dirty; FUNC_LEAVE_NOAPI(SUCCEED) @@ -278,6 +279,8 @@ H5D__compact_readvv(const H5D_io_info_t *io_info, size_t dset_max_nseq, size_t * FUNC_ENTER_STATIC HDassert(io_info); + if (io_info->store->compact.size < *(dset_offset_arr + dset_max_nseq - 1) + *(dset_size_arr + dset_max_nseq - 1)) + HGOTO_ERROR(H5E_IO, H5E_WRITEERROR, FAIL, "source size less than requested data") /* Use the vectorized memory copy routine to do actual work */ if ((ret_value = H5VM_memcpyvv(io_info->u.rbuf, mem_max_nseq, mem_curr_seq, mem_size_arr, mem_offset_arr, @@ -320,6 +323,8 @@ H5D__compact_writevv(const H5D_io_info_t *io_info, size_t dset_max_nseq, size_t FUNC_ENTER_STATIC HDassert(io_info); + if (io_info->store->compact.size < *(dset_offset_arr + dset_max_nseq - 1) + *(dset_size_arr + dset_max_nseq - 1)) + HGOTO_ERROR(H5E_IO, H5E_WRITEERROR, FAIL, "source size less than requested data") /* Use the vectorized memory copy routine to do actual work */ if ((ret_value = H5VM_memcpyvv(io_info->store->compact.buf, dset_max_nseq, dset_curr_seq, dset_size_arr, diff --git a/src/H5Dpkg.h b/src/H5Dpkg.h index 64692c5d1d..8a4acd62e3 100644 --- a/src/H5Dpkg.h +++ b/src/H5Dpkg.h @@ -196,6 +196,7 @@ typedef struct { typedef struct { void * buf; /* Buffer for compact dataset */ hbool_t *dirty; /* Pointer to dirty flag to mark */ + size_t size; /* Buffer size for compact dataset */ } H5D_compact_storage_t; typedef union H5D_storage_t {