Prevent buffer overrun in H5S_select_deserialize (#2931)

* Prevent buffer overrun in H5S_select_deserialize

The call to H5S_select_deserialize from H5S_decode doesn't have
the buffer size available to it, so to allow decoding there
I set it to assume a max size buffer for now.

Making the buffer size known in H5S_decode could be done by
modifying the external API's H5Sdecode, or splitting H5Sdecode
into two functions using a macro (similar to H5Sencode), with the
macro taking one argument and assuming a max buffer size.

* Conditional buffer check in H5S_select_deserialize

Moved and renamed a macro for only checking buffer overflow when
buffer size is known from H5Odtype.c to H5private.h,
so it can be used throughout the library.

Also silenced some build warnings about types.

1 files changed, 59 insertions, 17 deletions

diff --git a/src/H5Spoint.c b/src/H5Spoint.c
index 9cac3c4..b393c8d 100644
--- a/src/H5Spoint.c
+++ b/src/H5Spoint.c
@@ -60,7 +60,7 @@ static herr_t   H5S__point_release(H5S_t *space);
 static htri_t   H5S__point_is_valid(const H5S_t *space);
 static hssize_t H5S__point_serial_size(H5S_t *space);
 static herr_t   H5S__point_serialize(H5S_t *space, uint8_t **p);
-static herr_t   H5S__point_deserialize(H5S_t **space, const uint8_t **p);
+static herr_t   H5S__point_deserialize(H5S_t **space, const uint8_t **p, const size_t p_size, hbool_t skip);
 static herr_t   H5S__point_bounds(const H5S_t *space, hsize_t *start, hsize_t *end);
 static herr_t   H5S__point_offset(const H5S_t *space, hsize_t *off);
 static int      H5S__point_unlim_dim(const H5S_t *space);
@@ -1352,20 +1352,20 @@ done:
  REVISION LOG
 --------------------------------------------------------------------------*/
 static herr_t
-H5S__point_deserialize(H5S_t **space, const uint8_t **p)
+H5S__point_deserialize(H5S_t **space, const uint8_t **p, const size_t p_size, hbool_t skip)
 {
-    H5S_t *tmp_space = NULL;                 /* Pointer to actual dataspace to use,
-                                                either *space or a newly allocated one */
-    hsize_t        dims[H5S_MAX_RANK];       /* Dimension sizes */
-    uint32_t       version;                  /* Version number */
-    uint8_t        enc_size = 0;             /* Encoded size of selection info */
-    hsize_t       *coord    = NULL, *tcoord; /* Pointer to array of elements */
-    const uint8_t *pp;                       /* Local pointer for decoding */
-    uint64_t       num_elem = 0;             /* Number of elements in selection */
-    unsigned       rank;                     /* Rank of points */
-    unsigned       i, j;                     /* local counting variables */
-    herr_t         ret_value = SUCCEED;      /* Return value */
-
+    H5S_t *tmp_space = NULL;                    /* Pointer to actual dataspace to use,
+                                                   either *space or a newly allocated one */
+    hsize_t        dims[H5S_MAX_RANK];          /* Dimension sizes */
+    uint32_t       version;                     /* Version number */
+    uint8_t        enc_size = 0;                /* Encoded size of selection info */
+    hsize_t       *coord    = NULL, *tcoord;    /* Pointer to array of elements */
+    const uint8_t *pp;                          /* Local pointer for decoding */
+    uint64_t       num_elem = 0;                /* Number of elements in selection */
+    unsigned       rank;                        /* Rank of points */
+    unsigned       i, j;                        /* local counting variables */
+    herr_t         ret_value = SUCCEED;         /* Return value */
+    const uint8_t *p_end     = *p + p_size - 1; /* Pointer to last valid byte in buffer */
     FUNC_ENTER_PACKAGE
 
     /* Check args */
@@ -1386,16 +1386,23 @@ H5S__point_deserialize(H5S_t **space, const uint8_t **p)
         tmp_space = *space;
 
     /* Decode version */
+    if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, pp, sizeof(uint32_t), p_end))
+        HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, "buffer overflow while decoding selection version")
     UINT32DECODE(pp, version);
 
     if (version < H5S_POINT_VERSION_1 || version > H5S_POINT_VERSION_LATEST)
         HGOTO_ERROR(H5E_DATASPACE, H5E_BADVALUE, FAIL, "bad version number for point selection")
 
