Fixed HDFFV-10480 (CVE-2018-11206) and HDFFV-11159 (CVE-2018-14033) (#…

…417) * 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 and A user's patch was applied to this previously, but it is redone for a more correct fix, that is the check now accounted for the previous advance of the buffer pointer. Platforms tested: Linux/64 (jelly) * Fixed format issues with clang formatter.
HDFGroup · Mar 4, 2021 · 7b23ce1 · 7b23ce1
1 parent e65814b
commit 7b23ce1
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 15 deletions.
diff --git a/src/H5Ofill.c b/src/H5Ofill.c
@@ -194,8 +194,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_NOAPI_NOINIT
 
@@ -226,8 +227,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);
@@ -310,10 +314,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_NOAPI_NOINIT
 
@@ -334,8 +339,10 @@ H5O_fill_old_decode(H5F_t *f, H5O_t *open_oh, unsigned H5_ATTR_UNUSED mesg_flags
     /* 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
@@ -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,