Skip to content

Commit

Permalink
Cherry-pick of CVE fixes from 1.10 (#3490)
Browse files Browse the repository at this point in the history
  • Loading branch information
derobins authored Sep 3, 2023
1 parent b1e6533 commit 90852b2
Show file tree
Hide file tree
Showing 9 changed files with 175 additions and 98 deletions.
26 changes: 26 additions & 0 deletions release_docs/RELEASE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,32 @@ Bug Fixes since HDF5-1.12.2 release
===================================
Library
-------
- Fixed CVE-2018-11202

A malformed file could result in chunk index memory leaks. Under most
conditions (i.e., when the --enable-using-memchecker option is NOT
used), this would result in a small memory leak and and infinite loop
and abort when shutting down the library. The infinite loop would be
due to the "free list" package not being able to clear its resources
so the library couldn't shut down. When the "using a memory checker"
option is used, the free lists are disabled so there is just a memory
leak with no abort on library shutdown.

The chunk index resources are now correctly cleaned up when reading
misparsed files and valgrind confirms no memory leaks.

- Fixed an assertion in a previous fix for CVE-2016-4332

An assert could fail when processing corrupt files that have invalid
shared message flags (as in CVE-2016-4332).

The assert statement in question has been replaced with pointer checks
that don't raise errors. Since the function is in cleanup code, we do
our best to close and free things, even when presented with partially
initialized structs.

Fixes CVE-2016-4332 and HDFFV-9950 (confirmed via the cve_hdf5 repo)

- Fixed a file space allocation bug in the parallel library for chunked
datasets

Expand Down
3 changes: 2 additions & 1 deletion src/H5Dbtree.c
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,8 @@ H5D__btree_decode_key(const H5B_shared_t *shared, const uint8_t *raw, void *_key

/* Retrieve coordinate offset */
UINT64DECODE(raw, tmp_offset);
HDassert(0 == (tmp_offset % layout->dim[u]));
if (0 != (tmp_offset % layout->dim[u]))
HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, FAIL, "bad coordinate offset");

/* Convert to a scaled offset */
key->scaled[u] = tmp_offset / layout->dim[u];
Expand Down
15 changes: 14 additions & 1 deletion src/H5Dchunk.c
Original file line number Diff line number Diff line change
Expand Up @@ -701,9 +701,12 @@ H5D__chunk_set_info_real(H5O_layout_chunk_t *layout, unsigned ndims, const hsize

/* Sanity checks */
HDassert(layout);
HDassert(ndims > 0);
HDassert(curr_dims);

/* Can happen when corrupt files are parsed */
if (ndims == 0)
HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, FAIL, "number of dimensions cannot be zero")

/* Compute the # of chunks in dataset dimensions */
for (u = 0, layout->nchunks = 1, layout->max_nchunks = 1; u < ndims; u++) {
/* Round up to the next integer # of chunks, to accommodate partial chunks */
Expand Down Expand Up @@ -915,6 +918,7 @@ H5D__chunk_init(H5F_t *f, const H5D_t *const dset, hid_t dapl_id)
H5D_rdcc_t *rdcc = &(dset->shared->cache.chunk); /* Convenience pointer to dataset's chunk cache */
H5P_genplist_t *dapl; /* Data access property list object pointer */
H5O_storage_chunk_t *sc = &(dset->shared->layout.storage.u.chunk);
hbool_t idx_init = FALSE;
herr_t ret_value = SUCCEED; /* Return value */

FUNC_ENTER_STATIC
Expand Down Expand Up @@ -990,12 +994,21 @@ H5D__chunk_init(H5F_t *f, const H5D_t *const dset, hid_t dapl_id)
/* Allocate any indexing structures */
if (sc->ops->init && (sc->ops->init)(&idx_info, dset->shared->space, dset->oloc.addr) < 0)
HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "can't initialize indexing information")
idx_init = TRUE;

/* Set the number of chunks in dataset, etc. */
if (H5D__chunk_set_info(dset) < 0)
HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "unable to set # of chunks for dataset")

done:
if (FAIL == ret_value) {
if (rdcc->slot)
rdcc->slot = H5FL_SEQ_FREE(H5D_rdcc_ent_ptr_t, rdcc->slot);

if (idx_init && sc->ops->dest && (sc->ops->dest)(&idx_info) < 0)
HDONE_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL, "unable to release chunk index info");
}

FUNC_LEAVE_NOAPI(ret_value)
} /* end H5D__chunk_init() */

