summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorBinh-Minh Ribler <bmribler@hdfgroup.org>2019-02-01 15:51:51 (GMT)
committerBinh-Minh Ribler <bmribler@hdfgroup.org>2019-02-01 15:51:51 (GMT)
commitae94128b328236cbb36c1da005e2211b5154d2b9 (patch)
tree9f9abc872cee2924176bd745b98b63bf76d391c8
parentd99c9670e576471bc0a822c25f268703cf715869 (diff)
parent7fb50a9700cddfb4aba6d69127590c8460031413 (diff)
downloadhdf5-ae94128b328236cbb36c1da005e2211b5154d2b9.zip
hdf5-ae94128b328236cbb36c1da005e2211b5154d2b9.tar.gz
hdf5-ae94128b328236cbb36c1da005e2211b5154d2b9.tar.bz2
Merge pull request #1514 in HDFFV/hdf5 from ~BMRIBLER/hdf5_1_10_bmr:hdf5_1_10 to hdf5_1_10
HDFFV-10586, HDFFV-10588, HDFFV-10684 * commit '7fb50a9700cddfb4aba6d69127590c8460031413': Renamed data file Fixed HDFFV-10586, HDFFV-10588, and HDFFV-10684
-rw-r--r--MANIFEST1
-rw-r--r--release_docs/RELEASE.txt34
-rw-r--r--src/H5E.c6
-rw-r--r--src/H5Eint.c37
-rw-r--r--src/H5Oattr.c5
-rw-r--r--test/CMakeTests.cmake1
-rw-r--r--test/corrupted_name_len.h5bin0 -> 82816 bytes
-rw-r--r--test/titerate.c101
-rw-r--r--tools/src/h5repack/h5repack_filters.c6
9 files changed, 163 insertions, 28 deletions
diff --git a/MANIFEST b/MANIFEST
index 33038a3..5597c64 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -1035,6 +1035,7 @@
./test/le_extlink2.h5
./test/lheap.c
./test/links.c
+./test/corrupted_name_len.h5
./test/mergemsg.h5
./test/mf.c
./test/mount.c
diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt
index c7c3be3..ffb3092 100644
--- a/release_docs/RELEASE.txt
+++ b/release_docs/RELEASE.txt
@@ -396,14 +396,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 a memory leak, the actual issue
+ was 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:
diff --git a/src/H5E.c b/src/H5E.c
index eecf6e7..84e4b1a 100644
--- a/src/H5E.c
+++ b/src/H5E.c
@@ -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..023024e 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
+ corrupted_name_len.h5
mergemsg.h5
multi_file_v16-r.h5
multi_file_v16-s.h5
diff --git a/test/corrupted_name_len.h5 b/test/corrupted_name_len.h5
new file mode 100644
index 0000000..b5980b7
--- /dev/null
+++ b/test/corrupted_name_len.h5
Binary files differ
diff --git a/test/titerate.c b/test/titerate.c
index de652a7..093e8b5 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 "corrupted_name_len.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++) {