-    if (version >= (uint32_t)H5S_POINT_VERSION_2)
+    if (version >= (uint32_t)H5S_POINT_VERSION_2) {
         /* Decode size of point info */
+        if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, pp, 1, p_end))
+            HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, "buffer overflow while decoding point info")
         enc_size = *(pp)++;
+    }
     else {
         /* Skip over the remainder of the header */
+        if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, pp, 8, p_end))
+            HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, "buffer overflow while decoding selection headers")
         pp += 8;
         enc_size = H5S_SELECT_INFO_ENC_SIZE_4;
     }
@@ -1405,6 +1412,8 @@ H5S__point_deserialize(H5S_t **space, const uint8_t **p)
         HGOTO_ERROR(H5E_DATASPACE, H5E_CANTLOAD, FAIL, "unknown size of point/offset info for selection")
 
     /* Decode the rank of the point selection */
+    if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, pp, sizeof(uint32_t), p_end))
+        HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, "buffer overflow while decoding selection rank")
     UINT32DECODE(pp, rank);
 
     if (!*space) {
@@ -1422,12 +1431,24 @@ H5S__point_deserialize(H5S_t **space, const uint8_t **p)
     /* decode the number of points */
     switch (enc_size) {
         case H5S_SELECT_INFO_ENC_SIZE_2:
+            if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, pp, sizeof(uint16_t), p_end))
+                HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL,
+                            "buffer overflow while decoding number of points")
+
             UINT16DECODE(pp, num_elem);
             break;
         case H5S_SELECT_INFO_ENC_SIZE_4:
+            if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, pp, sizeof(uint32_t), p_end))
+                HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL,
+                            "buffer overflow while decoding number of points")
+
             UINT32DECODE(pp, num_elem);
             break;
         case H5S_SELECT_INFO_ENC_SIZE_8:
+            if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, pp, sizeof(uint64_t), p_end))
+                HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL,
+                            "buffer overflow while decoding number of points")
+
             UINT64DECODE(pp, num_elem);
             break;
         default:
@@ -1439,6 +1460,29 @@ H5S__point_deserialize(H5S_t **space, const uint8_t **p)
     if (NULL == (coord = (hsize_t *)H5MM_malloc(num_elem * rank * sizeof(hsize_t))))
         HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "can't allocate coordinate information")
 
+    /* Determine necessary size of buffer for coordinates */
+    size_t enc_type_size = 0;
+
+    switch (enc_size) {
+        case H5S_SELECT_INFO_ENC_SIZE_2:
+            enc_type_size = sizeof(uint16_t);
+            break;
+        case H5S_SELECT_INFO_ENC_SIZE_4:
+            enc_type_size = sizeof(uint32_t);
+            break;
+        case H5S_SELECT_INFO_ENC_SIZE_8:
+            enc_type_size = sizeof(uint64_t);
+            break;
+        default:
+            HGOTO_ERROR(H5E_DATASPACE, H5E_UNSUPPORTED, FAIL, "unknown point info size")
+            break;
+    }
+
+    size_t coordinate_buffer_requirement = num_elem * rank * enc_type_size;
+
+    if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, pp, coordinate_buffer_requirement, p_end))
+        HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, "buffer overflow while decoding selection coordinates")
+
     /* Retrieve the coordinates from the buffer */
     for (tcoord = coord, i = 0; i < num_elem; i++)
         for (j = 0; j < (unsigned)rank; j++, tcoord++)
@@ -1446,11 +1490,9 @@ H5S__point_deserialize(H5S_t **space, const uint8_t **p)
                 case H5S_SELECT_INFO_ENC_SIZE_2:
                     UINT16DECODE(pp, *tcoord);
                     break;
-
                 case H5S_SELECT_INFO_ENC_SIZE_4:
                     UINT32DECODE(pp, *tcoord);
                     break;
-
                 case H5S_SELECT_INFO_ENC_SIZE_8:
                     UINT64DECODE(pp, *tcoord);
                     break;