Expand Down
106 changes: 72 additions & 34 deletions src/H5Fsuper_cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -409,13 +409,13 @@ H5F__cache_superblock_verify_chksum(const void *_image, size_t len, void *_udata
*-------------------------------------------------------------------------
*/
static void *
H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUSED len, void *_udata,
hbool_t H5_ATTR_UNUSED *dirty)
H5F__cache_superblock_deserialize(const void *_image, size_t len, void *_udata, hbool_t H5_ATTR_UNUSED *dirty)
{
H5F_super_t *sblock = NULL; /* File's superblock */
H5F_superblock_cache_ud_t *udata = (H5F_superblock_cache_ud_t *)_udata; /* User data */
const uint8_t *image = _image; /* Pointer into raw data buffer */
H5F_super_t *ret_value = NULL; /* Return value */
const uint8_t *image = _image; /* Pointer into raw data buffer */
const uint8_t *end = image + len - 1; /* Pointer to end of buffer */
H5F_super_t *ret_value = NULL;

FUNC_ENTER_STATIC

Expand All @@ -427,11 +427,11 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS

/* Allocate space for the superblock */
if (NULL == (sblock = H5FL_CALLOC(H5F_super_t)))
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed")
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed");

/* Deserialize the file superblock's prefix */
if (H5F__superblock_prefix_decode(sblock, &image, udata, FALSE) < 0)
HGOTO_ERROR(H5E_FILE, H5E_CANTDECODE, NULL, "can't decode file superblock prefix")
HGOTO_ERROR(H5E_FILE, H5E_CANTDECODE, NULL, "can't decode file superblock prefix");

/* Check for older version of superblock format */
if (sblock->super_vers < HDF5_SUPERBLOCK_VERSION_2) {
Expand All @@ -441,85 +441,113 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS
unsigned chunk_btree_k; /* B-tree chunk internal node 'K' value */

/* Freespace version (hard-wired) */
if (H5_IS_BUFFER_OVERFLOW(image, 1, end))
HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds");
if (HDF5_FREESPACE_VERSION != *image++)
HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad free space version number")
HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad free space version number");

/* Root group version number (hard-wired) */
if (H5_IS_BUFFER_OVERFLOW(image, 1, end))
HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds");
if (HDF5_OBJECTDIR_VERSION != *image++)
HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad object directory version number")
HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad object directory version number");

/* Skip over reserved byte */
if (H5_IS_BUFFER_OVERFLOW(image, 1, end))
HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds");
image++;

/* Shared header version number (hard-wired) */
if (H5_IS_BUFFER_OVERFLOW(image, 1, end))
HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds");
if (HDF5_SHAREDHEADER_VERSION != *image++)
HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad shared-header format version number")
HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad shared-header format version number");

/* Skip over size of file addresses (already decoded) */
if (H5_IS_BUFFER_OVERFLOW(image, 1, end))
HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds");
image++;
udata->f->shared->sizeof_addr = sblock->sizeof_addr; /* Keep a local copy also */

/* Skip over size of file sizes (already decoded) */
if (H5_IS_BUFFER_OVERFLOW(image, 1, end))
HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds");
image++;
udata->f->shared->sizeof_size = sblock->sizeof_size; /* Keep a local copy also */

/* Skip over reserved byte */
if (H5_IS_BUFFER_OVERFLOW(image, 1, end))
HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds");
image++;

/* Various B-tree sizes */
if (H5_IS_BUFFER_OVERFLOW(image, 2, end))
HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds");
UINT16DECODE(image, sym_leaf_k);
if (sym_leaf_k == 0)
HGOTO_ERROR(H5E_FILE, H5E_BADRANGE, NULL, "bad symbol table leaf node 1/2 rank")
HGOTO_ERROR(H5E_FILE, H5E_BADRANGE, NULL, "bad symbol table leaf node 1/2 rank");
udata->sym_leaf_k = sym_leaf_k; /* Keep a local copy also */

/* Need 'get' call to set other array values */
if (H5_IS_BUFFER_OVERFLOW(image, 2, end))
HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds");
UINT16DECODE(image, snode_btree_k);
if (snode_btree_k == 0)
HGOTO_ERROR(H5E_FILE, H5E_BADRANGE, NULL, "bad 1/2 rank for btree internal nodes")
HGOTO_ERROR(H5E_FILE, H5E_BADRANGE, NULL, "bad 1/2 rank for btree internal nodes");
udata->btree_k[H5B_SNODE_ID] = snode_btree_k;

/*
* Delay setting the value in the property list until we've checked
/* Delay setting the value in the property list until we've checked
* for the indexed storage B-tree internal 'K' value later.
*/

/* File status flags (not really used yet) */
if (H5_IS_BUFFER_OVERFLOW(image, 4, end))
HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds");
UINT32DECODE(image, status_flags);
HDassert(status_flags <= 255);
if (status_flags > 255)
HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad superblock status flags");
sblock->status_flags = (uint8_t)status_flags;
if (sblock->status_flags & ~H5F_SUPER_ALL_FLAGS)
HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad flag value for superblock")
HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad flag value for superblock");

/*
* If the superblock version # is greater than 0, read in the indexed
/* If the superblock version # is greater than 0, read in the indexed
* storage B-tree internal 'K' value
*/
if (sblock->super_vers > HDF5_SUPERBLOCK_VERSION_DEF) {
if (H5_IS_BUFFER_OVERFLOW(image, 2, end))
HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds");
UINT16DECODE(image, chunk_btree_k);

/* Reserved bytes are present only in version 1 */
if (sblock->super_vers == HDF5_SUPERBLOCK_VERSION_1)
image += 2; /* reserved */
} /* end if */
if (sblock->super_vers == HDF5_SUPERBLOCK_VERSION_1) {
/* Reserved */
if (H5_IS_BUFFER_OVERFLOW(image, 2, end))
HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds");
image += 2;
}
}
else
chunk_btree_k = HDF5_BTREE_CHUNK_IK_DEF;
udata->btree_k[H5B_CHUNK_ID] = chunk_btree_k;

/* Remainder of "variable-sized" portion of superblock */
if (H5_IS_BUFFER_OVERFLOW(image, H5F_sizeof_addr(udata->f) * 4, end))
HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds");
H5F_addr_decode(udata->f, (const uint8_t **)&image, &sblock->base_addr /*out*/);
H5F_addr_decode(udata->f, (const uint8_t **)&image, &sblock->ext_addr /*out*/);
H5F_addr_decode(udata->f, (const uint8_t **)&image, &udata->stored_eof /*out*/);
H5F_addr_decode(udata->f, (const uint8_t **)&image, &sblock->driver_addr /*out*/);

/* Allocate space for the root group symbol table entry */
HDassert(!sblock->root_ent);
if (sblock->root_ent)
HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "root entry should not exist yet");
if (NULL == (sblock->root_ent = (H5G_entry_t *)H5MM_calloc(sizeof(H5G_entry_t))))
HGOTO_ERROR(H5E_FILE, H5E_CANTALLOC, NULL,
"can't allocate space for root group symbol table entry")
"can't allocate space for root group symbol table entry");

