From 7e3f95679677db07a5cc606f5edfb723ea56d04e Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Mon, 26 Feb 2018 12:22:32 -0800 Subject: Fix for HDFFV-10354 (CVE-2017-17505). --- release_docs/RELEASE.txt | 23 +++++++++++++++++++++++ src/H5Opline.c | 44 +++++++++++++++++++++++++++++--------------- 2 files changed, 52 insertions(+), 15 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index c8e2bd5..eb9f0d9 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -132,6 +132,29 @@ Support for New Platforms, Languages, and Compilers Bug Fixes since HDF5-1.8.20 =========================== + - If an HDF5 file contains a filter pipeline message with a 'number of + filters' field that exceeds the maximum number of allowed filters, + the error handling code will attempt to dereference a NULL pointer. + + This issue was reported to The HDF Group as issue #CVE-2017-17505. + + 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. + + This problem arose because the error handling code assumed that + the 'number of filters' field implied that a dynamic array of that + size had already been created and that the cleanup code should + iterate over that array and clean up each element's resources. If + an error occurred before the array has been allocated, this will + not be true. + + This has been changed so that the number of filters is set to + zero on errors. Additionally, the filter array traversal in the + error handling code now requires that the filter array not be NULL. + + (DER - 2018/02/06, HDFFV-10354) + Configuration ------------- - CMake diff --git a/src/H5Opline.c b/src/H5Opline.c index 42fb72c..fb08431 100644 --- a/src/H5Opline.c +++ b/src/H5Opline.c @@ -132,8 +132,15 @@ H5O_pline_decode(H5F_t H5_ATTR_UNUSED *f, hid_t H5_ATTR_UNUSED dxpl_id, H5O_t H5 /* Number of filters */ pline->nused = *p++; - if(pline->nused > H5Z_MAX_NFILTERS) - HGOTO_ERROR(H5E_PLINE, H5E_CANTLOAD, NULL, "filter pipeline message has too many filters") + if(pline->nused > H5Z_MAX_NFILTERS) { + + /* Reset the number of filters used to avoid array traversal in error + * handling code. + */ + pline->nused = 0; + + HGOTO_ERROR(H5E_PLINE, H5E_CANTLOAD, NULL, "filter pipeline message has too many filters") + } /* Reserved */ if(pline->version == H5O_PLINE_VERSION_1) @@ -495,23 +502,30 @@ H5O_pline_reset(void *mesg) FUNC_ENTER_NOAPI_NOINIT_NOERR + /* NOTE: This function can be called during error processing from + * other API calls so DO NOT ASSUME THAT ANY VALUES ARE SANE. + */ + HDassert(pline); - /* Free information for each filter */ - for(i = 0; i < pline->nused; i++) { - if(pline->filter[i].name && pline->filter[i].name != pline->filter[i]._name) - HDassert((HDstrlen(pline->filter[i].name) + 1) > H5Z_COMMON_NAME_LEN); - if(pline->filter[i].name != pline->filter[i]._name) - pline->filter[i].name = (char *)H5MM_xfree(pline->filter[i].name); - if(pline->filter[i].cd_values && pline->filter[i].cd_values != pline->filter[i]._cd_values) - HDassert(pline->filter[i].cd_nelmts > H5Z_COMMON_CD_VALUES); - if(pline->filter[i].cd_values != pline->filter[i]._cd_values) - pline->filter[i].cd_values = (unsigned *)H5MM_xfree(pline->filter[i].cd_values); - } /* end for */ + /* Free the filter information and array */ + if (pline->filter) { + + /* Free information for each filter */ + for(i = 0; i < pline->nused; i++) { + if(pline->filter[i].name && pline->filter[i].name != pline->filter[i]._name) + HDassert((HDstrlen(pline->filter[i].name) + 1) > H5Z_COMMON_NAME_LEN); + if(pline->filter[i].name != pline->filter[i]._name) + pline->filter[i].name = (char *)H5MM_xfree(pline->filter[i].name); + if(pline->filter[i].cd_values && pline->filter[i].cd_values != pline->filter[i]._cd_values) + HDassert(pline->filter[i].cd_nelmts > H5Z_COMMON_CD_VALUES); + if(pline->filter[i].cd_values != pline->filter[i]._cd_values) + pline->filter[i].cd_values = (unsigned *)H5MM_xfree(pline->filter[i].cd_values); + } /* end for */ - /* Free filter array */ - if(pline->filter) + /* Free filter array */ pline->filter = (H5Z_filter_info_t *)H5MM_xfree(pline->filter); + } /* Reset # of filters */ pline->nused = pline->nalloc = 0; -- cgit v0.12 From 11a188a4b6f1da0bd81c54976e6ceb8530d71aa1 Mon Sep 17 00:00:00 2001 From: lrknox Date: Tue, 8 May 2018 15:39:40 -0500 Subject: Correct typo in h5jam/unjam usage messages. --- tools/h5jam/h5jam.c | 4 ++-- tools/h5jam/h5unjam.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/h5jam/h5jam.c b/tools/h5jam/h5jam.c index 519d6df..a7c1cc7 100644 --- a/tools/h5jam/h5jam.c +++ b/tools/h5jam/h5jam.c @@ -241,7 +241,7 @@ main (int argc, const char *argv[]) if (ub_file == NULL) { /* no user block */ - error_msg("missing arguemnt for -u .\n"); + error_msg("missing argument for -u .\n"); help_ref_msg(stderr); leave (EXIT_FAILURE); } @@ -255,7 +255,7 @@ main (int argc, const char *argv[]) } if (input_file == NULL) { - error_msg("missing arguemnt for -i .\n"); + error_msg("missing argument for -i .\n"); help_ref_msg(stderr); leave (EXIT_FAILURE); } diff --git a/tools/h5jam/h5unjam.c b/tools/h5jam/h5unjam.c index 9c83740..a269cfe 100644 --- a/tools/h5jam/h5unjam.c +++ b/tools/h5jam/h5unjam.c @@ -237,7 +237,7 @@ main(int argc, const char *argv[]) if (input_file == NULL) { /* no user block */ - error_msg("missing arguemnt for HDF5 file input.\n"); + error_msg("missing argument for HDF5 file input.\n"); help_ref_msg(stderr); h5tools_setstatus(EXIT_FAILURE); goto done; -- cgit v0.12