Harden the v1 B-tree and local heap cache clients (#2803)

* Hardens v1 B-tree deserialize function
* Harden the H5HL deserialize functionality

4 files changed, 90 insertions, 96 deletions

diff --git a/src/H5Bcache.c b/src/H5Bcache.c
index cd0a0ba..437bc1b 100644
--- a/src/H5Bcache.c
+++ b/src/H5Bcache.c
@@ -13,10 +13,8 @@
 /*-------------------------------------------------------------------------
  *
  * Created:		H5Bcache.c
- *			Oct 31 2005
- *			Quincey Koziol
  *
- * Purpose:		Implement B-tree metadata cache methods.
+ * Purpose:		Implement B-tree metadata cache methods
  *
  *-------------------------------------------------------------------------
  */
@@ -83,13 +81,9 @@ const H5AC_class_t H5AC_BT[1] = {{
 /*-------------------------------------------------------------------------
  * Function:    H5B__cache_get_initial_load_size
  *
- * Purpose:     Compute the size of the data structure on disk.
- *
- * Return:      Non-negative on success/Negative on failure
- *
- * Programmer:  Quincey Koziol
- *              May 18, 2010
+ * Purpose:     Compute the size of the data structure on disk
  *
+ * Return:      SUCCEED/FAIL
  *-------------------------------------------------------------------------
  */
 static herr_t
@@ -117,24 +111,20 @@ H5B__cache_get_initial_load_size(void *_udata, size_t *image_len)
 /*-------------------------------------------------------------------------
  * Function:    H5B__cache_deserialize
  *
- * Purpose:     Deserialize the data structure from disk.
- *
- * Return:	Success:	Pointer to a new B-tree node.
- *		Failure:	NULL
- *
- * Programmer:  Quincey Koziol
- *              Mar 24, 2008
+ * Purpose:     Deserialize the data structure from disk
  *
+ * Return:      Success:    Pointer to a new B-tree node
+ *              Failure:    NULL
  *-------------------------------------------------------------------------
  */
 static void *
-H5B__cache_deserialize(const void *_image, size_t H5_ATTR_UNUSED len, void *_udata,
-                       hbool_t H5_ATTR_UNUSED *dirty)
+H5B__cache_deserialize(const void *_image, size_t len, void *_udata, hbool_t H5_ATTR_UNUSED *dirty)
 {
     H5B_t          *bt    = NULL;                     /* Pointer to the deserialized B-tree node */
     H5B_cache_ud_t *udata = (H5B_cache_ud_t *)_udata; /* User data for callback */
     H5B_shared_t   *shared;                           /* Pointer to shared B-tree info */
     const uint8_t  *image = (const uint8_t *)_image;  /* Pointer into image buffer */
+    const uint8_t  *p_end = image + len - 1;          /* End of image buffer */
     uint8_t        *native;                           /* Pointer to native keys */
     unsigned        u;                                /* Local index variable */
     H5B_t          *ret_value = NULL;                 /* Return value */
@@ -156,7 +146,8 @@ H5B__cache_deserialize(const void *_image, size_t H5_ATTR_UNUSED len, void *_uda
 
     /* Get a pointer to the shared info, for convenience */
     shared = (H5B_shared_t *)H5UC_GET_OBJ(bt->rc_shared);
-    HDassert(shared);
+    if (NULL == shared)
+        HGOTO_ERROR(H5E_BTREE, H5E_CANTGET, NULL, "can't get a pointer to shared data")
 
     /* Allocate space for the native keys and child addresses */
     if (NULL == (bt->native = H5FL_BLK_MALLOC(native_block, shared->sizeof_keys)))
@@ -164,49 +155,61 @@ H5B__cache_deserialize(const void *_image, size_t H5_ATTR_UNUSED len, void *_uda
     if (NULL == (bt->child = H5FL_SEQ_MALLOC(haddr_t, (size_t)shared->two_k)))
         HGOTO_ERROR(H5E_BTREE, H5E_CANTALLOC, NULL, "can't allocate buffer for child addresses")
 
-    /* magic number */
+    /* Magic number */
+    if (H5_IS_BUFFER_OVERFLOW(image, H5_SIZEOF_MAGIC, p_end))
+        HGOTO_ERROR(H5E_BTREE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding");
     if (HDmemcmp(image, H5B_MAGIC, (size_t)H5_SIZEOF_MAGIC) != 0)
         HGOTO_ERROR(H5E_BTREE, H5E_BADVALUE, NULL, "wrong B-tree signature")
     image += H5_SIZEOF_MAGIC;
 
-    /* node type and level */
+    /* Node type and level */
+    if (H5_IS_BUFFER_OVERFLOW(image, 2, p_end))
+        HGOTO_ERROR(H5E_BTREE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding");
     if (*image++ != (uint8_t)udata->type->id)
         HGOTO_ERROR(H5E_BTREE, H5E_CANTLOAD, NULL, "incorrect B-tree node type")
     bt->level = *image++;
 
-    /* entries used */
+    /* Entries used */
+    if (H5_IS_BUFFER_OVERFLOW(image, 2, p_end))
+        HGOTO_ERROR(H5E_BTREE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding");
     UINT16DECODE(image, bt->nchildren);
 
     /* Check if bt->nchildren is greater than two_k */
     if (bt->nchildren > shared->two_k)
         HGOTO_ERROR(H5E_BTREE, H5E_BADVALUE, NULL, "number of children is greater than maximum")
 
-    /* sibling pointers */
+    /* Sibling pointers */
+    if (H5_IS_BUFFER_OVERFLOW(image, H5F_sizeof_addr(udata->f), p_end))
+        HGOTO_ERROR(H5E_BTREE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding");
     H5F_addr_decode(udata->f, (const uint8_t **)&image, &(bt->left));
+
+    if (H5_IS_BUFFER_OVERFLOW(image, H5F_sizeof_addr(udata->f), p_end))
+        HGOTO_ERROR(H5E_BTREE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding");
     H5F_addr_decode(udata->f, (const uint8_t **)&image, &(bt->right));
 
-    /* the child/key pairs */
+    /* Child/key pairs */
     native = bt->native;
     for (u = 0; u < bt->nchildren; u++) {
         /* Decode native key value */
+        if (H5_IS_BUFFER_OVERFLOW(image, shared->sizeof_rkey, p_end))
+            HGOTO_ERROR(H5E_BTREE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding");
         if ((udata->type->decode)(shared, image, native) < 0)
             HGOTO_ERROR(H5E_BTREE, H5E_CANTDECODE, NULL, "unable to decode key")
         image += shared->sizeof_rkey;
         native += udata->type->sizeof_nkey;
 
         /* Decode address value */
+        if (H5_IS_BUFFER_OVERFLOW(image, H5F_sizeof_addr(udata->f), p_end))
+            HGOTO_ERROR(H5E_BTREE, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding");
         H5F_addr_decode(udata->f, (const uint8_t **)&image, bt->child + u);
-    } /* end for */
+    }
 
-    /* Decode final key */
+    /* Final key */
     if (bt->nchildren > 0) {
         /* Decode native key value */
         if ((udata->type->decode)(shared, image, native) < 0)
             HGOTO_ERROR(H5E_BTREE, H5E_CANTDECODE, NULL, "unable to decode key")
-    } /* end if */
-
-    /* Sanity check */
-    HDassert((size_t)((const uint8_t *)image - (const uint8_t *)_image) <= len);
+    }
 
     /* Set return value */
     ret_value = bt;
@@ -224,11 +227,7 @@ done:
  *
  * Purpose:     Compute the size of the data structure on disk.
  *
- * Return:      Non-negative on success/Negative on failure
- *
- * Programmer:  Quincey Koziol
- *              May 20, 2010
- *
+ * Return:      SUCCEED/FAIL
  *-------------------------------------------------------------------------
  */
 static herr_t
@@ -256,13 +255,9 @@ H5B__cache_image_len(const void *_thing, size_t *image_len)
 /*-------------------------------------------------------------------------
  * Function:    H5B__cache_serialize
  *
- * Purpose:     Serialize the data structure for writing to disk.
- *
- * Return:      Non-negative on success/Negative on failure
- *
- * Programmer:  Quincey Koziol
- *              Mar 24, 2008
+ * Purpose:     Serialize the data structure for writing to disk
  *
+ * Return:      SUCCEED/FAIL
  *-------------------------------------------------------------------------
  */
 static herr_t
@@ -341,11 +336,7 @@ done:
  *
  * Purpose:     Destroy/release an "in core representation" of a data structure
  *
- * Return:      Non-negative on success/Negative on failure
- *
- * Programmer:  Quincey Koziol
- *              Mar 26, 2008
- *
+ * Return:      SUCCEED/FAIL
  *-------------------------------------------------------------------------
  */
 static herr_t
diff --git a/src/H5Bpkg.h b/src/H5Bpkg.h
index ef9f56e..f50bd48 100644
--- a/src/H5Bpkg.h
+++ b/src/H5Bpkg.h
@@ -44,21 +44,21 @@
 /* The B-tree node as stored in memory...  */
 typedef struct H5B_t {
     H5AC_info_t cache_info; /* Information for H5AC cache functions */
-                            /* _must_ be first field in structure */
-    H5UC_t  *rc_shared;     /*ref-counted shared info         */
-    unsigned level;         /*node level                 */
-    unsigned nchildren;     /*number of child pointers         */
-    haddr_t  left;          /*address of left sibling         */
-    haddr_t  right;         /*address of right sibling         */
-    uint8_t *native;        /*array of keys in native format     */
-    haddr_t *child;         /*2k child pointers             */
+                            /* MUST be first field in structure */
+    H5UC_t  *rc_shared;     /* Ref-counted shared info */
+    unsigned level;         /* Node level */
+    unsigned nchildren;     /* Number of child pointers */
+    haddr_t  left;          /* Address of left sibling */
+    haddr_t  right;         /* Address of right sibling */
+    uint8_t *native;        /* Array of keys in native format */
+    haddr_t *child;         /* 2k child pointers */
 } H5B_t;
 
 /* Callback info for loading a B-tree node into the cache */
 typedef struct H5B_cache_ud_t {
-    H5F_t                    *f;         /* File that B-tree node is within   */
-    const struct H5B_class_t *type;      /* Type of tree                 */
-    H5UC_t                   *rc_shared; /* Ref-counted shared info         */
+    H5F_t                    *f;         /* File that B-tree node is within */
+    const struct H5B_class_t *type;      /* Type of tree */
+    H5UC_t                   *rc_shared; /* Ref-counted shared info */
 } H5B_cache_ud_t;
 
 /*****************************/
diff --git a/src/H5Bprivate.h b/src/H5Bprivate.h
index 49e400c..0017c43 100644
--- a/src/H5Bprivate.h
+++ b/src/H5Bprivate.h
@@ -82,16 +82,16 @@ typedef int (*H5B_operator_t)(H5F_t *f, const void *_lt_key, haddr_t addr, const
  * the instances of nodes in that B-tree.
  */
 typedef struct H5B_shared_t {
-    const struct H5B_class_t *type;         /* Type of tree			     */
-    unsigned                  two_k;        /* 2*"K" value for tree's nodes      */
-    size_t                    sizeof_rkey;  /* Size of raw (disk) key	     */
-    size_t                    sizeof_rnode; /* Size of raw (disk) node	     */
-    size_t                    sizeof_keys;  /* Size of native (memory) key node  */
-    size_t                    sizeof_addr;  /* Size of file address (in bytes)   */
-    size_t                    sizeof_len;   /* Size of file lengths (in bytes)   */
+    const struct H5B_class_t *type;         /* Type of tree */
+    unsigned                  two_k;        /* 2*"K" value for tree's nodes */
+    size_t                    sizeof_rkey;  /* Size of raw (disk) key */
+    size_t                    sizeof_rnode; /* Size of raw (disk) node */
+    size_t                    sizeof_keys;  /* Size of native (memory) key node */
+    size_t                    sizeof_addr;  /* Size of file address (in bytes) */
+    size_t                    sizeof_len;   /* Size of file lengths (in bytes) */
     uint8_t                  *page;         /* Disk page */
     size_t                   *nkey;         /* Offsets of each native key in native key buffer */
-    void                     *udata;        /* 'Local' info for a B-tree         */
+    void                     *udata;        /* 'Local' info for a B-tree */
 } H5B_shared_t;
 
 /*
@@ -101,7 +101,6 @@ typedef struct H5B_shared_t {
  * has an array of K values indexed by the `id' class field below.  The
  * array is initialized with the HDF5_BTREE_K_DEFAULT macro.
  */
-
 typedef struct H5B_class_t {
     H5B_subid_t id;                                     /*id as found in file*/
     size_t      sizeof_nkey;                            /*size of native (memory) key*/
diff --git a/src/H5HLcache.c b/src/H5HLcache.c
index 72af9b4..c04efb6 100644
--- a/src/H5HLcache.c
+++ b/src/H5HLcache.c
@@ -13,10 +13,8 @@
 /*-------------------------------------------------------------------------
  *
  * Created:     H5HLcache.c
- *              Feb  5 2008
- *              Quincey Koziol
  *
- * Purpose:     Implement local heap metadata cache methods.
+ * Purpose:     Implement local heap metadata cache methods
  *
  *-------------------------------------------------------------------------
  */
@@ -81,7 +79,8 @@ static herr_t H5HL__cache_datablock_notify(H5C_notify_action_t action, void *_th
 static herr_t H5HL__cache_datablock_free_icr(void *thing);
 
 /* Header deserialization */
-static herr_t H5HL__hdr_deserialize(H5HL_t *heap, const uint8_t *image, H5HL_cache_prfx_ud_t *udata);
+static herr_t H5HL__hdr_deserialize(H5HL_t *heap, const uint8_t *image, size_t len,
+                                    H5HL_cache_prfx_ud_t *udata);
 
 /* Free list de/serialization */
 static herr_t H5HL__fl_deserialize(H5HL_t *heap);
@@ -137,38 +136,39 @@ const H5AC_class_t H5AC_LHEAP_DBLK[1] = {{
 /*-------------------------------------------------------------------------
  * Function:    H5HL__hdr_deserialize()
  *
- * Purpose:	Decode a local heap's header
- *
- * Return:      Success:        SUCCEED
- *              Failure:        FAIL
- *
- * Programmer:  Quincey Koziol
- *              December 15, 2016
+ * Purpose:     Decode a local heap's header
  *
+ * Return:      SUCCEED/FAIL
  *-------------------------------------------------------------------------
  */
 static herr_t
-H5HL__hdr_deserialize(H5HL_t *heap, const uint8_t *image, H5HL_cache_prfx_ud_t *udata)
+H5HL__hdr_deserialize(H5HL_t *heap, const uint8_t *image, size_t len, H5HL_cache_prfx_ud_t *udata)
 {
-    herr_t ret_value = SUCCEED; /* Return value */
+    const uint8_t *p_end     = image + len - 1; /* End of image buffer */
+    herr_t         ret_value = SUCCEED;
 
     FUNC_ENTER_PACKAGE
 
-    /* Sanity checks */
     HDassert(heap);
     HDassert(image);
     HDassert(udata);
 
-    /* Check magic number */
+    /* Magic number */
+    if (H5_IS_BUFFER_OVERFLOW(image, H5_SIZEOF_MAGIC, p_end))
+        HGOTO_ERROR(H5E_HEAP, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding");
     if (HDmemcmp(image, H5HL_MAGIC, (size_t)H5_SIZEOF_MAGIC) != 0)
         HGOTO_ERROR(H5E_HEAP, H5E_BADVALUE, FAIL, "bad local heap signature")
     image += H5_SIZEOF_MAGIC;
 
     /* Version */
+    if (H5_IS_BUFFER_OVERFLOW(image, 1, p_end))
+        HGOTO_ERROR(H5E_HEAP, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding");
     if (H5HL_VERSION != *image++)
         HGOTO_ERROR(H5E_HEAP, H5E_VERSION, FAIL, "wrong version number in local heap")
 
     /* Reserved */
+    if (H5_IS_BUFFER_OVERFLOW(image, 3, p_end))
+        HGOTO_ERROR(H5E_HEAP, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding");
     image += 3;
 
     /* Store the prefix's address & length */
@@ -176,14 +176,20 @@ H5HL__hdr_deserialize(H5HL_t *heap, const uint8_t *image, H5HL_cache_prfx_ud_t *
     heap->prfx_size = udata->sizeof_prfx;
 
     /* Heap data size */
+    if (H5_IS_BUFFER_OVERFLOW(image, udata->sizeof_size, p_end))
+        HGOTO_ERROR(H5E_HEAP, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding");
     H5F_DECODE_LENGTH_LEN(image, heap->dblk_size, udata->sizeof_size);
 
     /* Free list head */
+    if (H5_IS_BUFFER_OVERFLOW(image, udata->sizeof_size, p_end))
+        HGOTO_ERROR(H5E_HEAP, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding");
     H5F_DECODE_LENGTH_LEN(image, heap->free_block, udata->sizeof_size);
     if (heap->free_block != H5HL_FREE_NULL && heap->free_block >= heap->dblk_size)
         HGOTO_ERROR(H5E_HEAP, H5E_BADVALUE, FAIL, "bad heap free list")
 
     /* Heap data address */
+    if (H5_IS_BUFFER_OVERFLOW(image, udata->sizeof_addr, p_end))
+        HGOTO_ERROR(H5E_HEAP, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding");
     H5F_addr_decode_len(udata->sizeof_addr, &image, &(heap->dblk_addr));
 
 done:
@@ -344,8 +350,7 @@ H5HL__cache_prefix_get_initial_load_size(void H5_ATTR_UNUSED *_udata, size_t *im
  *-------------------------------------------------------------------------
  */
 static herr_t
-H5HL__cache_prefix_get_final_load_size(const void *_image, size_t H5_ATTR_NDEBUG_UNUSED image_len,
-                                       void *_udata, size_t *actual_len)
+H5HL__cache_prefix_get_final_load_size(const void *_image, size_t image_len, void *_udata, size_t *actual_len)
 {
     const uint8_t        *image = (const uint8_t *)_image;        /* Pointer into raw data buffer */
     H5HL_cache_prfx_ud_t *udata = (H5HL_cache_prfx_ud_t *)_udata; /* User data for callback */
@@ -363,7 +368,7 @@ H5HL__cache_prefix_get_final_load_size(const void *_image, size_t H5_ATTR_NDEBUG
     HDmemset(&heap, 0, sizeof(H5HL_t));
 
     /* Deserialize the heap's header */
-    if (H5HL__hdr_deserialize(&heap, (const uint8_t *)image, udata) < 0)
+    if (H5HL__hdr_deserialize(&heap, (const uint8_t *)image, image_len, udata) < 0)
         HGOTO_ERROR(H5E_HEAP, H5E_CANTDECODE, FAIL, "can't decode local heap header")
 
     /* Set the final size for the cache image */
@@ -383,25 +388,22 @@ done:
 /*-------------------------------------------------------------------------
  * Function:    H5HL__cache_prefix_deserialize
  *
- * Purpose:	Given a buffer containing the on disk image of the local
- *		heap prefix, deserialize it, load its contents into a newly allocated
- *		instance of H5HL_prfx_t, and return a pointer to the new instance.
- *
- * Return:      Success:        Pointer to in core representation
- *              Failure:        NULL
- *
- * Programmer:  John Mainzer
- *              6/21/14
+ * Purpose:     Given a buffer containing the on disk image of the local
+ *              heap prefix, deserialize it, load its contents into a newly
+ *              allocated instance of H5HL_prfx_t, and return a pointer to
+ *              the new instance.
  *
+ * Return:      Success:    Pointer to in core representation
+ *              Failure:    NULL
  *-------------------------------------------------------------------------
  */
 static void *
-H5HL__cache_prefix_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUSED len, void *_udata,
-                               hbool_t H5_ATTR_UNUSED *dirty)
+H5HL__cache_prefix_deserialize(const void *_image, size_t len, void *_udata, hbool_t H5_ATTR_UNUSED *dirty)
 {
     H5HL_t               *heap      = NULL;                           /* Local heap */
     H5HL_prfx_t          *prfx      = NULL;                           /* Heap prefix deserialized */
     const uint8_t        *image     = (const uint8_t *)_image;        /* Pointer into decoding buffer */
+    const uint8_t        *p_end     = image + len - 1;                /* End of image buffer */
     H5HL_cache_prfx_ud_t *udata     = (H5HL_cache_prfx_ud_t *)_udata; /* User data for callback */
     void                 *ret_value = NULL;                           /* Return value */
 
@@ -422,7 +424,7 @@ H5HL__cache_prefix_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUSED
         HGOTO_ERROR(H5E_HEAP, H5E_CANTALLOC, NULL, "can't allocate local heap structure");
 
     /* Deserialize the heap's header */
-    if (H5HL__hdr_deserialize(heap, (const uint8_t *)image, udata) < 0)
+    if (H5HL__hdr_deserialize(heap, (const uint8_t *)image, len, udata) < 0)
         HGOTO_ERROR(H5E_HEAP, H5E_CANTDECODE, NULL, "can't decode local heap header")
 
     /* Allocate the heap prefix */
@@ -446,6 +448,8 @@ H5HL__cache_prefix_deserialize(const void *_image, size_t H5_ATTR_NDEBUG_UNUSED
             image = ((const uint8_t *)_image) + heap->prfx_size;
 
             /* Copy the heap data from the speculative read buffer */
+            if (H5_IS_BUFFER_OVERFLOW(image, heap->dblk_size, p_end))
+                HGOTO_ERROR(H5E_HEAP, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding");
             H5MM_memcpy(heap->dblk_image, image, heap->dblk_size);
 
             /* Build free list */