From 78af4c21cb717d400141d5ed15505e8257497ec9 Mon Sep 17 00:00:00 2001 From: Songyu Lu Date: Wed, 14 Nov 2018 14:31:37 -0600 Subject: HDFFV-10571 Divided by Zero vulnerability. Minor fix: I added an error check to make sure the chunk size is not zero. --- src/H5Dchunk.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/H5Dchunk.c b/src/H5Dchunk.c index 22dc05a..fabae54 100644 --- a/src/H5Dchunk.c +++ b/src/H5Dchunk.c @@ -695,6 +695,10 @@ H5D__chunk_set_info_real(H5O_layout_chunk_t *layout, unsigned ndims, /* Compute the # of chunks in dataset dimensions */ for(u = 0, layout->nchunks = 1, layout->max_nchunks = 1; u < ndims; u++) { + /* Just in case that something goes very wrong, such as file corruption. */ + if(layout->dim[u] == 0) + HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, FAIL, "chunk size must be positive: layout->dim[%u] = %u ", u, layout->dim[u]) + /* Round up to the next integer # of chunks, to accommodate partial chunks */ layout->chunks[u] = ((curr_dims[u] + layout->dim[u]) - 1) / layout->dim[u]; if(H5S_UNLIMITED == max_dims[u]) -- cgit v0.12 From 1d89a55590d106ea4d242402d0cd57a9d1ce5252 Mon Sep 17 00:00:00 2001 From: Songyu Lu Date: Wed, 14 Nov 2018 14:54:03 -0600 Subject: HDFFV-10571: Minor change - revised the comment to be clearer. --- src/H5Dchunk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/H5Dchunk.c b/src/H5Dchunk.c index fabae54..659d580 100644 --- a/src/H5Dchunk.c +++ b/src/H5Dchunk.c @@ -697,7 +697,7 @@ H5D__chunk_set_info_real(H5O_layout_chunk_t *layout, unsigned ndims, for(u = 0, layout->nchunks = 1, layout->max_nchunks = 1; u < ndims; u++) { /* Just in case that something goes very wrong, such as file corruption. */ if(layout->dim[u] == 0) - HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, FAIL, "chunk size must be positive: layout->dim[%u] = %u ", u, layout->dim[u]) + HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, FAIL, "chunk dimension must be positive: layout->dim[%u] = %u ", u, layout->dim[u]) /* Round up to the next integer # of chunks, to accommodate partial chunks */ layout->chunks[u] = ((curr_dims[u] + layout->dim[u]) - 1) / layout->dim[u]; -- cgit v0.12 From c132cb5565de02fd4e1e2cfe8ea7b89b21f785ac Mon Sep 17 00:00:00 2001 From: Songyu Lu Date: Thu, 15 Nov 2018 14:57:26 -0600 Subject: HDFFV-10571: Minor change - adding the error check right after decoding of chunk dimension for safeguard. --- src/H5Dchunk.c | 8 ++++---- src/H5Olayout.c | 7 ++++++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/H5Dchunk.c b/src/H5Dchunk.c index 659d580..00a6b50 100644 --- a/src/H5Dchunk.c +++ b/src/H5Dchunk.c @@ -684,6 +684,7 @@ H5D__chunk_set_info_real(H5O_layout_chunk_t *layout, unsigned ndims, const hsize_t *curr_dims, const hsize_t *max_dims) { unsigned u; /* Local index variable */ + herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_STATIC @@ -695,10 +696,9 @@ H5D__chunk_set_info_real(H5O_layout_chunk_t *layout, unsigned ndims, /* Compute the # of chunks in dataset dimensions */ for(u = 0, layout->nchunks = 1, layout->max_nchunks = 1; u < ndims; u++) { - /* Just in case that something goes very wrong, such as file corruption. */ - if(layout->dim[u] == 0) - HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, FAIL, "chunk dimension must be positive: layout->dim[%u] = %u ", u, layout->dim[u]) - + /* Sanity check */ + HDassert(layout->dim[u] > 0); + /* Round up to the next integer # of chunks, to accommodate partial chunks */ layout->chunks[u] = ((curr_dims[u] + layout->dim[u]) - 1) / layout->dim[u]; if(H5S_UNLIMITED == max_dims[u]) diff --git a/src/H5Olayout.c b/src/H5Olayout.c index 5f16837..11d5d1c 100644 --- a/src/H5Olayout.c +++ b/src/H5Olayout.c @@ -243,9 +243,14 @@ H5O__layout_decode(H5F_t *f, H5O_t H5_ATTR_UNUSED *open_oh, H5F_addr_decode(f, &p, &(mesg->storage.u.chunk.idx_addr)); /* Chunk dimensions */ - for(u = 0; u < mesg->u.chunk.ndims; u++) + for(u = 0; u < mesg->u.chunk.ndims; u++) { UINT32DECODE(p, mesg->u.chunk.dim[u]); + /* Just in case that something goes very wrong, such as file corruption. */ + if(mesg->u.chunk.dim[u] == 0) + HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, NULL, "chunk dimension must be positive: mesg->u.chunk.dim[%u] = %u", u, mesg->u.chunk.dim[u]) + } + /* Compute chunk size */ for(u = 1, mesg->u.chunk.size = mesg->u.chunk.dim[0]; u < mesg->u.chunk.ndims; u++) mesg->u.chunk.size *= mesg->u.chunk.dim[u]; -- cgit v0.12 From 198bc059b0d91aa804efe859f0a8961e3ee0cf8f Mon Sep 17 00:00:00 2001 From: Songyu Lu Date: Thu, 15 Nov 2018 15:05:23 -0600 Subject: HDFFV-10571: Minor change - reformatting the error check. --- src/H5Olayout.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/H5Olayout.c b/src/H5Olayout.c index 11d5d1c..b720f93 100644 --- a/src/H5Olayout.c +++ b/src/H5Olayout.c @@ -248,7 +248,8 @@ H5O__layout_decode(H5F_t *f, H5O_t H5_ATTR_UNUSED *open_oh, /* Just in case that something goes very wrong, such as file corruption. */ if(mesg->u.chunk.dim[u] == 0) - HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, NULL, "chunk dimension must be positive: mesg->u.chunk.dim[%u] = %u", u, mesg->u.chunk.dim[u]) + HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, NULL, "chunk dimension must be positive: mesg->u.chunk.dim[%u] = %u", + u, mesg->u.chunk.dim[u]) } /* Compute chunk size */ -- cgit v0.12 From c923cdad6e515c842f3795a5b6d754ad94021e09 Mon Sep 17 00:00:00 2001 From: Songyu Lu Date: Thu, 15 Nov 2018 16:50:13 -0600 Subject: HDFFV-10571: Minor format changes. --- src/H5Dchunk.c | 1 - src/H5Olayout.c | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/H5Dchunk.c b/src/H5Dchunk.c index 00a6b50..67cfe62 100644 --- a/src/H5Dchunk.c +++ b/src/H5Dchunk.c @@ -684,7 +684,6 @@ H5D__chunk_set_info_real(H5O_layout_chunk_t *layout, unsigned ndims, const hsize_t *curr_dims, const hsize_t *max_dims) { unsigned u; /* Local index variable */ - herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_STATIC diff --git a/src/H5Olayout.c b/src/H5Olayout.c index b720f93..2b65e0c 100644 --- a/src/H5Olayout.c +++ b/src/H5Olayout.c @@ -250,7 +250,7 @@ H5O__layout_decode(H5F_t *f, H5O_t H5_ATTR_UNUSED *open_oh, if(mesg->u.chunk.dim[u] == 0) HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, NULL, "chunk dimension must be positive: mesg->u.chunk.dim[%u] = %u", u, mesg->u.chunk.dim[u]) - } + } /* end for */ /* Compute chunk size */ for(u = 1, mesg->u.chunk.size = mesg->u.chunk.dim[0]; u < mesg->u.chunk.ndims; u++) -- cgit v0.12