201 lines
9.6 KiB
Diff
201 lines
9.6 KiB
Diff
|
|
From 02d03b4624122955ee3de635699a4e3880fea377 Mon Sep 17 00:00:00 2001
|
||
|
|
From: Binh-Minh Ribler <bmribler@hdfgroup.org>
|
||
|
|
Date: Wed, 30 Jan 2019 20:04:30 -0600
|
||
|
|
Subject: [PATCH] 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)
|
||
|
|
---
|
||
|
|
src/H5E.c | 6 ++---
|
||
|
|
src/H5Eint.c | 37 ++++++++++++++-----------------
|
||
|
|
src/H5Oattr.c | 5 +++++
|
||
|
|
tools/h5repack/h5repack_filters.c | 6 +++--
|
||
|
|
4 files changed, 29 insertions(+), 25 deletions(-)
|
||
|
|
|
||
|
|
diff --git a/src/H5E.c b/src/H5E.c
|
||
|
|
index a94c5bc..e8da166 100644
|
||
|
|
--- a/src/H5E.c
|
||
|
|
+++ b/src/H5E.c
|
||
|
|
@@ -1471,7 +1471,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
|
||
|
|
@@ -1553,8 +1553,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 636866b..85edb90 100644
|
||
|
|
--- a/src/H5Eint.c
|
||
|
|
+++ b/src/H5Eint.c
|
||
|
|
@@ -442,13 +442,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
|
||
|
|
@@ -533,9 +533,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
|
||
|
|
|
||
|
|
@@ -553,9 +552,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;
|
||
|
|
@@ -564,12 +562,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;
|
||
|
|
@@ -578,12 +576,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!");
|
||
|
|
@@ -592,19 +590,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 149f04a..a8d2a31 100644
|
||
|
|
--- a/src/H5Oattr.c
|
||
|
|
+++ b/src/H5Oattr.c
|
||
|
|
@@ -175,6 +175,11 @@ H5O_attr_decode(H5F_t *f, hid_t dxpl_id, H5O_t *open_oh, unsigned H5_ATTR_UNUSED
|
||
|
|
/* 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/tools/h5repack/h5repack_filters.c b/tools/h5repack/h5repack_filters.c
|
||
|
|
index 523f81a..1eea3e9 100644
|
||
|
|
--- a/tools/h5repack/h5repack_filters.c
|
||
|
|
+++ b/tools/h5repack/h5repack_filters.c
|
||
|
|
@@ -333,12 +333,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++) {
|
||
|
|
--
|
||
|
|
2.23.0
|
||
|
|
|