From dafc7285bb1df4a6529a64c215c5de4017016d24 Mon Sep 17 00:00:00 2001 From: bmribler <39579120+bmribler@users.noreply.github.com> Date: Fri, 19 Mar 2021 09:15:03 -0400 Subject: Fixed HDFFV-10480 (CVE-2018-11206) and HDFFV-11159 (CVE-2018-14033) (#405) * Fixed HDFFV-10480 (CVE-2018-11206) and HDFFV-11159 (CVE-2018-14033) Description Checked against buffer size to prevent segfault, in case of data corruption. + HDFFV-11159 CVE-2018-14033 Buffer over-read in H5O_layout_decode + HDFFV-10480 CVE-2018-11206 Buffer over-read in H5O_fill_new[/old]_decode Platforms tested: Linux/64 (jelly) * Accidentally left in another occurrence of the previous patch from user after a more correct fix was applied, that is the check now accounted for the previous advance of the buffer pointer. Removed it. * Typo * Fixed format issues. * Added test. * Changed arguments to ADD_H5_TEST * Fixing arguments to ADD_H5_TEST again. * Fixing arguments again. * Took out the CMake changes until Allen can help. * Added files: tCVE_2018_11206_fill_old.h5 tCVE_2018_11206_fill_new.h5 * Revert "Took out the CMake changes until Allen can help." This reverts commit c21324d6e0044994c5cd24b0671e7d1dd41096cc. * Revert "Fixing arguments again." This reverts commit 5832a70674339e4b524749adde5a181f8c3a446a. * Revert "Fixing arguments to ADD_H5_TEST again." This reverts commit b45de823c22ce83a388d46466ef7c04b66ff05ed. * Revert "Changed arguments to ADD_H5_TEST" This reverts commit 16719824f57e52158451ddd261788c0dcaa3ec55. * Added first argument to ADD_H5_TEST for HDFFV-10480 fix. * Changed argument 0 to 1 * Revert "Changed argument 0 to 1" This reverts commit b343d6613ba681b43248dd5820e96389984ebcf7. * Revert "Added first argument to ADD_H5_TEST for HDFFV-10480 fix." This reverts commit b8a0f9a9e8ec8e6c6ff38d33195d63edff76a563. * Added first argument and corrected the second. * Updated fixes for HDFFV-10480 and HDFFV-11159/HDFFV-11049 * Improved error messages. --- MANIFEST | 2 ++ release_docs/RELEASE.txt | 20 ++++++++++++++++ src/H5Ofill.c | 27 +++++++++++++-------- src/H5Olayout.c | 29 +++++++++++++++-------- tools/test/h5dump/CMakeTests.cmake | 6 +++++ tools/test/h5dump/testh5dump.sh.in | 35 ++++++++++++++++++++++++++++ tools/testfiles/tCVE_2018_11206_fill_new.h5 | Bin 0 -> 1752 bytes tools/testfiles/tCVE_2018_11206_fill_old.h5 | Bin 0 -> 2560 bytes 8 files changed, 99 insertions(+), 20 deletions(-) create mode 100644 tools/testfiles/tCVE_2018_11206_fill_new.h5 create mode 100644 tools/testfiles/tCVE_2018_11206_fill_old.h5 diff --git a/MANIFEST b/MANIFEST index 5d1eb49..f8c64ab 100644 --- a/MANIFEST +++ b/MANIFEST @@ -2134,6 +2134,8 @@ ./tools/testfiles/twithddl.exp ./tools/testfiles/twithddlfile.ddl ./tools/testfiles/twithddlfile.exp +./tools/testfiles/tCVE_2018_11206_fill_old.h5 +./tools/testfiles/tCVE_2018_11206_fill_new.h5 # h5dump test error files ./tools/test/h5dump/errfiles/filter_fail.err diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 0d3c2a5..242d3e6 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -707,6 +707,26 @@ Bug Fixes since HDF5-1.12.0 release =================================== Library ------- + - Fixed CVE-2018-17435 + + The tool h5dump produced a segfault when the size of a fill value + message was corrupted and caused a buffer overflow. + + The problem was fixed by verifying the fill value's size + against the buffer size before attempting to access the buffer. + + (BMR - 2021/03/15, HDFFV-10480) + + - Fixed CVE-2018-14033 (same issue as CVE-2020-10811) + + The tool h5dump produced a segfault when the storage size message + was corrupted and caused a buffer overflow. + + The problem was fixed by verifying the storage size against the + buffer size before attempting to access the buffer. + + (BMR - 2021/03/15, HDFFV-11159/HDFFV-11049) + - Remove underscores on header file guards Header file guards used a variety of underscores at the beginning of the define. diff --git a/src/H5Ofill.c b/src/H5Ofill.c index 695673f..5068039 100644 --- a/src/H5Ofill.c +++ b/src/H5Ofill.c @@ -196,8 +196,9 @@ H5O__fill_new_decode(H5F_t H5_ATTR_UNUSED *f, H5O_t H5_ATTR_UNUSED *open_oh, unsigned H5_ATTR_UNUSED mesg_flags, unsigned H5_ATTR_UNUSED *ioflags, size_t p_size, const uint8_t *p) { - H5O_fill_t *fill = NULL; - void * ret_value = NULL; /* Return value */ + H5O_fill_t * fill = NULL; + const uint8_t *p_end = p + p_size - 1; /* End of the p buffer */ + void * ret_value = NULL; /* Return value */ FUNC_ENTER_STATIC @@ -228,8 +229,11 @@ H5O__fill_new_decode(H5F_t H5_ATTR_UNUSED *f, H5O_t H5_ATTR_UNUSED *open_oh, INT32DECODE(p, fill->size); if (fill->size > 0) { H5_CHECK_OVERFLOW(fill->size, ssize_t, size_t); - if ((size_t)fill->size > p_size) - HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "destination buffer too small") + + /* Ensure that fill size doesn't exceed buffer size, due to possible data corruption */ + if (p + fill->size - 1 > p_end) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "fill size exceeds buffer size") + if (NULL == (fill->buf = H5MM_malloc((size_t)fill->size))) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed for fill value") H5MM_memcpy(fill->buf, p, (size_t)fill->size); @@ -311,10 +315,11 @@ static void * H5O__fill_old_decode(H5F_t *f, H5O_t *open_oh, unsigned H5_ATTR_UNUSED mesg_flags, unsigned H5_ATTR_UNUSED *ioflags, size_t p_size, const uint8_t *p) { - H5O_fill_t *fill = NULL; /* Decoded fill value message */ - htri_t exists = FALSE; - H5T_t * dt = NULL; - void * ret_value = NULL; /* Return value */ + H5O_fill_t * fill = NULL; /* Decoded fill value message */ + htri_t exists = FALSE; + H5T_t * dt = NULL; + const uint8_t *p_end = p + p_size - 1; /* End of the p buffer */ + void * ret_value = NULL; /* Return value */ FUNC_ENTER_STATIC @@ -335,8 +340,10 @@ H5O__fill_old_decode(H5F_t *f, H5O_t *open_oh, unsigned H5_ATTR_UNUSED mesg_flag /* Only decode the fill value itself if there is one */ if (fill->size > 0) { H5_CHECK_OVERFLOW(fill->size, ssize_t, size_t); - if ((size_t)fill->size > p_size) - HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "destination buffer too small") + + /* Ensure that fill size doesn't exceed buffer size, due to possible data corruption */ + if (p + fill->size - 1 > p_end) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "fill size exceeds buffer size") /* Get the datatype message */ if ((exists = H5O_msg_exists_oh(open_oh, H5O_DTYPE_ID)) < 0) diff --git a/src/H5Olayout.c b/src/H5Olayout.c index 4020b23..651e317 100644 --- a/src/H5Olayout.c +++ b/src/H5Olayout.c @@ -17,7 +17,7 @@ * Purpose: Messages related to data layout. */ -#define H5D_FRIEND /*suppress error about including H5Dpkg */ +#define H5D_FRIEND /*suppress error about including H5Dpkg */ #include "H5Omodule.h" /* This source code file is part of the H5O module */ #include "H5private.h" /* Generic Functions */ @@ -90,12 +90,13 @@ H5FL_DEFINE(H5O_layout_t); */ static void * H5O__layout_decode(H5F_t *f, H5O_t H5_ATTR_UNUSED *open_oh, unsigned H5_ATTR_UNUSED mesg_flags, - unsigned H5_ATTR_UNUSED *ioflags, size_t H5_ATTR_UNUSED p_size, const uint8_t *p) + unsigned H5_ATTR_UNUSED *ioflags, size_t p_size, const uint8_t *p) { - H5O_layout_t *mesg = NULL; - uint8_t * heap_block = NULL; - unsigned u; - void * ret_value = NULL; /* Return value */ + H5O_layout_t * mesg = NULL; + uint8_t * heap_block = NULL; + unsigned u; + const uint8_t *p_end = p + p_size - 1; /* End of the p buffer */ + void * ret_value = NULL; /* Return value */ FUNC_ENTER_STATIC @@ -179,6 +180,10 @@ H5O__layout_decode(H5F_t *f, H5O_t H5_ATTR_UNUSED *open_oh, unsigned H5_ATTR_UNU if (mesg->type == H5D_COMPACT) { UINT32DECODE(p, mesg->storage.u.compact.size); if (mesg->storage.u.compact.size > 0) { + /* Ensure that size doesn't exceed buffer size, due to possible data corruption */ + if (p + mesg->storage.u.compact.size - 1 > p_end) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "storage size exceeds buffer size") + if (NULL == (mesg->storage.u.compact.buf = H5MM_malloc(mesg->storage.u.compact.size))) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed for compact data buffer") @@ -198,6 +203,10 @@ H5O__layout_decode(H5F_t *f, H5O_t H5_ATTR_UNUSED *open_oh, unsigned H5_ATTR_UNU UINT16DECODE(p, mesg->storage.u.compact.size); if (mesg->storage.u.compact.size > 0) { + /* Ensure that size doesn't exceed buffer size, due to possible data corruption */ + if (p + mesg->storage.u.compact.size - 1 > p_end) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "storage size exceeds buffer size") + /* Allocate space for compact data */ if (NULL == (mesg->storage.u.compact.buf = H5MM_malloc(mesg->storage.u.compact.size))) HGOTO_ERROR(H5E_OHDR, H5E_CANTALLOC, NULL, @@ -887,13 +896,13 @@ done: } /* end H5O__layout_reset() */ /*------------------------------------------------------------------------- - * Function: H5O__layout_free + * Function: H5O__layout_free * - * Purpose: Free's the message + * Purpose: Free's the message * - * Return: Non-negative on success/Negative on failure + * Return: Non-negative on success/Negative on failure * - * Programmer: Quincey Koziol + * Programmer: Quincey Koziol * Saturday, March 11, 2000 * *------------------------------------------------------------------------- diff --git a/tools/test/h5dump/CMakeTests.cmake b/tools/test/h5dump/CMakeTests.cmake index 4b61569..411e9ef 100644 --- a/tools/test/h5dump/CMakeTests.cmake +++ b/tools/test/h5dump/CMakeTests.cmake @@ -333,6 +333,8 @@ ${HDF5_TOOLS_DIR}/testfiles/tvlstr.h5 ${HDF5_TOOLS_DIR}/testfiles/tvms.h5 ${HDF5_TOOLS_DIR}/testfiles/t128bit_float.h5 + ${HDF5_TOOLS_DIR}/testfiles/tCVE_2018_11206_fill_old.h5 + ${HDF5_TOOLS_DIR}/testfiles/tCVE_2018_11206_fill_new.h5 ${HDF5_TOOLS_DIR}/testfiles/zerodim.h5 #STD_REF_OBJ files ${HDF5_TOOLS_DIR}/testfiles/trefer_attr.h5 @@ -1179,6 +1181,10 @@ # test to verify HDFFV-9407: long double full precision # ADD_H5_GREP_TEST (t128bit_float 1 "1.123456789012345" -m %.35Lg t128bit_float.h5) + # test to verify HDFFV-10480: out of bounds read in H5O_fill_new[old]_decode + ADD_H5_TEST (tCVE_2018_11206_fill_old 1 tCVE_2018_11206_fill_old.h5) + ADD_H5_TEST (tCVE_2018_11206_fill_new 1 tCVE_2018_11206_fill_new.h5) + ############################################################################## ### P L U G I N T E S T S ############################################################################## diff --git a/tools/test/h5dump/testh5dump.sh.in b/tools/test/h5dump/testh5dump.sh.in index f985da5..c4e2fd4 100644 --- a/tools/test/h5dump/testh5dump.sh.in +++ b/tools/test/h5dump/testh5dump.sh.in @@ -181,6 +181,8 @@ $SRC_H5DUMP_TESTFILES/tvlenstr_array.h5 $SRC_H5DUMP_TESTFILES/tvlstr.h5 $SRC_H5DUMP_TESTFILES/tvms.h5 $SRC_H5DUMP_TESTFILES/err_attr_dspace.h5 +$SRC_H5DUMP_TESTFILES/tCVE_2018_11206_fill_old.h5 +$SRC_H5DUMP_TESTFILES/tCVE_2018_11206_fill_new.h5 " LIST_OTHER_TEST_FILES=" @@ -870,6 +872,35 @@ TOOLTEST5() { fi } +# same as TOOLTEST1 but expects h5dump to fail +# +TOOLTEST_FAIL() { + + infile=$1 + expect="$TESTDIR/`basename $1 exp`.ddl" + actual="$TESTDIR/`basename $1 .exp`.out" + + # Run test. + TESTING $DUMPER $@ + ( + cd $TESTDIR + $RUNSERIAL $DUMPER_BIN "$@" $infile + ) >&$actual + RET=$? + # Segfault occurred + if [ $RET == 139 ] ; then + nerrors="`expr $nerrors + 1`" + echo "*FAILED - test on $infile failed with segmentation fault" + # Should fail but didn't + elif [ $RET == 0 ] ; then + nerrors="`expr $nerrors + 1`" + echo "*FAILED - test on $infile did not fail as expected" + else + echo " PASSED" + fi + +} + # ADD_HELP_TEST TOOLTEST_HELP() { @@ -1448,6 +1479,10 @@ TOOLTEST err_attr_dspace.ddl err_attr_dspace.h5 # test to verify HDFFV-9407: long double full precision #GREPTEST OUTTXT "1.123456789012345" t128bit_float.ddl -m %.35Lf t128bit_float.h5 +# test to verify HDFFV-10480: out of bounds read in H5O_fill_new[old]_decode +TOOLTEST_FAIL tCVE_2018_11206_fill_old.h5 +TOOLTEST_FAIL tCVE_2018_11206_fill_new.h5 + # Clean up temporary files/directories CLEAN_TESTFILES_AND_TESTDIR diff --git a/tools/testfiles/tCVE_2018_11206_fill_new.h5 b/tools/testfiles/tCVE_2018_11206_fill_new.h5 new file mode 100644 index 0000000..643591c Binary files /dev/null and b/tools/testfiles/tCVE_2018_11206_fill_new.h5 differ diff --git a/tools/testfiles/tCVE_2018_11206_fill_old.h5 b/tools/testfiles/tCVE_2018_11206_fill_old.h5 new file mode 100644 index 0000000..7f5b41a Binary files /dev/null and b/tools/testfiles/tCVE_2018_11206_fill_old.h5 differ -- cgit v0.12