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

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
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++) {