Fixed HDFFV-10586, HDFFV-10588, and HDFFV-10684

Description:
    HDFFV-10586 CVE-2018-17434 Divide by zero in h5repack_filters
        Added a check for zero value
    HDFFV-10588 CVE-2018-17437 Memory leak in H5O_dtype_decode_helper
        This is actually an Invalid read issue.  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.
        Added a check to detect attribute name and its length mismatch.  The
        fix does not cover all cases, but it'll reduce the chance of this issue
        when a name length is corrupted or the attribute name is corrupted.
    HDFFV-10684 H5Ewalk does not stop until all errors in the stack are visited
        The test for HDFFV-10588 has revealed a bug in H5Ewalk.
        H5Ewalk did not stop midway even when the call back function returns
        H5_ITER_STOP. This is because a condition is missing from the for
        loops in H5E__walk causing the callback functions unable to stop until
        all the errors in the stack are iterated. Quincey advised on the final
        fix.  In this fix, "status" is switched to "ret_value" and HGOTO_ERROR
        to HERROR, and the for loops won't continue when "ret_value" is not 0.
Platforms tested:
    Linux/64 (jelly)
    Linux/64 (platypus)
    Darwin (osx1011test)

3 files changed, 25 insertions, 23 deletions

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