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 c21324d. * Revert "Fixing arguments again." This reverts commit 5832a70. * Revert "Fixing arguments to ADD_H5_TEST again." This reverts commit b45de82. * Revert "Changed arguments to ADD_H5_TEST" This reverts commit 1671982. * 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 b343d66. * Revert "Added first argument to ADD_H5_TEST for HDFFV-10480 fix." This reverts commit b8a0f9a. * Added first argument and corrected the second. * Updated fixes for HDFFV-10480 and HDFFV-11159/HDFFV-11049 * Improved error messages.
HDFGroup · Mar 19, 2021 · dafc728 · dafc728
1 parent 49a14f9
commit dafc728
Show file tree

Hide file tree

Showing 8 changed files with 99 additions and 20 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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 @@ H5O__layout_reset(void *_mesg)
 } /* 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
@@ -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
@@ -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
diff --git a/tools/testfiles/tCVE_2018_11206_fill_old.h5 b/tools/testfiles/tCVE_2018_11206_fill_old.h5