Merge pull request #1448 in HDFFV/hdf5 from ~NFORTNE2/hdf5_naf:develop to develop

Avoid potential invalid read when decoding unknown object header message.

* commit '685288beabc4f4383d69ec07be05c067fab78544':
  Delay checking if decoded message's "shareable" flag is appropriate for the message type until we've verified we understand the message type. Reduce size of H5O_msg_class_g to *not* include space for H5O_BOGUS_INVALID.  Make bogus messages shareable.  Add new bogus message test with shareable messages to cover the formerly problematic code.  Re-run gen_bogus.c to add this test case and also to fix the bogus_invalid messages that were no longer H5O_BOGUS_INVLAID due to a new message class being added in a previous commit.  Added comment to remind developers to run gen_bogus.c when adding a new message class.

5 files changed, 34 insertions, 19 deletions

diff --git a/src/H5Obogus.c b/src/H5Obogus.c
index fbf04f2..f3cca0f 100644
--- a/src/H5Obogus.c
+++ b/src/H5Obogus.c
@@ -49,7 +49,7 @@ const H5O_msg_class_t H5O_MSG_BOGUS_VALID[1] = {{
     H5O_BOGUS_VALID_ID,		/*message id number             */
     "bogus valid",             	/*message name for debugging    */
     0,     	                /*native message size           */
-    0,				/* messages are sharable?       */
+    H5O_SHARE_IS_SHARABLE,      /* messages are sharable?       */
     H5O__bogus_decode,        	/*decode message                */
     H5O_bogus_encode,        	/*encode message                */
     NULL,          	        /*copy the native value         */
@@ -73,7 +73,7 @@ const H5O_msg_class_t H5O_MSG_BOGUS_INVALID[1] = {{
     H5O_BOGUS_INVALID_ID, 	/*message id number             */
     "bogus invalid",          	/*message name for debugging    */
     0,                          /*native message size           */
-    0,                          /* messages are sharable?       */
+    H5O_SHARE_IS_SHARABLE,      /* messages are sharable?       */
     H5O__bogus_decode,          /*decode message                */
     H5O_bogus_encode,           /*encode message                */
     NULL,                       /*copy the native value         */
diff --git a/src/H5Ocache.c b/src/H5Ocache.c
index fba4f6e..23c38b9 100644
--- a/src/H5Ocache.c
+++ b/src/H5Ocache.c
@@ -1402,10 +1402,9 @@ H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t len, const uint8_t *image
             HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "bad flag combination for message")
         if((flags & H5O_MSG_FLAG_WAS_UNKNOWN) && !(flags & H5O_MSG_FLAG_MARK_IF_UNKNOWN))
             HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "bad flag combination for message")
-        if((flags & H5O_MSG_FLAG_SHAREABLE)
-                && H5O_msg_class_g[id]
-                && !(H5O_msg_class_g[id]->share_flags & H5O_SHARE_IS_SHARABLE))
-            HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "message of unsharable class flagged as sharable")
+        /* Delay checking the "shareable" flag until we've made sure id
+         * references a valid message class that this version of the library
+         * knows about */
 
         /* Reserved bytes/creation index */
         if(oh->version == H5O_VERSION_1)
@@ -1517,9 +1516,17 @@ H5O__chunk_deserialize(H5O_t *oh, haddr_t addr, size_t len, const uint8_t *image
                     mesgs_modified = TRUE;
                 } /* end if */
             } /* end if */
-            else
+            else {
+                /* Check for message of unshareable class marked as "shareable"
+                 */
+                if((flags & H5O_MSG_FLAG_SHAREABLE)
+                        && H5O_msg_class_g[id]
+                        && !(H5O_msg_class_g[id]->share_flags & H5O_SHARE_IS_SHARABLE))
+                    HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, FAIL, "message of unshareable class flagged as shareable")
+
                 /* Set message class for "known" messages */
                 mesg->type = H5O_msg_class_g[id];
+            } /* end else */
 
             /* Do some inspection/interpretation of new messages from this chunk */
             /* (detect continuation messages, ref. count messages, etc.) */
