Merge pull request #1054 in HDFFV/hdf5 from ~DEROBINS/hdf5_der:hdf5_1_8 to hdf5_1_8

* commit '2636f401ba236e99adda4cc50fb89bebbe0b73fd':
  Moved a fix for HDFFV-10358 (CVE-2017-17509) from develop to 1.8.

4 files changed, 52 insertions, 32 deletions

diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt
index 30b148a..ad06d40 100644
--- a/release_docs/RELEASE.txt
+++ b/release_docs/RELEASE.txt
@@ -209,6 +209,23 @@ Bug Fixes since HDF5-1.8.20
 
       (DER - 2018/02/26, HDFFV-10357)
 
+    - If an HDF5 file contains a malformed symbol table node that declares
+      it contains more symbols than it actually contains, the library
+      can run off the end of the metadata cache buffer while processing
+      the symbol table node.
+
+      This issue was reported to The HDF Group as issue #CVE-2017-17509.
+
+      NOTE: The HDF5 C library cannot produce such a file. This condition
+            should only occur in a corrupt (or deliberately altered) file
+            or a file created by third-party software.
+
+      Performing bounds checks on the buffer while processing fixes the
+      problem. Instead of the segmentation fault, the normal HDF5 error
+      handling is invoked.
+
+      (DER - 2018/03/12, HDFFV-10358)
+
     Configuration
     -------------
     - CMake
