From 87ed76d8992aa6939cbe99910058dbad0efed0d6 Mon Sep 17 00:00:00 2001
From: bmribler <39579120+bmribler@users.noreply.github.com>
Date: Thu, 3 Dec 2020 11:07:16 -0500
Subject: Fixed HDFFV-10480 and HDFFV-11159 (#145)

* Fixed HDFFV-10480 and HDFFV-11159

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 typo
---
 src/H5Ofill.c   | 15 +++++++++++++--
 src/H5Olayout.c | 11 ++++++++++-
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/src/H5Ofill.c b/src/H5Ofill.c
index 1095d1b..bf13212 100644
--- a/src/H5Ofill.c
+++ b/src/H5Ofill.c
@@ -184,9 +184,10 @@ H5FL_BLK_EXTERN(type_conv);
 static void *
 H5O_fill_new_decode(H5F_t H5_ATTR_UNUSED *f, hid_t H5_ATTR_UNUSED dxpl_id, 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)
+                    size_t p_size, const uint8_t *p)
 {
     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
@@ -218,6 +219,11 @@ H5O_fill_new_decode(H5F_t H5_ATTR_UNUSED *f, hid_t H5_ATTR_UNUSED dxpl_id, H5O_t
             INT32DECODE(p, fill->size);
             if (fill->size > 0) {
                 H5_CHECK_OVERFLOW(fill->size, ssize_t, size_t);
+
+                /* 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")
                 HDmemcpy(fill->buf, p, (size_t)fill->size);
@@ -299,9 +305,10 @@ done:
 static void *
 H5O_fill_old_decode(H5F_t H5_ATTR_UNUSED *f, hid_t H5_ATTR_UNUSED dxpl_id, 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)
+                    size_t p_size, const uint8_t *p)
 {
     H5O_fill_t *fill = NULL; /* Decoded fill value message */
+    const uint8_t *p_end = p + p_size - 1; /* End of the p buffer */
     void *      ret_value;   /* Return value */
 
     FUNC_ENTER_NOAPI_NOINIT
@@ -322,6 +329,10 @@ H5O_fill_old_decode(H5F_t H5_ATTR_UNUSED *f, hid_t H5_ATTR_UNUSED dxpl_id, H5O_t
 
     /* Only decode the fill value itself if there is one */
     if (fill->size > 0) {
+        /* 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")
         HDmemcpy(fill->buf, p, (size_t)fill->size);
diff --git a/src/H5Olayout.c b/src/H5Olayout.c
index 4466940..b9b8cc6 100644
--- a/src/H5Olayout.c
+++ b/src/H5Olayout.c
@@ -90,10 +90,11 @@ H5FL_DEFINE(H5O_layout_t);
 static void *
 H5O_layout_decode(H5F_t *f, hid_t H5_ATTR_UNUSED dxpl_id, 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)
+                  size_t p_size, const uint8_t *p)
 {
     H5O_layout_t *mesg = 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_NOAPI_NOINIT
@@ -174,6 +175,10 @@ H5O_layout_decode(H5F_t *f, hid_t H5_ATTR_UNUSED dxpl_id, H5O_t H5_ATTR_UNUSED *
         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 fill 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")
@@ -191,6 +196,10 @@ H5O_layout_decode(H5F_t *f, hid_t H5_ATTR_UNUSED dxpl_id, H5O_t H5_ATTR_UNUSED *
             case H5D_COMPACT:
                 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")
+
                     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")
-- 
cgit v0.12