diff --git a/src/H5Omessage.c b/src/H5Omessage.c
index d6e3dce..18f3706 100644
--- a/src/H5Omessage.c
+++ b/src/H5Omessage.c
@@ -1598,16 +1598,23 @@ H5O_msg_is_shared(unsigned type_id, const void *mesg)
     FUNC_ENTER_NOAPI_NOINIT_NOERR
 
     /* Check args */
-    HDassert(type_id < NELMTS(H5O_msg_class_g));
-    type = H5O_msg_class_g[type_id];    /* map the type ID to the actual type object */
-    HDassert(type);
-    HDassert(mesg);
-
-    /* If messages in a class aren't sharable, then obviously this message isn't shared! :-) */
-    if(type->share_flags & H5O_SHARE_IS_SHARABLE)
-        ret_value = H5O_IS_STORED_SHARED(((const H5O_shared_t *)mesg)->type);
-    else
+#ifdef H5O_ENABLE_BOGUS
+    if(type_id >= NELMTS(H5O_msg_class_g))
         ret_value = FALSE;
+    else
+#endif /* H5O_ENABLE_BOGUS */
+    {
+        HDassert(type_id < NELMTS(H5O_msg_class_g));
+        type = H5O_msg_class_g[type_id];    /* map the type ID to the actual type object */
+        HDassert(type);
+        HDassert(mesg);
+
+        /* If messages in a class aren't sharable, then obviously this message isn't shared! :-) */
+        if(type->share_flags & H5O_SHARE_IS_SHARABLE)
+            ret_value = H5O_IS_STORED_SHARED(((const H5O_shared_t *)mesg)->type);
+        else
+            ret_value = FALSE;
+    } /* end block/else */
 
     FUNC_LEAVE_NOAPI(ret_value)
 } /* end H5O_msg_is_shared() */
diff --git a/src/H5Opkg.h b/src/H5Opkg.h
index dbc8089..668f4f0 100644
--- a/src/H5Opkg.h
+++ b/src/H5Opkg.h
@@ -29,7 +29,7 @@
 #define H5O_NMESGS	8 		/*initial number of messages	     */
 #define H5O_NCHUNKS	2		/*initial number of chunks	     */
 #define H5O_MIN_SIZE	22		/* Min. obj header data size (must be big enough for a message prefix and a continuation message) */
-#define H5O_MSG_TYPES   27              /* # of types of messages            */
+#define H5O_MSG_TYPES   26              /* # of types of messages            */
 #define H5O_MAX_CRT_ORDER_IDX 65535     /* Max. creation order index value   */
 
 /* Versions of object header structure */
diff --git a/src/H5Oprivate.h b/src/H5Oprivate.h
index e0926e2..5987ecf 100644
--- a/src/H5Oprivate.h
+++ b/src/H5Oprivate.h
@@ -227,11 +227,11 @@ typedef struct H5O_copy_t {
  * Note: Must increment H5O_MSG_TYPES in H5Opkg.h and update H5O_msg_class_g
  *      in H5O.c when creating a new message type.  Also bump the value of
  *      H5O_BOGUS_INVALID_ID, below, to be one greater than the value of
- *      H5O_UNKNOWN_ID.
+ *      H5O_UNKNOWN_ID, and re-run gen_bogus.c.
  *
  * (this should never exist in a file)
  */
-#define H5O_BOGUS_INVALID_ID	0x001A  /* "Bogus invalid" Message.  */
+#define H5O_BOGUS_INVALID_ID	0x001a  /* "Bogus invalid" Message.  */
 
 /* Shared object message types.
  * Shared objects can be committed, in which case the shared message contains
@@ -651,6 +651,7 @@ typedef struct H5O_layout_t {
  */
 #define H5O_BOGUS_VALUE         0xdeadbeef
 typedef struct H5O_bogus_t {
+    H5O_shared_t        sh_loc;         /* Shared message info (must be first) */
     unsigned u;                         /* Hold the bogus info */
 } H5O_bogus_t;
 #endif /* H5O_ENABLE_BOGUS */