Fixes a bad memory read and unfreed memory in fsinfo code

MANIFEST

@@ -1168,6 +1168,7 @@
 ./test/cork.c
 ./test/corrupt_stab_msg.h5
 ./test/cross_read.c
+./test/cve_2020_10810.h5
 ./test/dangle.c
 ./test/deflate.h5
 ./test/del_many_dense_attrs.c

release_docs/RELEASE.txt

@@ -924,6 +924,24 @@ Bug Fixes since HDF5-1.12.0 release
 ===================================
     Library
     -------
+    - Fixed an invalid read and memory leak when parsing corrupt file space
+      info messages
+
+        When the corrupt file from CVE-2020-10810 was parsed by the library,
+        the code that imports the version 0 file space info object header
+        message to the version 1 struct could read past the buffer read from
+        the disk, causing an invalid memory read. Not catching this error would
+        cause downstream errors that eventually resulted in a previously
+        allocated buffer to be unfreed when the library shut down. In builds
+        where the free lists are in use, this could result in an infinite loop
+        and SIGABRT when the library shuts down.
+
+        We now track the buffer size and raise an error on attempts to read
+        past the end of it.
+
+        (DER - 2021/08/12, HDFFV-11053)
+
+
     - Fixed CVE-2018-14460
 
         The tool h5repack produced a segfault when the rank in dataspace

src/H5Ocache.c

@@ -78,8 +78,8 @@ static herr_t H5O__cache_chk_free_icr(void *thing);
 static herr_t H5O__prefix_deserialize(const uint8_t *image, H5O_cache_ud_t *udata);
 
 /* Chunk routines */
-static herr_t H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t len, const uint8_t *image,
-                                     H5O_common_cache_ud_t *udata, hbool_t *dirty);
+static herr_t H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t chunk_size, const uint8_t *image,
+                                     size_t len, H5O_common_cache_ud_t *udata, hbool_t *dirty);
 static herr_t H5O__chunk_serialize(const H5F_t *f, H5O_t *oh, unsigned chunkno);
 
 /* Misc. routines */
@@ -287,7 +287,7 @@ H5O__cache_verify_chksum(const void *_image, size_t len, void *_udata)
  *-------------------------------------------------------------------------
  */
 static void *
