diff options
-rw-r--r-- | MANIFEST | 1 | ||||
-rw-r--r-- | release_docs/RELEASE.txt | 34 | ||||
-rw-r--r-- | src/H5E.c | 6 | ||||
-rw-r--r-- | src/H5Eint.c | 37 | ||||
-rw-r--r-- | src/H5Oattr.c | 5 | ||||
-rw-r--r-- | test/CMakeTests.cmake | 1 | ||||
-rw-r--r-- | test/memleak_H5O_dtype_decode_helper_H5Odtype.h5 | bin | 0 -> 82816 bytes | |||
-rw-r--r-- | test/titerate.c | 101 | ||||
-rw-r--r-- | tools/src/h5repack/h5repack_filters.c | 6 |
9 files changed, 163 insertions, 28 deletions
@@ -1036,6 +1036,7 @@ ./test/le_extlink2.h5 ./test/lheap.c ./test/links.c +./test/memleak_H5O_dtype_decode_helper_H5Odtype.h5 ./test/mergemsg.h5 ./test/mf.c ./test/mount.c diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 836d1a1..5ff697c 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -355,14 +355,42 @@ Bug Fixes since HDF5-1.10.3 release (JTH - 2018/08/25, HDFFV-10501) - - There was an incorrect protection against division by zero reported - to The HDF Group as issue #CVE-2018-17233. + - There was missing protection against division by zero reported to + The HDF Group as issue #CVE-2018-17233. Protection against division by zero was added to address the issue #CVE-2018-17233. In addition, several similar occurrences in the same file were fixed as well. - (BMR - 2018/02/26, HDFFV-10577) + (BMR - 2018/12/23, HDFFV-10577) + + - There was missing protection against division by zero reported to + The HDF Group as issue #CVE-2018-17434. + + Protection against division by zero was added to address the issue + #CVE-2018-17434. + + (BMR - 2019/01/29, HDFFV-10586) + + - The issue CVE-2018-17437 was reported to The HDF Group + + Although CVE-2018-17437 reported memory leak, the actual issues + were invalid read. It was found that the attribute name length + in an attribute message was corrupted, which caused the buffer + pointer to be advanced too far and later caused an invalid read. + + A check was added to detect when the attribute name or its length + was corrupted and report the potential of data corruption. + + (BMR - 2019/01/29, HDFFV-10588) + + - H5Ewalk did not stop when it was supposed to + + H5Ewalk was supposed to stop when the callback function stopped + even though the errors in the stack were not all visited, but it + did not. This problem is now fixed. + + (BMR - 2019/01/29, HDFFV-10684) Java Library: @@ -1481,7 +1481,7 @@ done: * * Purpose: Prints the error stack in some default way. This is just a * convenience function for H5Ewalk() with a function that - * prints error messages. Users are encouraged to write there + * prints error messages. Users are encouraged to write their * own more specific error handlers. * * Return: Non-negative on success/Negative on failure @@ -1563,8 +1563,8 @@ H5Ewalk2(hid_t err_stack, H5E_direction_t direction, H5E_walk2_t stack_func, voi /* Walk the error stack */ op.vers = 2; op.u.func2 = stack_func; - if(H5E_walk(estack, direction, &op, client_data) < 0) - HGOTO_ERROR(H5E_ERROR, H5E_CANTLIST, FAIL, "can't walk error stack") + if((ret_value = H5E_walk(estack, direction, &op, client_data)) < 0) + HERROR(H5E_ERROR, H5E_CANTLIST, "can't walk error stack"); done: FUNC_LEAVE_API(ret_value) diff --git a/src/H5Eint.c b/src/H5Eint.c index 15953af..ff2a860 100644 --- a/src/H5Eint.c +++ b/src/H5Eint.c @@ -419,13 +419,13 @@ done: /*------------------------------------------------------------------------- * Function: H5E_print * - * Purpose: Private function to print the error stack in some default + * Purpose: Private function to print the error stack in some default * way. This is just a convenience function for H5Ewalk() and * H5Ewalk2() with a function that prints error messages. - * Users are encouraged to write there own more specific error + * Users are encouraged to write their own more specific error * handlers. * - * Return: Non-negative on success/Negative on failure + * Return: Non-negative on success/Negative on failure * * Programmer: Robb Matzke * Friday, February 27, 1998 @@ -510,9 +510,8 @@ herr_t H5E_walk(const H5E_t *estack, H5E_direction_t direction, const H5E_walk_op_t *op, void *client_data) { - int i; /* Local index variable */ - herr_t status; /* Status from callback function */ - herr_t ret_value = SUCCEED; /* Return value */ + int i; /* Local index variable */ + herr_t ret_value = H5_ITER_CONT; /* Return value */ FUNC_ENTER_NOAPI_NOINIT @@ -530,9 +529,8 @@ H5E_walk(const H5E_t *estack, H5E_direction_t direction, const H5E_walk_op_t *op if(op->u.func1) { H5E_error1_t old_err; - status = SUCCEED; if(H5E_WALK_UPWARD == direction) { - for(i = 0; i < (int)estack->nused && status >= 0; i++) { + for(i = 0; i < (int)estack->nused && ret_value == H5_ITER_CONT; i++) { /* Point to each error record on the stack and pass it to callback function.*/ old_err.maj_num = estack->slot[i].maj_num; old_err.min_num = estack->slot[i].min_num; @@ -541,12 +539,12 @@ H5E_walk(const H5E_t *estack, H5E_direction_t direction, const H5E_walk_op_t *op old_err.desc = estack->slot[i].desc; old_err.line = estack->slot[i].line; - status = (op->u.func1)(i, &old_err, client_data); + ret_value = (op->u.func1)(i, &old_err, client_data); } /* end for */ } /* end if */ else { H5_CHECK_OVERFLOW(estack->nused - 1, size_t, int); - for(i = (int)(estack->nused - 1); i >= 0 && status >= 0; i--) { + for(i = (int)(estack->nused - 1); i >= 0 && ret_value == H5_ITER_CONT; i--) { /* Point to each error record on the stack and pass it to callback function.*/ old_err.maj_num = estack->slot[i].maj_num; old_err.min_num = estack->slot[i].min_num; @@ -555,12 +553,12 @@ H5E_walk(const H5E_t *estack, H5E_direction_t direction, const H5E_walk_op_t *op old_err.desc = estack->slot[i].desc; old_err.line = estack->slot[i].line; - status = (op->u.func1)((int)(estack->nused - (size_t)(i + 1)), &old_err, client_data); + ret_value = (op->u.func1)((int)(estack->nused - (size_t)(i + 1)), &old_err, client_data); } /* end for */ } /* end else */ - if(status < 0) - HGOTO_ERROR(H5E_ERROR, H5E_CANTLIST, FAIL, "can't walk error stack") + if(ret_value < 0) + HERROR(H5E_ERROR, H5E_CANTLIST, "can't walk error stack"); } /* end if */ #else /* H5_NO_DEPRECATED_SYMBOLS */ HDassert(0 && "version 1 error stack walk without deprecated symbols!"); @@ -569,19 +567,18 @@ H5E_walk(const H5E_t *estack, H5E_direction_t direction, const H5E_walk_op_t *op else { HDassert(op->vers == 2); if(op->u.func2) { - status = SUCCEED; if(H5E_WALK_UPWARD == direction) { - for(i = 0; i < (int)estack->nused && status >= 0; i++) - status = (op->u.func2)((unsigned)i, estack->slot + i, client_data); + for(i = 0; i < (int)estack->nused && ret_value == H5_ITER_CONT; i++) + ret_value = (op->u.func2)((unsigned)i, estack->slot + i, client_data); } /* end if */ else { H5_CHECK_OVERFLOW(estack->nused - 1, size_t, int); - for(i = (int)(estack->nused - 1); i >= 0 && status >= 0; i--) - status = (op->u.func2)((unsigned)(estack->nused - (size_t)(i + 1)), estack->slot + i, client_data); + for(i = (int)(estack->nused - 1); i >= 0 && ret_value == H5_ITER_CONT; i--) + ret_value = (op->u.func2)((unsigned)(estack->nused - (size_t)(i + 1)), estack->slot + i, client_data); } /* end else */ - if(status < 0) - HGOTO_ERROR(H5E_ERROR, H5E_CANTLIST, FAIL, "can't walk error stack") + if(ret_value < 0) + HERROR(H5E_ERROR, H5E_CANTLIST, "can't walk error stack"); } /* end if */ } /* end else */ diff --git a/src/H5Oattr.c b/src/H5Oattr.c index a62a3a3..1957554 100644 --- a/src/H5Oattr.c +++ b/src/H5Oattr.c @@ -177,6 +177,11 @@ H5O_attr_decode(H5F_t *f, H5O_t *open_oh, unsigned H5_ATTR_UNUSED mesg_flags, /* Decode and store the name */ if(NULL == (attr->shared->name = H5MM_strdup((const char *)p))) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed") + + /* Make an attempt to detect corrupted name or name length - HDFFV-10588 */ + if(name_len != (HDstrlen(attr->shared->name) + 1)) + HGOTO_ERROR(H5E_ATTR, H5E_CANTDECODE, NULL, "attribute name has different length than stored length") + if(attr->shared->version < H5O_ATTR_VERSION_2) p += H5O_ALIGN_OLD(name_len); /* advance the memory pointer */ else diff --git a/test/CMakeTests.cmake b/test/CMakeTests.cmake index 881bdb8..37d3ad3 100644 --- a/test/CMakeTests.cmake +++ b/test/CMakeTests.cmake @@ -171,6 +171,7 @@ set (HDF5_REFERENCE_TEST_FILES le_data.h5 le_extlink1.h5 le_extlink2.h5 + memleak_H5O_dtype_decode_helper_H5Odtype.h5 mergemsg.h5 multi_file_v16-r.h5 multi_file_v16-s.h5 diff --git a/test/memleak_H5O_dtype_decode_helper_H5Odtype.h5 b/test/memleak_H5O_dtype_decode_helper_H5Odtype.h5 Binary files differnew file mode 100644 index 0000000..b5980b7 --- /dev/null +++ b/test/memleak_H5O_dtype_decode_helper_H5Odtype.h5 diff --git a/test/titerate.c b/test/titerate.c index de652a7..5fad1b4 100644 --- a/test/titerate.c +++ b/test/titerate.c @@ -20,6 +20,7 @@ *************************************************************/ #include "testhdf5.h" +#include "H5srcdir.h" #define DATAFILE "titerate.h5" @@ -53,6 +54,17 @@ typedef struct { iter_enum command; /* The type of return value */ } iter_info; +/* Definition for test_corrupted_attnamelen */ +#define CORRUPTED_ATNAMELEN_FILE "memleak_H5O_dtype_decode_helper_H5Odtype.h5" +#define DSET_NAME "image" +typedef struct searched_err_t { + char message[256]; + bool found; +} searched_err_t; + +/* Call back function for test_corrupted_attnamelen */ +static int find_err_msg_cb(unsigned n, const H5E_error2_t *err_desc, void *_client_data); + /* Local functions */ int iter_strcmp(const void *s1, const void *s2); int iter_strcmp2(const void *s1, const void *s2); @@ -915,6 +927,92 @@ static void test_links(hid_t fapl) CHECK(ret, FAIL, "H5Fclose"); } /* test_links() */ +/*------------------------------------------------------------------------- + * Function: find_err_msg_cb + * + * Purpose: Callback function to find the given error message. + * Helper function for test_corrupted_attnamelen(). + * + * Return: H5_ITER_STOP when the message is found + * H5_ITER_CONT, otherwise + * + *------------------------------------------------------------------------- + */ +static int +find_err_msg_cb(unsigned n, const H5E_error2_t *err_desc, void *_client_data) +{ + int status = H5_ITER_CONT; + searched_err_t *searched_err = (searched_err_t *)_client_data; + + if (searched_err == NULL) + return -1; + + /* If the searched error message is found, stop the iteration */ + if (err_desc->desc != NULL && strcmp(err_desc->desc, searched_err->message) == 0) + { + searched_err->found = true; + status = H5_ITER_STOP; + } + return status; +} /* end find_err_msg_cb() */ + +/************************************************************************** +** +** test_corrupted_attnamelen(): Test the fix for the JIRA issue HDFFV-10588, +** where corrupted attribute's name length can be +** detected and invalid read can be avoided. +** +**************************************************************************/ +static void test_corrupted_attnamelen(void) +{ + hid_t fid = -1; /* File ID */ + hid_t did = -1; /* Dataset ID */ + searched_err_t err_caught; /* Data to be passed to callback func */ + int err_status; /* Status returned by H5Aiterate2 */ + herr_t ret; /* Return value */ + const char *testfile = H5_get_srcdir_filename(CORRUPTED_ATNAMELEN_FILE); /* Corrected test file name */ + + const char *err_message = "attribute name has different length than stored length"; + /* the error message produced when the failure occurs */ + + /* Output message about test being performed */ + MESSAGE(5, ("Testing the Handling of Corrupted Attribute's Name Length\n")); + + fid = H5Fopen(testfile, H5F_ACC_RDONLY, H5P_DEFAULT); + CHECK(fid, FAIL, "H5Fopen"); + + /* Open the dataset */ + did = H5Dopen2(fid, DSET_NAME, H5P_DEFAULT); + CHECK(did, FAIL, "H5Dopen2"); + + /* Call H5Aiterate2 to trigger the failure in HDFFV-10588. Failure should + occur in the decoding stage, so some arguments are not needed. */ + err_status = H5Aiterate2(did, H5_INDEX_NAME, H5_ITER_INC, NULL, NULL, NULL); + + /* Make sure the intended error was caught */ + if(err_status == -1) + { + /* Initialize client data */ + HDstrcpy(err_caught.message, err_message); + err_caught.found = false; + + /* Look for the correct error message */ + ret = H5Ewalk2(H5E_DEFAULT, H5E_WALK_UPWARD, find_err_msg_cb, &err_caught); + CHECK(ret, FAIL, "H5Ewalk2"); + + /* Fail if the indicated message is not found */ + CHECK(err_caught.found, false, "test_corrupted_attnamelen: Expected error not found"); + } + + /* Close the dataset and file */ + ret = H5Dclose(did); + CHECK(ret, FAIL, "H5Dclose"); + + ret = H5Fclose(fid); + CHECK(ret, FAIL, "H5Fclose"); + +} /* test_corrupted_attnamelen() */ + /**************************************************************** ** ** test_iterate(): Main iteration testing routine. @@ -951,6 +1049,9 @@ test_iterate(void) test_links(new_format ? fapl2 : fapl); /* Test soft and hard link iteration */ } /* end for */ + /* Test the fix for issue HDFFV-10588 */ + test_corrupted_attnamelen(); + /* Close FAPLs */ ret = H5Pclose(fapl); CHECK(ret, FAIL, "H5Pclose"); diff --git a/tools/src/h5repack/h5repack_filters.c b/tools/src/h5repack/h5repack_filters.c index 0092abc..3d9472a 100644 --- a/tools/src/h5repack/h5repack_filters.c +++ b/tools/src/h5repack/h5repack_filters.c @@ -338,12 +338,14 @@ int apply_filters(const char* name, /* object name from traverse list */ sm_nbytes = msize; for (i = rank; i > 0; --i) { - hsize_t size = H5TOOLS_BUFSIZE / sm_nbytes; + hsize_t size = 0; + if(sm_nbytes == 0) + HGOTO_ERROR(FAIL, H5E_tools_min_id_g, "number of bytes per stripmine must be > 0"); + size = H5TOOLS_BUFSIZE / sm_nbytes; if (size == 0) /* datum size > H5TOOLS_BUFSIZE */ size = 1; sm_size[i - 1] = MIN(dims[i - 1], size); sm_nbytes *= sm_size[i - 1]; - HDassert(sm_nbytes > 0); } for (i = 0; i < rank; i++) { |