From 6713ba2c6e54c0dd91133db45618d95c625b2cc4 Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Wed, 2 Aug 2023 11:47:06 -0700 Subject: [PATCH 1/4] Fixes 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. --- release_docs/RELEASE.txt | 14 ++++++++++++++ src/H5Dchunk.c | 3 +++ 2 files changed, 17 insertions(+) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index c382af05669..a646db37d24 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -265,6 +265,20 @@ Bug Fixes since HDF5-1.14.0 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 issue where an assert statement was converted to an incorrect error check statement diff --git a/src/H5Dchunk.c b/src/H5Dchunk.c index 7eba50cb4a1..cbcc1c5778a 100644 --- a/src/H5Dchunk.c +++ b/src/H5Dchunk.c @@ -961,6 +961,9 @@ H5D__chunk_init(H5F_t *f, const H5D_t *const dset, hid_t dapl_id) HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "unable to set # of chunks for dataset"); done: + if (FAIL == ret_value) + if (H5D__chunk_dest(dset) < 0) + HDONE_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL, "unable to clean up chunk structures during error cleanup"); FUNC_LEAVE_NOAPI(ret_value) } /* end H5D__chunk_init() */ From 179f00ff58c2cb68130c5fd5c1367c73a7cb7f18 Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Wed, 2 Aug 2023 11:51:47 -0700 Subject: [PATCH 2/4] Format source --- src/H5Dchunk.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/H5Dchunk.c b/src/H5Dchunk.c index cbcc1c5778a..fbf454a1e97 100644 --- a/src/H5Dchunk.c +++ b/src/H5Dchunk.c @@ -963,7 +963,8 @@ H5D__chunk_init(H5F_t *f, const H5D_t *const dset, hid_t dapl_id) done: if (FAIL == ret_value) if (H5D__chunk_dest(dset) < 0) - HDONE_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL, "unable to clean up chunk structures during error cleanup"); + HDONE_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL, + "unable to clean up chunk structures during error cleanup"); FUNC_LEAVE_NOAPI(ret_value) } /* end H5D__chunk_init() */ From a277e9bf87c35d41e6c3b335cf075a1c356979de Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Wed, 2 Aug 2023 12:02:22 -0700 Subject: [PATCH 3/4] Rework to avoid const warning --- src/H5Dchunk.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/H5Dchunk.c b/src/H5Dchunk.c index fbf454a1e97..3e7582be47c 100644 --- a/src/H5Dchunk.c +++ b/src/H5Dchunk.c @@ -961,10 +961,13 @@ H5D__chunk_init(H5F_t *f, const H5D_t *const dset, hid_t dapl_id) HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "unable to set # of chunks for dataset"); done: - if (FAIL == ret_value) - if (H5D__chunk_dest(dset) < 0) - HDONE_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL, - "unable to clean up chunk structures during error cleanup"); + if (FAIL == ret_value) { + if (rdcc->slot) + rdcc->slot = H5FL_SEQ_FREE(H5D_rdcc_ent_ptr_t, rdcc->slot); + + if (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() */ From aa825c1206d8f88bcec0576867a8810351618a76 Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Wed, 2 Aug 2023 13:05:37 -0700 Subject: [PATCH 4/4] Add a flag to only clean index if created --- src/H5Dchunk.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/H5Dchunk.c b/src/H5Dchunk.c index 3e7582be47c..be8ded47223 100644 --- a/src/H5Dchunk.c +++ b/src/H5Dchunk.c @@ -880,6 +880,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); + bool idx_init = false; herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_PACKAGE @@ -955,6 +956,7 @@ 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) @@ -965,7 +967,7 @@ H5D__chunk_init(H5F_t *f, const H5D_t *const dset, hid_t dapl_id) if (rdcc->slot) rdcc->slot = H5FL_SEQ_FREE(H5D_rdcc_ent_ptr_t, rdcc->slot); - if (sc->ops->dest && (sc->ops->dest)(&idx_info) < 0) + 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)