From e1b59919bb96f68f3b372a73790ecbe4ac3b395a Mon Sep 17 00:00:00 2001 From: Binh-Minh Ribler Date: Sun, 6 Jan 2019 01:44:40 -0600 Subject: HDFFV-10578 and HDFFV-10676 Description: HDFFV-10578 - CVE-2018-17234 The file has some issue, however, there was a bug in h5dump that caused memory leaks after the problem in the file was encountered. The bug was that an if statement was missing in the function table_list_add() resulting in the memory not being freed at a later time. After the fix had been applied, there were no more leaks after h5dump detected the issue in the file and reported the error. In H5O__chunk_deserialize, replaced an assert with an if statement and reporting error, per Neil's recommendation HDFFV-10676 - CVE-2018-13873 Also in H5O__chunk_deserialize, added an assertion to detect out of bound ids --- src/H5Ocache.c | 5 ++++- tools/src/h5dump/h5dump.c | 7 ++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/H5Ocache.c b/src/H5Ocache.c index fba4f6e..034048f 100644 --- a/src/H5Ocache.c +++ b/src/H5Ocache.c @@ -1390,7 +1390,8 @@ H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t len, const uint8_t *image /* Message size */ UINT16DECODE(chunk_image, mesg_size); - HDassert(mesg_size == H5O_ALIGN_OH(oh, mesg_size)); + if(mesg_size != H5O_ALIGN_OH(oh, mesg_size)) + HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "message not aligned") /* Message flags */ flags = *chunk_image++; @@ -1402,6 +1403,8 @@ H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t len, const uint8_t *image HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "bad flag combination for message") if((flags & H5O_MSG_FLAG_WAS_UNKNOWN) && !(flags & H5O_MSG_FLAG_MARK_IF_UNKNOWN)) HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "bad flag combination for message") + + HDassert(id < NELMTS(H5O_msg_class_g)); if((flags & H5O_MSG_FLAG_SHAREABLE) && H5O_msg_class_g[id] && !(H5O_msg_class_g[id]->share_flags & H5O_SHARE_IS_SHARABLE)) diff --git a/tools/src/h5dump/h5dump.c b/tools/src/h5dump/h5dump.c index b9e37e8..5267188 100644 --- a/tools/src/h5dump/h5dump.c +++ b/tools/src/h5dump/h5dump.c @@ -407,9 +407,10 @@ table_list_add(hid_t oid, unsigned long file_no) } if(init_objs(oid, &info, &table_list.tables[idx].group_table, &table_list.tables[idx].dset_table, &table_list.tables[idx].type_table) < 0) { - H5Idec_ref(oid); - table_list.nused--; - return -1; + if (H5Idec_ref(oid) < 0) { + table_list.nused--; + return -1; + } } #ifdef H5DUMP_DEBUG -- cgit v0.12 From 820d8e34c5845f85690a4d65ca31b21fdcfd23cf Mon Sep 17 00:00:00 2001 From: Binh-Minh Ribler Date: Sun, 6 Jan 2019 21:42:16 -0600 Subject: Updated per review Description: HDFFV-10676 - CVE-2018-13873 Changed the new assert to if statement, per Dana's comment. Platforms tested: Linux/64 (jelly) --- src/H5Ocache.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/H5Ocache.c b/src/H5Ocache.c index 034048f..34277d1 100644 --- a/src/H5Ocache.c +++ b/src/H5Ocache.c @@ -1404,7 +1404,8 @@ H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t len, const uint8_t *image if((flags & H5O_MSG_FLAG_WAS_UNKNOWN) && !(flags & H5O_MSG_FLAG_MARK_IF_UNKNOWN)) HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "bad flag combination for message") - HDassert(id < NELMTS(H5O_msg_class_g)); + if(id >= NELMTS(H5O_msg_class_g)) + HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "invalid type of current message") if((flags & H5O_MSG_FLAG_SHAREABLE) && H5O_msg_class_g[id] && !(H5O_msg_class_g[id]->share_flags & H5O_SHARE_IS_SHARABLE)) -- cgit v0.12 From 463515f7db9b5dd32c6746488e18760525d33568 Mon Sep 17 00:00:00 2001 From: Binh-Minh Ribler Date: Mon, 7 Jan 2019 14:00:56 -0600 Subject: Removed the previous change in table_list_add() --- tools/src/h5dump/h5dump.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/src/h5dump/h5dump.c b/tools/src/h5dump/h5dump.c index 5267188..52158fb 100644 --- a/tools/src/h5dump/h5dump.c +++ b/tools/src/h5dump/h5dump.c @@ -407,10 +407,9 @@ table_list_add(hid_t oid, unsigned long file_no) } if(init_objs(oid, &info, &table_list.tables[idx].group_table, &table_list.tables[idx].dset_table, &table_list.tables[idx].type_table) < 0) { - if (H5Idec_ref(oid) < 0) { + H5Idec_ref(oid); table_list.nused--; return -1; - } } #ifdef H5DUMP_DEBUG -- cgit v0.12 From 5b721a060e87092afc9956c39381aba370f37bb4 Mon Sep 17 00:00:00 2001 From: Binh-Minh Ribler Date: Mon, 7 Jan 2019 14:03:22 -0600 Subject: Removed previous change in table_list_add(). --- tools/src/h5dump/h5dump.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/src/h5dump/h5dump.c b/tools/src/h5dump/h5dump.c index 52158fb..b9e37e8 100644 --- a/tools/src/h5dump/h5dump.c +++ b/tools/src/h5dump/h5dump.c @@ -407,9 +407,9 @@ table_list_add(hid_t oid, unsigned long file_no) } if(init_objs(oid, &info, &table_list.tables[idx].group_table, &table_list.tables[idx].dset_table, &table_list.tables[idx].type_table) < 0) { - H5Idec_ref(oid); - table_list.nused--; - return -1; + H5Idec_ref(oid); + table_list.nused--; + return -1; } #ifdef H5DUMP_DEBUG -- cgit v0.12 From c092f9167cb47f5f958eb03e11680b81837d60b2 Mon Sep 17 00:00:00 2001 From: Binh-Minh Ribler Date: Mon, 7 Jan 2019 14:07:25 -0600 Subject: Removed the previous change in H5O__chunk_deserialize() --- src/H5Ocache.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/H5Ocache.c b/src/H5Ocache.c index 34277d1..085d3bc 100644 --- a/src/H5Ocache.c +++ b/src/H5Ocache.c @@ -1404,8 +1404,6 @@ H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t len, const uint8_t *image if((flags & H5O_MSG_FLAG_WAS_UNKNOWN) && !(flags & H5O_MSG_FLAG_MARK_IF_UNKNOWN)) HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "bad flag combination for message") - if(id >= NELMTS(H5O_msg_class_g)) - HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "invalid type of current message") if((flags & H5O_MSG_FLAG_SHAREABLE) && H5O_msg_class_g[id] && !(H5O_msg_class_g[id]->share_flags & H5O_SHARE_IS_SHARABLE)) -- cgit v0.12 From 6c7462b1a913e4b7fb4fb76c4a3097a83a517720 Mon Sep 17 00:00:00 2001 From: Binh-Minh Ribler Date: Mon, 7 Jan 2019 14:09:30 -0600 Subject: Removed previous change in H5O__chunk_deserialize(). --- src/H5Ocache.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/H5Ocache.c b/src/H5Ocache.c index 085d3bc..d32a217 100644 --- a/src/H5Ocache.c +++ b/src/H5Ocache.c @@ -1403,7 +1403,6 @@ H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t len, const uint8_t *image HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "bad flag combination for message") if((flags & H5O_MSG_FLAG_WAS_UNKNOWN) && !(flags & H5O_MSG_FLAG_MARK_IF_UNKNOWN)) HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "bad flag combination for message") - if((flags & H5O_MSG_FLAG_SHAREABLE) && H5O_msg_class_g[id] && !(H5O_msg_class_g[id]->share_flags & H5O_SHARE_IS_SHARABLE)) -- cgit v0.12 From 78d0564c2afa2241899a2bd43aa1bd6af7a3fda2 Mon Sep 17 00:00:00 2001 From: Binh-Minh Ribler Date: Mon, 7 Jan 2019 20:36:34 -0600 Subject: Refixed HDFFV-10578 Description: Applied Neil's fix for this issue after removing previous attempt. The resources are now released in init_objs() when failure occurs there. Neil will fix HDFFV-10676 separately. Platforms tested: Linux/64 (jelly) Linux/64 (platypus) Darwin (osx1010test) --- src/H5VM.c | 2 +- tools/lib/h5tools_utils.c | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/H5VM.c b/src/H5VM.c index 4c0b837..452d378 100644 --- a/src/H5VM.c +++ b/src/H5VM.c @@ -1548,7 +1548,7 @@ done: * * Purpose: Given source and destination buffers in memory (SRC & DST) * copy sequences of from the source buffer into the destination - * buffer. Each set of sequnces has an array of lengths, an + * buffer. Each set of sequences has an array of lengths, an * array of offsets, the maximum number of sequences and the * current sequence to start at in the sequence. * diff --git a/tools/lib/h5tools_utils.c b/tools/lib/h5tools_utils.c index 8ac0d32..b33ba13 100644 --- a/tools/lib/h5tools_utils.c +++ b/tools/lib/h5tools_utils.c @@ -561,6 +561,8 @@ herr_t init_objs(hid_t fid, find_objs_t *info, table_t **group_table, table_t **dset_table, table_t **type_table) { + herr_t ret_value = SUCCEED; + /* Initialize the tables */ init_table(group_table); init_table(dset_table); @@ -573,7 +575,20 @@ init_objs(hid_t fid, find_objs_t *info, table_t **group_table, info->dset_table = *dset_table; /* Find all shared objects */ - return(h5trav_visit(fid, "/", TRUE, TRUE, find_objs_cb, NULL, info, H5O_INFO_BASIC)); + if((ret_value = h5trav_visit(fid, "/", TRUE, TRUE, find_objs_cb, NULL, info, H5O_INFO_BASIC)) < 0) + HGOTO_ERROR(FAIL, H5E_tools_min_id_g, "finding shared objects failed") + +done: + /* Release resources */ + if(ret_value == FAIL) { + free_table(*group_table); + info->group_table = NULL; + free_table(*type_table); + info->type_table = NULL; + free_table(*dset_table); + info->dset_table = NULL; + } + return ret_value; } -- cgit v0.12 From 90d13bef33f9e2e80b23996a0c39f16f7c34ecf8 Mon Sep 17 00:00:00 2001 From: Binh-Minh Ribler Date: Mon, 7 Jan 2019 20:46:55 -0600 Subject: Fixed typo Platforms tested: Darwin (osx1010test) --- tools/lib/h5tools_utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/lib/h5tools_utils.c b/tools/lib/h5tools_utils.c index b33ba13..e7e017f 100644 --- a/tools/lib/h5tools_utils.c +++ b/tools/lib/h5tools_utils.c @@ -580,7 +580,7 @@ init_objs(hid_t fid, find_objs_t *info, table_t **group_table, done: /* Release resources */ - if(ret_value == FAIL) { + if(ret_value < 0) { free_table(*group_table); info->group_table = NULL; free_table(*type_table); -- cgit v0.12