Harden H5G cache deserialization (#2810)

1 files changed, 45 insertions, 68 deletions

diff --git a/src/H5Gcache.c b/src/H5Gcache.c
index b6c6a85..e088fd8 100644
--- a/src/H5Gcache.c
+++ b/src/H5Gcache.c
@@ -13,10 +13,8 @@
 /*-------------------------------------------------------------------------
  *
  * Created:		H5Gcache.c
- *			Feb  5 2008
- *			Quincey Koziol
  *
- * Purpose:		Implement group metadata cache methods.
+ * Purpose:		Implement group metadata cache methods
  *
  *-------------------------------------------------------------------------
  */
@@ -101,15 +99,10 @@ H5FL_SEQ_EXTERN(H5G_entry_t);
 /*-------------------------------------------------------------------------
  * Function:    H5G__cache_node_get_initial_load_size()
  *
- * Purpose:	Determine the size of the on-disk image of the node, and
- *		return this value in *image_len.
- *
- * Return:      Success:        SUCCEED
- *              Failure:        FAIL
- *
- * Programmer:  John Mainzer
- *              7/21/14
+ * Purpose:     Determine the size of the on-disk image of the node, and
+ *              return this value in *image_len.
  *
+ * Return:      SUCCEED/FAIL
  *-------------------------------------------------------------------------
  */
 static herr_t
@@ -119,7 +112,6 @@ H5G__cache_node_get_initial_load_size(void *_udata, size_t *image_len)
 
     FUNC_ENTER_PACKAGE_NOERR
 
-    /* Sanity checks */
     HDassert(f);
     HDassert(image_len);
 
@@ -132,22 +124,18 @@ H5G__cache_node_get_initial_load_size(void *_udata, size_t *image_len)
 /*-------------------------------------------------------------------------
  * Function:    H5G__cache_node_deserialize
  *
- * Purpose:	Given a buffer containing the on disk image of a symbol table
- *		node, allocate an instance of H5G_node_t, load the contents of the
- *		image into it, and return a pointer to the instance.
+ * Purpose:     Given a buffer containing the on disk image of a symbol table
+ *              node, allocate an instance of H5G_node_t, load the contents of the
+ *              image into it, and return a pointer to the instance.
  *
- *		Note that deserializing the image requires access to the file
- *		pointer, which is not included in the parameter list for this
- *		callback.  Finesse this issue by passing in the file pointer
- *		twice to the H5AC_protect() call -- once as the file pointer
- *		proper, and again as the user data
+ *              Note that deserializing the image requires access to the file
+ *              pointer, which is not included in the parameter list for this
+ *              callback.  Finesse this issue by passing in the file pointer
+ *              twice to the H5AC_protect() call -- once as the file pointer
+ *              proper, and again as the user data
  *
  * Return:      Success:        Pointer to in core representation
  *              Failure:        NULL
- *
- * Programmer:  John Mainzer
- *              6/21/14
- *
  *-------------------------------------------------------------------------
  */
 static void *
@@ -157,11 +145,10 @@ H5G__cache_node_deserialize(const void *_image, size_t len, void *_udata, hbool_
     H5G_node_t    *sym       = NULL;                    /* Symbol table node created */
     const uint8_t *image     = (const uint8_t *)_image; /* Pointer to image to deserialize */
     const uint8_t *image_end = image + len - 1;         /* Pointer to end of image buffer */
-    void          *ret_value = NULL;                    /* Return value */
+    void          *ret_value = NULL;
 
     FUNC_ENTER_PACKAGE
 
-    /* Sanity checks */
     HDassert(image);
     HDassert(len > 0);
     HDassert(f);
@@ -174,22 +161,30 @@ H5G__cache_node_deserialize(const void *_image, size_t len, void *_udata, hbool_
     if (NULL == (sym->entry = H5FL_SEQ_CALLOC(H5G_entry_t, (size_t)(2 * H5F_SYM_LEAF_K(f)))))
         HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed")
 
-    /* magic */
+    /* Magic */
+    if (H5_IS_BUFFER_OVERFLOW(image, H5_SIZEOF_MAGIC, image_end))
+        HGOTO_ERROR(H5E_SYM, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding");
     if (HDmemcmp(image, H5G_NODE_MAGIC, (size_t)H5_SIZEOF_MAGIC) != 0)
         HGOTO_ERROR(H5E_SYM, H5E_BADVALUE, NULL, "bad symbol table node signature")
     image += H5_SIZEOF_MAGIC;
 
-    /* version */
+    /* Version */
+    if (H5_IS_BUFFER_OVERFLOW(image, 1, image_end))
+        HGOTO_ERROR(H5E_SYM, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding");
     if (H5G_NODE_VERS != *image++)
         HGOTO_ERROR(H5E_SYM, H5E_VERSION, NULL, "bad symbol table node version")
 
-    /* reserved */
+    /* Reserved */
+    if (H5_IS_BUFFER_OVERFLOW(image, 1, image_end))
+        HGOTO_ERROR(H5E_SYM, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding");
     image++;
 
-    /* number of symbols */
+    /* Number of symbols */
+    if (H5_IS_BUFFER_OVERFLOW(image, 2, image_end))
+        HGOTO_ERROR(H5E_SYM, H5E_OVERFLOW, NULL, "ran off end of input buffer while decoding");
     UINT16DECODE(image, sym->nsyms);
 
-    /* entries */
+    /* Entries */
     if (H5G__ent_decode_vec(f, &image, image_end, sym->entry, sym->nsyms) < 0)
         HGOTO_ERROR(H5E_SYM, H5E_CANTLOAD, NULL, "unable to decode symbol table entries")
 
@@ -208,14 +203,9 @@ done:
  * Function:    H5G__cache_node_image_len
  *
  * Purpose:     Compute the size of the data structure on disk and return
- *              it in *image_len.
- *
- * Return:      Success:        SUCCEED
- *              Failure:        FAIL
- *
- * Programmer:  John Mainzer
- *              6/21/14
+ *              it in *image_len
  *
+ * Return:      SUCCEED/FAIL
  *-------------------------------------------------------------------------
  */
 static herr_t
@@ -225,7 +215,6 @@ H5G__cache_node_image_len(const void *_thing, size_t *image_len)
 
     FUNC_ENTER_PACKAGE_NOERR
 
-    /* Sanity checks */
     HDassert(sym);
     HDassert(sym->cache_info.magic == H5C__H5C_CACHE_ENTRY_T_MAGIC);
     HDassert(sym->cache_info.type == H5AC_SNODE);
@@ -239,17 +228,12 @@ H5G__cache_node_image_len(const void *_thing, size_t *image_len)
 /*-------------------------------------------------------------------------
  * Function:    H5G__cache_node_serialize
  *
- * Purpose:	Given a correctly sized buffer and an instance of H5G_node_t,
- *		serialize the contents of the instance of H5G_node_t, and write
- *		this data into the supplied buffer.  This buffer will be written
- *		to disk.
- *
- * Return:      Success:        SUCCEED
- *              Failure:        FAIL
- *
- * Programmer:  John Mainzer
- *              7/21/14
+ * Purpose:     Given a correctly sized buffer and an instance of H5G_node_t,
+ *              serialize the contents of the instance of H5G_node_t, and write
+ *              this data into the supplied buffer.  This buffer will be written
+ *              to disk.
  *
+ * Return:      SUCCEED/FAIL
  *-------------------------------------------------------------------------
  */
 static herr_t
@@ -257,11 +241,10 @@ H5G__cache_node_serialize(const H5F_t *f, void *_image, size_t len, void *_thing
 {
     H5G_node_t *sym       = (H5G_node_t *)_thing; /* Pointer to object */
     uint8_t    *image     = (uint8_t *)_image;    /* Pointer into raw data buffer */
-    herr_t      ret_value = SUCCEED;              /* Return value */
+    herr_t      ret_value = SUCCEED;
 
     FUNC_ENTER_PACKAGE
 
-    /* Sanity checks */
     HDassert(f);
     HDassert(image);
     HDassert(sym);
@@ -269,20 +252,20 @@ H5G__cache_node_serialize(const H5F_t *f, void *_image, size_t len, void *_thing
     HDassert(sym->cache_info.type == H5AC_SNODE);
     HDassert(len == sym->node_size);
 
-    /* magic number */
+    /* Magic number */
     H5MM_memcpy(image, H5G_NODE_MAGIC, (size_t)H5_SIZEOF_MAGIC);
     image += H5_SIZEOF_MAGIC;
 
-    /* version number */
+    /* Version number */
     *image++ = H5G_NODE_VERS;
 
-    /* reserved */
+    /* Reserved */
     *image++ = 0;
 
-    /* number of symbols */
+    /* Number of symbols */
     UINT16ENCODE(image, sym->nsyms);
 
-    /* entries */
+    /* Entries */
     if (H5G__ent_encode_vec(f, &image, sym->entry, sym->nsyms) < 0)
         HGOTO_ERROR(H5E_SYM, H5E_CANTENCODE, FAIL, "can't serialize")
 
@@ -296,29 +279,23 @@ done:
 /*-------------------------------------------------------------------------
  * Function:    H5G__cache_node_free_icr
  *
- * Purpose:	Destroys a symbol table node in memory.
- *
- * Note:	The metadata cache sets the object's cache_info.magic to
- *		H5C__H5C_CACHE_ENTRY_T_BAD_MAGIC before calling a free_icr
- *		callback (checked in assert).
- *
- * Return:      Success:        SUCCEED
- *              Failure:        FAIL
+ * Purpose:     Destroy a symbol table node in memory
  *
- * Programmer:  John Mainzer
- *              6/21/14
+ * Note:        The metadata cache sets the object's cache_info.magic to
+ *              H5C__H5C_CACHE_ENTRY_T_BAD_MAGIC before calling a free_icr
+ *              callback (checked in assert).
  *
+ * Return:      SUCCEED/FAIL
  *-------------------------------------------------------------------------
  */
 static herr_t
 H5G__cache_node_free_icr(void *_thing)
 {
     H5G_node_t *sym       = (H5G_node_t *)_thing; /* Pointer to the object */
-    herr_t      ret_value = SUCCEED;              /* Return value */
+    herr_t      ret_value = SUCCEED;
 
     FUNC_ENTER_PACKAGE
 
-    /* Sanity checks */
     HDassert(sym);
     HDassert(sym->cache_info.magic == H5C__H5C_CACHE_ENTRY_T_BAD_MAGIC);
     HDassert(sym->cache_info.type == H5AC_SNODE);