-H5O__cache_deserialize(const void *image, size_t H5_ATTR_NDEBUG_UNUSED len, void *_udata, hbool_t *dirty)
+H5O__cache_deserialize(const void *image, size_t len, void *_udata, hbool_t *dirty)
 {
     H5O_t *         oh        = NULL;                     /* Object header read in */
     H5O_cache_ud_t *udata     = (H5O_cache_ud_t *)_udata; /* User data for callback */
@@ -333,7 +333,7 @@ H5O__cache_deserialize(const void *image, size_t H5_ATTR_NDEBUG_UNUSED len, void
         oh->proxy = NULL;
 
     /* Parse the first chunk */
-    if (H5O__chunk_deserialize(oh, udata->common.addr, udata->chunk0_size, (const uint8_t *)image,
+    if (H5O__chunk_deserialize(oh, udata->common.addr, udata->chunk0_size, (const uint8_t *)image, len,
                                &(udata->common), dirty) < 0)
         HGOTO_ERROR(H5E_OHDR, H5E_CANTINIT, NULL, "can't deserialize first object header chunk")
 
@@ -736,7 +736,7 @@ H5O__cache_chk_verify_chksum(const void *_image, size_t len, void *_udata)
  *-------------------------------------------------------------------------
  */
 static void *
-H5O__cache_chk_deserialize(const void *image, size_t H5_ATTR_NDEBUG_UNUSED len, void *_udata, hbool_t *dirty)
+H5O__cache_chk_deserialize(const void *image, size_t len, void *_udata, hbool_t *dirty)
 {
     H5O_chunk_proxy_t * chk_proxy = NULL;                         /* Chunk proxy object */
     H5O_chk_cache_ud_t *udata     = (H5O_chk_cache_ud_t *)_udata; /* User data for callback */
@@ -763,7 +763,7 @@ H5O__cache_chk_deserialize(const void *image, size_t H5_ATTR_NDEBUG_UNUSED len,
         HDassert(udata->common.cont_msg_info);
 
         /* Parse the chunk */
-        if (H5O__chunk_deserialize(udata->oh, udata->common.addr, udata->size, (const uint8_t *)image,
+        if (H5O__chunk_deserialize(udata->oh, udata->common.addr, udata->size, (const uint8_t *)image, len,
                                    &(udata->common), dirty) < 0)
             HGOTO_ERROR(H5E_OHDR, H5E_CANTINIT, NULL, "can't deserialize object header chunk")
 
@@ -1275,7 +1275,7 @@ H5O__prefix_deserialize(const uint8_t *_image, H5O_cache_ud_t *udata)
  *-------------------------------------------------------------------------
  */
 static herr_t
-H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t len, const uint8_t *image,
+H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t chunk_size, const uint8_t *image, size_t len,
                        H5O_common_cache_ud_t *udata, hbool_t *dirty)
 {
     const uint8_t *chunk_image;          /* Pointer into buffer to decode */
@@ -1295,6 +1295,7 @@ H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t len, const uint8_t *image
     HDassert(oh);
     HDassert(H5F_addr_defined(addr));
     HDassert(image);
+    HDassert(len);
     HDassert(udata->f);
     HDassert(udata->cont_msg_info);
 
@@ -1315,14 +1316,16 @@ H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t len, const uint8_t *image
     oh->chunk[chunkno].addr = addr;
     if (chunkno == 0)
         /* First chunk's 'image' includes room for the object header prefix */
-        oh->chunk[0].size = len + (size_t)H5O_SIZEOF_HDR(oh);
+        oh->chunk[0].size = chunk_size + (size_t)H5O_SIZEOF_HDR(oh);
     else
-        oh->chunk[chunkno].size = len;
+        oh->chunk[chunkno].size = chunk_size;
     if (NULL == (oh->chunk[chunkno].image = H5FL_BLK_MALLOC(chunk_image, oh->chunk[chunkno].size)))
         HGOTO_ERROR(H5E_OHDR, H5E_CANTALLOC, FAIL, "memory allocation failed")
     oh->chunk[chunkno].chunk_proxy = NULL;
 
     /* Copy disk image into chunk's image */
+    if (len < oh->chunk[chunkno].size)
+        HGOTO_ERROR(H5E_OHDR, H5E_CANTCOPY, FAIL, "attempted to copy too many disk image bytes into buffer")
     H5MM_memcpy(oh->chunk[chunkno].image, image, oh->chunk[chunkno].size);
 
     /* Point into chunk image to decode */

src/H5Ofsinfo.c

@@ -91,11 +91,12 @@ H5FL_DEFINE_STATIC(H5O_fsinfo_t);
  */
 static void *
 H5O__fsinfo_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_fsinfo_t * fsinfo = NULL;    /* File space info message */
-    H5F_mem_page_t ptype;            /* Memory type for iteration */
-    unsigned       vers;             /* message version */
+    H5O_fsinfo_t * fsinfo = NULL; /* File space info message */
+    H5F_mem_page_t ptype;         /* Memory type for iteration */
+    unsigned       vers;          /* message version */
+    const uint8_t *p_end     = p + p_size;
     void *         ret_value = NULL; /* Return value */
 
     FUNC_ENTER_STATIC
@@ -136,8 +137,12 @@ H5O__fsinfo_decode(H5F_t *f, H5O_t H5_ATTR_UNUSED *open_oh, unsigned H5_ATTR_UNU
                 fsinfo->threshold = threshold;
                 if (HADDR_UNDEF == (fsinfo->eoa_pre_fsm_fsalloc = H5F_get_eoa(f, H5FD_MEM_DEFAULT)))
                     HGOTO_ERROR(H5E_FILE, H5E_CANTGET, NULL, "unable to get file size")
-                for (type = H5FD_MEM_SUPER; type < H5FD_MEM_NTYPES; type++)
+                for (type = H5FD_MEM_SUPER; type < H5FD_MEM_NTYPES; type++) {
+                    if (p + H5_SIZEOF_HADDR_T > p_end)
+                        HGOTO_ERROR(H5E_FILE, H5E_CANTDECODE, NULL,
+                                    "ran off end of input buffer while decoding")
                     H5F_addr_decode(f, &p, &(fsinfo->fs_addr[type - 1]));
+                }
                 break;
 
             case H5F_FILE_SPACE_ALL:

test/ohdr.c

@@ -456,6 +456,59 @@ test_ohdr_swmr(hbool_t new_format)
     return FAIL;
 } /* test_ohdr_swmr() */
 
+/*
+ * Tests bad object header messages.
+ *
+ * Currently tests for CVE-2020-10810 fixes but can be expanded to handle
+ * other CVE badness.
+ */
+
+/* This is a generated file that can be obtained from:
+ *
+ * https://nvd.nist.gov/vuln/detail/CVE-2020-10810
+ *
+ * It was formerly named H5AC_unpin_entry_POC
+ */
+#define CVE_2020_10810_FILENAME "cve_2020_10810.h5"
+
+static herr_t
+test_ohdr_badness(hid_t fapl)
+{
+    hid_t fid = H5I_INVALID_HID;
+
+    /* CVE-2020-10810 involved a malformed fsinfo message
+     * This test ensures the fundamental problem is fixed. Running it under
+     * valgrind et al. will ensure that the memory leaks and invalid access
+     * are fixed.
+     */
+    TESTING("Fix for CVE-2020-10810");
+
+    H5E_BEGIN_TRY
+    {
+        /* This should fail due to the malformed fsinfo message. It should
+         * fail gracefully and not segfault.
+         */
+        fid = H5Fopen(CVE_2020_10810_FILENAME, H5F_ACC_RDWR, fapl);
+    }
+    H5E_END_TRY;
+
+    if (fid >= 0)
+        FAIL_PUTS_ERROR("should not have been able to open malformed file");
+
+    PASSED();
+
+    return SUCCEED;
+
+error:
+    H5E_BEGIN_TRY
+    {
+        H5Fclose(fid);
+    }
+    H5E_END_TRY;
+
+    return FAIL;
+}
+
 /*
  *  To test objects with unknown messages in a file with:
  *      a) H5O_BOGUS_VALID_ID:
@@ -2047,6 +2100,9 @@ main(void)
         } /* high */
     }     /* low */
 
+    /* Verify bad ohdr message fixes work */
+    test_ohdr_badness(fapl);
+
     /* Verify symbol table messages are cached */
     if (h5_verify_cached_stabs(FILENAME, fapl) < 0)
         TEST_ERROR