diff --git a/src/H5Gcache.c b/src/H5Gcache.c
index 1ca80df..994c5bf 100644
--- a/src/H5Gcache.c
+++ b/src/H5Gcache.c
@@ -103,49 +103,43 @@ H5FL_SEQ_EXTERN(H5G_entry_t);
 
 
 /*-------------------------------------------------------------------------
- * Function:	H5G_node_load
+ * Function:    H5G_node_load
  *
- * Purpose:	Loads a symbol table node from the file.
+ * Purpose:     Loads a symbol table node from the file.
  *
- * Return:	Success:	Ptr to the new table.
+ * Return:      Success:    Ptr to the new table.
+ *              Failure:    NULL
  *
- *		Failure:	NULL
- *
- * Programmer:	Robb Matzke
- *		matzke@llnl.gov
- *		Jun 23 1997
+ * Programmer:  Robb Matzke
+ *              matzke@llnl.gov
+ *              Jun 23 1997
  *
  *-------------------------------------------------------------------------
  */
 static H5G_node_t *
 H5G_node_load(H5F_t *f, hid_t dxpl_id, haddr_t addr, void *udata)
 {
-    H5G_node_t		   *sym = NULL;
-    H5WB_t                 *wb = NULL;     /* Wrapped buffer for node data */
-    uint8_t                 node_buf[H5G_NODE_BUF_SIZE]; /* Buffer for node */
-    uint8_t		   *node;           /* Pointer to node buffer */
+    H5G_node_t         *sym = NULL;
+    H5WB_t             *wb = NULL;                      /* Wrapped buffer for node data */
+    uint8_t             node_buf[H5G_NODE_BUF_SIZE];    /* Buffer for node */
+    uint8_t            *node;                           /* Pointer to node buffer */
     const uint8_t	   *p;
-    H5G_node_t		   *ret_value;	/*for error handling */
+    const uint8_t	   *p_end;
+    H5G_node_t         *ret_value;                      /* Return value */
 
     FUNC_ENTER_NOAPI_NOINIT
 
-    /*
-     * Check arguments.
-     */
+    /* Sanity checks */
     HDassert(f);
     HDassert(H5F_addr_defined(addr));
     HDassert(udata);
 
-    /*
-     * Initialize variables.
-     */
-
     /* Allocate symbol table data structures */
     if(NULL == (sym = H5FL_CALLOC(H5G_node_t)))
-	HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed")
+        HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed")
     sym->node_size = H5G_NODE_SIZE(f);
     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")
+        HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed")
 
     /* Wrap the local buffer for serialized node info */
     if(NULL == (wb = H5WB_wrap(node_buf, sizeof(node_buf))))
@@ -157,19 +151,24 @@ H5G_node_load(H5F_t *f, hid_t dxpl_id, haddr_t addr, void *udata)
 
     /* Read the serialized symbol table node. */
     if(H5F_block_read(f, H5FD_MEM_BTREE, addr, sym->node_size, dxpl_id, node) < 0)
-	HGOTO_ERROR(H5E_SYM, H5E_READERROR, NULL, "unable to read symbol table node")
+        HGOTO_ERROR(H5E_SYM, H5E_READERROR, NULL, "unable to read symbol table node")
 
     /* Get temporary pointer to serialized node */
     p = node;
 
+    /* Get a pointer to the end of the node. This ensures we don't run off
+     * the end of the buffer if the file is corrupt.
+     */
+    p_end = p + sym->node_size - 1;
+
     /* magic */
     if(HDmemcmp(p, H5G_NODE_MAGIC, (size_t)H5_SIZEOF_MAGIC))
-	HGOTO_ERROR(H5E_SYM, H5E_CANTLOAD, NULL, "bad symbol table node signature")
+        HGOTO_ERROR(H5E_SYM, H5E_CANTLOAD, NULL, "bad symbol table node signature")
     p += 4;
 
     /* version */
     if(H5G_NODE_VERS != *p++)
-	HGOTO_ERROR(H5E_SYM, H5E_CANTLOAD, NULL, "bad symbol table node version")
+        HGOTO_ERROR(H5E_SYM, H5E_CANTLOAD, NULL, "bad symbol table node version")
 
     /* reserved */
     p++;
@@ -178,8 +177,8 @@ H5G_node_load(H5F_t *f, hid_t dxpl_id, haddr_t addr, void *udata)
     UINT16DECODE(p, sym->nsyms);
 
     /* entries */
-    if(H5G__ent_decode_vec(f, &p, sym->entry, sym->nsyms) < 0)
-	HGOTO_ERROR(H5E_SYM, H5E_CANTLOAD, NULL, "unable to decode symbol table entries")
+    if(H5G__ent_decode_vec(f, &p, p_end, sym->entry, sym->nsyms) < 0)
+        HGOTO_ERROR(H5E_SYM, H5E_CANTLOAD, NULL, "unable to decode symbol table entries")
 
     /* Set return value */
     ret_value = sym;
diff --git a/src/H5Gent.c b/src/H5Gent.c
index bb6aa38..1bd8e63 100644
--- a/src/H5Gent.c
+++ b/src/H5Gent.c
@@ -91,7 +91,8 @@ H5FL_BLK_EXTERN(str_buf);
  *-------------------------------------------------------------------------
  */
 herr_t
-H5G__ent_decode_vec(const H5F_t *f, const uint8_t **pp, H5G_entry_t *ent, unsigned n)
+H5G__ent_decode_vec(const H5F_t *f, const uint8_t **pp, const uint8_t *p_end,
+        H5G_entry_t *ent, unsigned n)
 {
     unsigned    u;                      /* Local index variable */
     herr_t      ret_value = SUCCEED;    /* Return value */
@@ -104,9 +105,12 @@ H5G__ent_decode_vec(const H5F_t *f, const uint8_t **pp, H5G_entry_t *ent, unsign
     HDassert(ent);
 
     /* decode entries */
-    for(u = 0; u < n; u++)
+    for(u = 0; u < n; u++) {
+        if(*pp > p_end)
+            HGOTO_ERROR(H5E_SYM, H5E_CANTDECODE, FAIL, "ran off the end of the buffer")
         if(H5G_ent_decode(f, pp, ent + u) < 0)
             HGOTO_ERROR(H5E_SYM, H5E_CANTDECODE, FAIL, "can't decode")
+    }
 
 done:
     FUNC_LEAVE_NOAPI(ret_value)
diff --git a/src/H5Gpkg.h b/src/H5Gpkg.h
index 07aff02..f49ef26 100644
--- a/src/H5Gpkg.h
+++ b/src/H5Gpkg.h
@@ -1,4 +1,4 @@
-/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
+/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
  * Copyright by The HDF Group.                                               *
  * Copyright by the Board of Trustees of the University of Illinois.         *
  * All rights reserved.                                                      *
@@ -400,9 +400,9 @@ H5_DLL void H5G__ent_copy(H5G_entry_t *dst, const H5G_entry_t *src,
             H5_copy_depth_t depth);
 H5_DLL void H5G__ent_reset(H5G_entry_t *ent);
 H5_DLL herr_t H5G__ent_decode_vec(const H5F_t *f, const uint8_t **pp,
-				  H5G_entry_t *ent, unsigned n);
+				    const uint8_t *p_end, H5G_entry_t *ent, unsigned n);
 H5_DLL herr_t H5G__ent_encode_vec(const H5F_t *f, uint8_t **pp,
-				  const H5G_entry_t *ent, unsigned n);
+                    const H5G_entry_t *ent, unsigned n);
 H5_DLL herr_t H5G__ent_convert(H5F_t *f, hid_t dxpl_id, H5HL_t *heap,
     const char *name, const H5O_link_t *lnk, H5O_type_t obj_type,
     const void *crt_info, H5G_entry_t *ent);