/* decode the root group symbol table entry */
/* Decode the root group symbol table entry */
if (H5G_ent_decode(udata->f, (const uint8_t **)&image, sblock->root_ent) < 0)
HGOTO_ERROR(H5E_FILE, H5E_CANTDECODE, NULL, "can't decode root group symbol table entry")
HGOTO_ERROR(H5E_FILE, H5E_CANTDECODE, NULL, "can't decode root group symbol table entry");

/* Set the root group address to the correct value */
sblock->root_addr = sblock->root_ent->header;
Expand All @@ -533,26 +561,32 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS
/* Eliminate the driver info */
sblock->driver_addr = HADDR_UNDEF;
udata->drvrinfo_removed = TRUE;
} /* end if */
}

/* NOTE: Driver info block is decoded separately, later */

} /* end if */
}
else {
uint32_t read_chksum; /* Checksum read from file */

/* Skip over size of file addresses (already decoded) */
image++;
udata->f->shared->sizeof_addr = sblock->sizeof_addr; /* Keep a local copy also */

/* Skip over size of file sizes (already decoded) */
image++;
udata->f->shared->sizeof_size = sblock->sizeof_size; /* Keep a local copy also */

/* Check whether the image pointer is out of bounds */
if (H5_IS_BUFFER_OVERFLOW(image, 1, end))
HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds");

/* File status flags (not really used yet) */
sblock->status_flags = *image++;
if (sblock->status_flags & ~H5F_SUPER_ALL_FLAGS)
HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad flag value for superblock")
HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad flag value for superblock");

/* Check whether the image pointer will be out of bounds */
if (H5_IS_BUFFER_OVERFLOW(image, H5F_SIZEOF_ADDR(udata->f) * 4, end))
HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds");

/* Base, superblock extension, end of file & root group object header addresses */
H5F_addr_decode(udata->f, (const uint8_t **)&image, &sblock->base_addr /*out*/);
Expand All @@ -562,6 +596,10 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS

/* checksum verification already done in verify_chksum cb */

/* Check whether the image pointer will be out of bounds */
if (H5_IS_BUFFER_OVERFLOW(image, sizeof(uint32_t), end))
HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds");

/* Decode checksum */
UINT32DECODE(image, read_chksum);

Expand All @@ -571,19 +609,19 @@ H5F__cache_superblock_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUS
* any attempt to load the Driver Information Block.
*/
sblock->driver_addr = HADDR_UNDEF;
} /* end else */
}

/* Sanity check */
HDassert((size_t)(image - (const uint8_t *)_image) <= len);
/* Size check */
if ((size_t)(image - (const uint8_t *)_image) > len)
HDONE_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "bad decoded superblock size");

/* Set return value */
ret_value = sblock;

done:
/* Release the [possibly partially initialized] superblock on error */
if (!ret_value && sblock)
if (H5F__super_free(sblock) < 0)
HDONE_ERROR(H5E_FILE, H5E_CANTFREE, NULL, "unable to destroy superblock data")
HDONE_ERROR(H5E_FILE, H5E_CANTFREE, NULL, "unable to destroy superblock data");

FUNC_LEAVE_NOAPI(ret_value)
} /* end H5F__cache_superblock_deserialize() */
Expand Down
22 changes: 7 additions & 15 deletions src/H5Gint.c
Original file line number Diff line number Diff line change
Expand Up @@ -909,15 +909,13 @@ H5G__visit_cb(const H5O_link_t *lnk, void *_udata)
/* Check if we've seen the object the link references before */
if (NULL == H5SL_search(udata->visited, &obj_pos)) {
H5O_type_t otype; /* Basic object type (group, dataset, etc.) */
unsigned rc; /* Reference count of object */

/* Get the object's reference count and type */
if (H5O_get_rc_and_type(&obj_oloc, &rc, &otype) < 0)
if (H5O_get_rc_and_type(&obj_oloc, NULL, &otype) < 0)
HGOTO_ERROR(H5E_SYM, H5E_CANTGET, H5_ITER_ERROR, "unable to get object info")

/* If its ref count is > 1, we add it to the list of visited objects */
/* (because it could come up again during traversal) */
if (rc > 1) {
/* Add it to the list of visited objects */
{
H5_obj_t *new_node; /* New object node for visited list */

/* Allocate new object "position" node */
Expand All @@ -931,7 +929,7 @@ H5G__visit_cb(const H5O_link_t *lnk, void *_udata)
if (H5SL_insert(udata->visited, new_node, new_node) < 0)
HGOTO_ERROR(H5E_SYM, H5E_CANTINSERT, H5_ITER_ERROR,
"can't insert object node into visited list")
} /* end if */
}

/* If it's a group, we recurse into it */
if (otype == H5O_TYPE_GROUP) {
Expand Down Expand Up @@ -1026,7 +1024,6 @@ H5G_visit(H5G_loc_t *loc, const char *group_name, H5_index_t idx_type, H5_iter_o
hid_t gid = H5I_INVALID_HID; /* Group ID */
H5G_t *grp = NULL; /* Group opened */
H5G_loc_t start_loc; /* Location of starting group */
unsigned rc; /* Reference count of object */
herr_t ret_value = FAIL; /* Return value */

/* Portably clear udata struct (before FUNC_ENTER) */
Expand Down Expand Up @@ -1068,13 +1065,8 @@ H5G_visit(H5G_loc_t *loc, const char *group_name, H5_index_t idx_type, H5_iter_o
if ((udata.visited = H5SL_create(H5SL_TYPE_OBJ, NULL)) == NULL)
HGOTO_ERROR(H5E_SYM, H5E_CANTCREATE, FAIL, "can't create skip list for visited objects")

/* Get the group's reference count */
if (H5O_get_rc_and_type(&grp->oloc, &rc, NULL) < 0)
HGOTO_ERROR(H5E_SYM, H5E_CANTGET, FAIL, "unable to get object info")

/* If its ref count is > 1, we add it to the list of visited objects */
/* (because it could come up again during traversal) */
if (rc > 1) {
/* Add it to the list of visited objects */
{
H5_obj_t *obj_pos; /* New object node for visited list */

/* Allocate new object "position" node */
Expand All @@ -1088,7 +1080,7 @@ H5G_visit(H5G_loc_t *loc, const char *group_name, H5_index_t idx_type, H5_iter_o
/* Add to list of visited objects */
if (H5SL_insert(udata.visited, obj_pos, obj_pos) < 0)
HGOTO_ERROR(H5E_SYM, H5E_CANTINSERT, FAIL, "can't insert object node into visited list")
} /* end if */
}

/* Attempt to get the link info for this group */
if ((linfo_exists = H5G__obj_get_linfo(&(grp->oloc), &linfo)) < 0)
Expand Down
Loading

0 comments on commit 90852b2

Please sign in to comment.