diff options
author | mattjala <124107509+mattjala@users.noreply.github.com> | 2023-05-18 16:13:34 (GMT) |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-05-18 16:13:34 (GMT) |
commit | 8dcc7dc17ff4e797edc5570a6eaa1d8da691b97e (patch) | |
tree | 53b1f43c643978533a0e679aa07a8169f4367ee0 /src | |
parent | d5143567c8f08f6f6de80ffcffa3ede98634ca78 (diff) | |
download | hdf5-8dcc7dc17ff4e797edc5570a6eaa1d8da691b97e.zip hdf5-8dcc7dc17ff4e797edc5570a6eaa1d8da691b97e.tar.gz hdf5-8dcc7dc17ff4e797edc5570a6eaa1d8da691b97e.tar.bz2 |
Prevent buffer overrun in H5S_select_deserialize (#2956)
Diffstat (limited to 'src')
-rw-r--r-- | src/H5Odtype.c | 503 | ||||
-rw-r--r-- | src/H5Olayout.c | 20 | ||||
-rw-r--r-- | src/H5Rint.c | 19 | ||||
-rw-r--r-- | src/H5S.c | 5 | ||||
-rw-r--r-- | src/H5Sall.c | 18 | ||||
-rw-r--r-- | src/H5Shyper.c | 80 | ||||
-rw-r--r-- | src/H5Snone.c | 17 | ||||
-rw-r--r-- | src/H5Spkg.h | 3 | ||||
-rw-r--r-- | src/H5Spoint.c | 76 | ||||
-rw-r--r-- | src/H5Sprivate.h | 4 | ||||
-rw-r--r-- | src/H5Sselect.c | 19 | ||||
-rw-r--r-- | src/H5private.h | 9 |
12 files changed, 560 insertions, 213 deletions
diff --git a/src/H5Odtype.c b/src/H5Odtype.c index c13f80d..b6aaa7a 100644 --- a/src/H5Odtype.c +++ b/src/H5Odtype.c @@ -108,35 +108,46 @@ const H5O_msg_class_t H5O_MSG_DTYPE[1] = {{ }}; /*------------------------------------------------------------------------- - * Function: H5O__dtype_decode_helper + * Function: H5O__dtype_decode_helper * - * Purpose: Decodes a datatype + * Purpose: Decodes a datatype * - * Return: TRUE if we can upgrade the parent type's version even + * Return: TRUE if we can upgrade the parent type's version even * with strict format checks * FALSE if we cannot - * Negative on failure - * - * Programmer: Robb Matzke - * Monday, December 8, 1997 - * + * NEGATIVE on failure *------------------------------------------------------------------------- */ static htri_t -H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t *dt) +H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t *dt, hbool_t skip, + const uint8_t *p_end) { - unsigned flags, version; - unsigned i; - size_t z; - htri_t ret_value = FALSE; /* Return value */ + unsigned flags; + unsigned version; + htri_t ret_value = FALSE; - FUNC_ENTER_STATIC + FUNC_ENTER_PACKAGE - /* check args */ HDassert(pp && *pp); HDassert(dt && dt->shared); + /* XXX NOTE! + * + * H5Tencode() does not take a buffer size, so normal bounds checking in + * that case is impossible. + * + * Instead of using our normal H5_IS_BUFFER_OVERFLOW macro, use + * H5_IS_KNOWN_BUFFER_OVERFLOW, which will skip the check when the + * we're decoding a buffer from H5Tconvert(). + * + * Even if this is fixed at some point in the future, as long as we + * support the old, size-less API call, we will need to use the modified + * macros. + */ + /* Version, class & flags */ + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, 4, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); UINT32DECODE(*pp, flags); version = (flags >> 4) & 0x0f; if (version < H5O_DTYPE_VERSION_1 || version > H5O_DTYPE_VERSION_LATEST) @@ -146,8 +157,14 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t flags >>= 8; /* Size */ + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, 4, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); UINT32DECODE(*pp, dt->shared->size); + /* Check for invalid datatype size */ + if (dt->shared->size == 0) + HGOTO_ERROR(H5E_OHDR, H5E_BADVALUE, FAIL, "invalid datatype size") + switch (dt->shared->type) { case H5T_INTEGER: /* @@ -157,6 +174,8 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t dt->shared->u.atomic.lsb_pad = (flags & 0x2) ? H5T_PAD_ONE : H5T_PAD_ZERO; dt->shared->u.atomic.msb_pad = (flags & 0x4) ? H5T_PAD_ONE : H5T_PAD_ZERO; dt->shared->u.atomic.u.i.sign = (flags & 0x8) ? H5T_SGN_2 : H5T_SGN_NONE; + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, 2 + 2, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); UINT16DECODE(*pp, dt->shared->u.atomic.offset); UINT16DECODE(*pp, dt->shared->u.atomic.prec); break; @@ -174,7 +193,7 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t /* VAX order if both 1st and 6th bits are turned on*/ if (flags & 0x40) dt->shared->u.atomic.order = H5T_ORDER_VAX; - } /* end if */ + } dt->shared->u.atomic.lsb_pad = (flags & 0x2) ? H5T_PAD_ONE : H5T_PAD_ZERO; dt->shared->u.atomic.msb_pad = (flags & 0x4) ? H5T_PAD_ONE : H5T_PAD_ZERO; dt->shared->u.atomic.u.f.pad = (flags & 0x8) ? H5T_PAD_ONE : H5T_PAD_ZERO; @@ -193,21 +212,40 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t default: HGOTO_ERROR(H5E_DATATYPE, H5E_UNSUPPORTED, FAIL, "unknown floating-point normalization") - } /* end switch */ + } dt->shared->u.atomic.u.f.sign = (flags >> 8) & 0xff; + + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, 2 + 2, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); UINT16DECODE(*pp, dt->shared->u.atomic.offset); UINT16DECODE(*pp, dt->shared->u.atomic.prec); + + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, 1 + 1, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); dt->shared->u.atomic.u.f.epos = *(*pp)++; dt->shared->u.atomic.u.f.esize = *(*pp)++; - HDassert(dt->shared->u.atomic.u.f.esize > 0); + if (dt->shared->u.atomic.u.f.esize == 0) + HGOTO_ERROR(H5E_DATATYPE, H5E_BADVALUE, FAIL, "exponent size can't be zero") + + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, 1 + 1, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); dt->shared->u.atomic.u.f.mpos = *(*pp)++; dt->shared->u.atomic.u.f.msize = *(*pp)++; - HDassert(dt->shared->u.atomic.u.f.msize > 0); + if (dt->shared->u.atomic.u.f.msize == 0) + HGOTO_ERROR(H5E_DATATYPE, H5E_BADVALUE, FAIL, "mantissa size can't be zero") + + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, 4, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); UINT32DECODE(*pp, dt->shared->u.atomic.u.f.ebias); break; - case H5T_TIME: /* Time datatypes */ + case H5T_TIME: + /* + * Time datatypes... + */ dt->shared->u.atomic.order = (flags & 0x1) ? H5T_ORDER_BE : H5T_ORDER_LE; + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, 2, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); UINT16DECODE(*pp, dt->shared->u.atomic.prec); break; @@ -232,29 +270,42 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t dt->shared->u.atomic.order = (flags & 0x1) ? H5T_ORDER_BE : H5T_ORDER_LE; dt->shared->u.atomic.lsb_pad = (flags & 0x2) ? H5T_PAD_ONE : H5T_PAD_ZERO; dt->shared->u.atomic.msb_pad = (flags & 0x4) ? H5T_PAD_ONE : H5T_PAD_ZERO; + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, 2 + 2, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); UINT16DECODE(*pp, dt->shared->u.atomic.offset); UINT16DECODE(*pp, dt->shared->u.atomic.prec); break; - case H5T_OPAQUE: + case H5T_OPAQUE: { + size_t z; + /* * Opaque types... */ + + /* The opaque tag flag field must be aligned */ z = flags & (H5T_OPAQUE_TAG_MAX - 1); - HDassert(0 == (z & 0x7)); /*must be aligned*/ + if (0 != (z & 0x7)) + HGOTO_ERROR(H5E_OHDR, H5E_BADVALUE, FAIL, "opaque flag field must be aligned") + if (NULL == (dt->shared->u.opaque.tag = (char *)H5MM_malloc(z + 1))) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed") + + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, z, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); H5MM_memcpy(dt->shared->u.opaque.tag, *pp, z); dt->shared->u.opaque.tag[z] = '\0'; + *pp += z; break; + } case H5T_COMPOUND: { + unsigned nmembs; /* Number of compound members */ unsigned offset_nbytes; /* Size needed to encode member offsets */ size_t max_memb_pos = 0; /* Maximum member covered, so far */ unsigned max_version = 0; /* Maximum member version */ unsigned upgrade_to = 0; /* Version number we can "soft" upgrade to */ - unsigned j; /* Compute the # of bytes required to store a member offset */ offset_nbytes = H5VM_limit_enc_size((uint64_t)dt->shared->size); @@ -262,76 +313,146 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t /* * Compound datatypes... */ - dt->shared->u.compnd.nmembs = flags & 0xffff; - if (dt->shared->u.compnd.nmembs == 0) - HGOTO_ERROR(H5E_DATATYPE, H5E_BADVALUE, FAIL, "invalid number of members: %u", - dt->shared->u.compnd.nmembs) - dt->shared->u.compnd.nalloc = dt->shared->u.compnd.nmembs; - dt->shared->u.compnd.memb = - (H5T_cmemb_t *)H5MM_calloc(dt->shared->u.compnd.nalloc * sizeof(H5T_cmemb_t)); - dt->shared->u.compnd.memb_size = 0; - if (NULL == dt->shared->u.compnd.memb) + nmembs = flags & 0xffff; + if (nmembs == 0) + HGOTO_ERROR(H5E_DATATYPE, H5E_BADVALUE, FAIL, "invalid number of members: %u", nmembs) + if (NULL == + (dt->shared->u.compnd.memb = (H5T_cmemb_t *)H5MM_calloc(nmembs * sizeof(H5T_cmemb_t)))) HGOTO_ERROR(H5E_DATATYPE, H5E_CANTALLOC, FAIL, "memory allocation failed") - for (i = 0; i < dt->shared->u.compnd.nmembs; i++) { - unsigned ndims = 0; /* Number of dimensions of the array field */ - htri_t can_upgrade; /* Whether we can upgrade this type's version */ - hsize_t dim[H5O_LAYOUT_NDIMS]; /* Dimensions of the array */ - H5T_t *array_dt; /* Temporary pointer to the array datatype */ - H5T_t *temp_type; /* Temporary pointer to the field's datatype */ + dt->shared->u.compnd.nalloc = nmembs; + + if (dt->shared->u.compnd.memb_size != 0) + HGOTO_ERROR(H5E_DATATYPE, H5E_BADVALUE, FAIL, "member size not initialized to zero") + + for (dt->shared->u.compnd.nmembs = 0; dt->shared->u.compnd.nmembs < nmembs; + dt->shared->u.compnd.nmembs++) { + + size_t actual_name_length = 0; /* Actual length of name */ + unsigned ndims = 0; /* Number of dimensions of the array field */ + htri_t can_upgrade; /* Whether we can upgrade this type's version */ + hsize_t dim[H5O_LAYOUT_NDIMS]; /* Dimensions of the array */ + H5T_t *array_dt; /* Temporary pointer to the array datatype */ + H5T_t *temp_type; /* Temporary pointer to the field's datatype */ + + /* Get the length of the field name */ + if (!skip) { + /* There is a realistic buffer end, so check bounds */ + + size_t max = (size_t)(p_end - *pp + 1); /* Max possible name length */ + + actual_name_length = HDstrnlen((const char *)*pp, max); + if (actual_name_length == max) + HGOTO_ERROR(H5E_OHDR, H5E_NOSPACE, FAIL, "field name not null terminated") + } + else { + /* The buffer end can't be determined when it's an unbounded buffer + * passed via H5Tdecode(), so don't bounds check and hope for + * the best. + */ + actual_name_length = HDstrlen((const char *)*pp); + } + + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, actual_name_length, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); /* Decode the field name */ - dt->shared->u.compnd.memb[i].name = H5MM_xstrdup((const char *)*pp); + if (NULL == (dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].name = + H5MM_xstrdup((const char *)*pp))) + HGOTO_ERROR(H5E_RESOURCE, H5E_CANTCOPY, FAIL, + "can't duplicate compound member name string") /* Version 3 of the datatype message eliminated the padding to multiple of 8 bytes */ - if (version >= H5O_DTYPE_VERSION_3) + if (version >= H5O_DTYPE_VERSION_3) { /* Advance past name, including null terminator */ - *pp += HDstrlen((const char *)*pp) + 1; - else + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, actual_name_length + 1, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, + "ran off end of input buffer while decoding"); + *pp += actual_name_length + 1; + } + else { /* Advance multiple of 8 w/ null terminator */ - *pp += ((HDstrlen((const char *)*pp) + 8) / 8) * 8; + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, ((actual_name_length + 8) / 8) * 8, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, + "ran off end of input buffer while decoding"); + *pp += ((actual_name_length + 8) / 8) * 8; + } /* Decode the field offset */ /* (starting with version 3 of the datatype message, use the minimum # of bytes required) */ - if (version >= H5O_DTYPE_VERSION_3) - UINT32DECODE_VAR(*pp, dt->shared->u.compnd.memb[i].offset, offset_nbytes) - else - UINT32DECODE(*pp, dt->shared->u.compnd.memb[i].offset) + if (version >= H5O_DTYPE_VERSION_3) { + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, offset_nbytes, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, + "ran off end of input buffer while decoding"); + UINT32DECODE_VAR(*pp, dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].offset, + offset_nbytes) + } + else { + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, 4, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, + "ran off end of input buffer while decoding"); + UINT32DECODE(*pp, dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].offset) + } /* Older versions of the library allowed a field to have * intrinsic 'arrayness'. Newer versions of the library * use the separate array datatypes. */ if (version == H5O_DTYPE_VERSION_1) { /* Decode the number of dimensions */ + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, 1, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, + "ran off end of input buffer while decoding"); ndims = *(*pp)++; /* Check that ndims is valid */ - if (ndims > 4) + if (ndims > 4) { + dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].name = + H5MM_xfree(dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].name); HGOTO_ERROR(H5E_DATATYPE, H5E_BADTYPE, FAIL, "invalid number of dimensions for array") + } - *pp += 3; /*reserved bytes */ + /* Skip reserved bytes */ + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, 3, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, + "ran off end of input buffer while decoding"); + *pp += 3; /* Skip dimension permutation */ + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, 4, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, + "ran off end of input buffer while decoding"); *pp += 4; /* Skip reserved bytes */ + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, 4, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, + "ran off end of input buffer while decoding"); *pp += 4; /* Decode array dimension sizes */ - for (j = 0; j < 4; j++) - UINT32DECODE(*pp, dim[j]); - } /* end if */ + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, (4 * 4), p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, + "ran off end of input buffer while decoding"); + for (int i = 0; i < 4; i++) + UINT32DECODE(*pp, dim[i]); + } /* Allocate space for the field's datatype */ - if (NULL == (temp_type = H5T__alloc())) + if (NULL == (temp_type = H5T__alloc())) { + dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].name = + H5MM_xfree(dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].name); HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed") + } /* Decode the field's datatype information */ - if ((can_upgrade = H5O__dtype_decode_helper(ioflags, pp, temp_type)) < 0) { - for (j = 0; j <= i; j++) - H5MM_xfree(dt->shared->u.compnd.memb[j].name); - H5MM_xfree(dt->shared->u.compnd.memb); + if ((can_upgrade = H5O__dtype_decode_helper(ioflags, pp, temp_type, skip, p_end)) < 0) { + dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].name = + H5MM_xfree(dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].name); + if (H5T_close_real(temp_type) < 0) + HDONE_ERROR(H5E_DATATYPE, H5E_CANTRELEASE, FAIL, "can't release datatype info") HGOTO_ERROR(H5E_DATATYPE, H5E_CANTDECODE, FAIL, "unable to decode member type") - } /* end if */ + } + if (temp_type->shared->size == 0) + HGOTO_ERROR(H5E_DATATYPE, H5E_CANTDECODE, FAIL, "type size can't be zero") /* Upgrade the version if we can and it is necessary */ if (can_upgrade && temp_type->shared->version > version) { @@ -339,7 +460,7 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t /* Pass "can_upgrade" flag down to parent type */ ret_value = TRUE; - } /* end if */ + } /* Go create the array datatype now, for older versions of the datatype message */ if (version == H5O_DTYPE_VERSION_1) { @@ -347,15 +468,21 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t if (ndims > 0) { /* Create the array datatype for the field */ if ((array_dt = H5T__array_create(temp_type, ndims, dim)) == NULL) { - for (j = 0; j <= i; j++) - H5MM_xfree(dt->shared->u.compnd.memb[j].name); - H5MM_xfree(dt->shared->u.compnd.memb); + dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].name = + H5MM_xfree(dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].name); + if (H5T_close_real(temp_type) < 0) + HDONE_ERROR(H5E_DATATYPE, H5E_CANTRELEASE, FAIL, + "can't release datatype info") HGOTO_ERROR(H5E_DATATYPE, H5E_CANTREGISTER, FAIL, "unable to create array datatype") - } /* end if */ + } /* Close the base type for the array */ - (void)H5T_close_real(temp_type); + if (H5T_close_real(temp_type) < 0) { + dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].name = + H5MM_xfree(dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].name); + HGOTO_ERROR(H5E_DATATYPE, H5E_CANTRELEASE, FAIL, "can't release datatype info") + } /* Make the array type the type that is set for the field */ temp_type = array_dt; @@ -371,43 +498,51 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t /* Set the return value to indicate that we should freely * upgrade parent types */ ret_value = TRUE; - } /* end else */ - } /* end if */ - } /* end if */ + } + } + } /* Keep track of the maximum member version found */ if (temp_type->shared->version > max_version) max_version = temp_type->shared->version; - /* - * Set the "force conversion" flag if VL datatype fields exist in this + /* Set the "force conversion" flag if VL datatype fields exist in this * type or any component types */ if (temp_type->shared->force_conv == TRUE) dt->shared->force_conv = TRUE; /* Member size */ - dt->shared->u.compnd.memb[i].size = temp_type->shared->size; + dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].size = temp_type->shared->size; dt->shared->u.compnd.memb_size += temp_type->shared->size; /* Set the field datatype (finally :-) */ - dt->shared->u.compnd.memb[i].type = temp_type; - - /* Check if this field overlaps with a prior field */ - /* (probably indicates that the file is corrupt) */ - if (i > 0 && dt->shared->u.compnd.memb[i].offset < max_memb_pos) { - for (j = 0; j < i; j++) - if (dt->shared->u.compnd.memb[i].offset >= dt->shared->u.compnd.memb[j].offset && - dt->shared->u.compnd.memb[i].offset < - (dt->shared->u.compnd.memb[j].offset + dt->shared->u.compnd.memb[j].size)) + dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].type = temp_type; + + /* Check if this field overlaps with a prior field + * (probably indicates that the file is corrupt) + */ + if (dt->shared->u.compnd.nmembs > 0 && + dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].offset < max_memb_pos) { + for (unsigned u = 0; u < dt->shared->u.compnd.nmembs; u++) + if ((dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].offset >= + dt->shared->u.compnd.memb[u].offset && + dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].offset < + (dt->shared->u.compnd.memb[u].offset + dt->shared->u.compnd.memb[u].size)) || + (dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].offset < + dt->shared->u.compnd.memb[u].offset && + (dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].offset + + dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].size) > + dt->shared->u.compnd.memb[u].offset)) HGOTO_ERROR(H5E_DATATYPE, H5E_CANTDECODE, FAIL, "member overlaps with previous member") - } /* end if */ + } /* Update the maximum member position covered */ - max_memb_pos = MAX(max_memb_pos, - (dt->shared->u.compnd.memb[i].offset + dt->shared->u.compnd.memb[i].size)); - } /* end for */ + max_memb_pos = + MAX(max_memb_pos, (dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].offset + + dt->shared->u.compnd.memb[dt->shared->u.compnd.nmembs].size)); + } /* Check if the compound type is packed */ H5T__update_packed(dt); @@ -420,14 +555,17 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t /* We won't mark the message dirty since there were no * errors in the file, simply type versions that we will no * longer encode. */ - } /* end if */ + } /* Check that no member of this compound has a version greater * than the compound itself. */ H5O_DTYPE_CHECK_VERSION(dt, version, max_version, ioflags, "compound", FAIL) } break; - case H5T_REFERENCE: /* Reference datatypes... */ + case H5T_REFERENCE: + /* + * Reference datatypes... + */ dt->shared->u.atomic.order = H5T_ORDER_NONE; dt->shared->u.atomic.prec = 8 * dt->shared->size; dt->shared->u.atomic.offset = 0; @@ -461,57 +599,102 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t HGOTO_ERROR(H5E_DATATYPE, H5E_CANTINIT, FAIL, "invalid datatype location") break; - case H5T_ENUM: + case H5T_ENUM: { + unsigned nmembs; + /* * Enumeration datatypes... */ - dt->shared->u.enumer.nmembs = dt->shared->u.enumer.nalloc = flags & 0xffff; + nmembs = flags & 0xffff; if (NULL == (dt->shared->parent = H5T__alloc())) - HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed") - if (H5O__dtype_decode_helper(ioflags, pp, dt->shared->parent) < 0) + HGOTO_ERROR(H5E_RESOURCE, H5E_CANTALLOC, FAIL, "can't allocate parent datatype") + if (H5O__dtype_decode_helper(ioflags, pp, dt->shared->parent, skip, p_end) < 0) HGOTO_ERROR(H5E_DATATYPE, H5E_CANTDECODE, FAIL, "unable to decode parent datatype") + if (dt->shared->parent->shared->size != dt->shared->size) + HGOTO_ERROR(H5E_DATATYPE, H5E_BADSIZE, FAIL, "ENUM datatype size does not match parent") /* Check if the parent of this enum has a version greater than the * enum itself. */ H5O_DTYPE_CHECK_VERSION(dt, version, dt->shared->parent->shared->version, ioflags, "enum", FAIL) - if (NULL == (dt->shared->u.enumer.name = - (char **)H5MM_calloc(dt->shared->u.enumer.nalloc * sizeof(char *))) || - NULL == (dt->shared->u.enumer.value = (uint8_t *)H5MM_calloc( - dt->shared->u.enumer.nalloc * dt->shared->parent->shared->size))) - HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed") + /* Allocate name and value arrays */ + if (NULL == (dt->shared->u.enumer.name = (char **)H5MM_calloc(nmembs * sizeof(char *))) || + NULL == (dt->shared->u.enumer.value = + (uint8_t *)H5MM_calloc(nmembs * dt->shared->parent->shared->size))) + HGOTO_ERROR(H5E_RESOURCE, H5E_CANTALLOC, FAIL, "memory allocation failed") + dt->shared->u.enumer.nalloc = nmembs; /* Names */ - for (i = 0; i < dt->shared->u.enumer.nmembs; i++) { - dt->shared->u.enumer.name[i] = H5MM_xstrdup((const char *)*pp); + for (dt->shared->u.enumer.nmembs = 0; dt->shared->u.enumer.nmembs < nmembs; + dt->shared->u.enumer.nmembs++) { + + size_t actual_name_length = 0; /* Actual length of name */ + + /* Get the length of the enum name */ + if (!skip) { + /* There is a realistic buffer end, so check bounds */ + + size_t max = (size_t)(p_end - *pp + 1); /* Max possible name length */ + + actual_name_length = HDstrnlen((const char *)*pp, max); + if (actual_name_length == max) + HGOTO_ERROR(H5E_OHDR, H5E_NOSPACE, FAIL, "enum name not null terminated") + } + else { + /* The buffer end can't be determined when it's an unbounded buffer + * passed via H5Tdecode(), so don't bounds check and hope for + * the best. + */ + actual_name_length = HDstrlen((const char *)*pp); + } + + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, actual_name_length, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); + if (NULL == (dt->shared->u.enumer.name[dt->shared->u.enumer.nmembs] = + H5MM_xstrdup((const char *)*pp))) + HGOTO_ERROR(H5E_RESOURCE, H5E_CANTCOPY, FAIL, "can't duplicate enum name string") /* Version 3 of the datatype message eliminated the padding to multiple of 8 bytes */ - if (version >= H5O_DTYPE_VERSION_3) + if (version >= H5O_DTYPE_VERSION_3) { /* Advance past name, including null terminator */ - *pp += HDstrlen((const char *)*pp) + 1; - else + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, actual_name_length + 1, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, + "ran off end of input buffer while decoding"); + *pp += actual_name_length + 1; + } + else { /* Advance multiple of 8 w/ null terminator */ - *pp += ((HDstrlen((const char *)*pp) + 8) / 8) * 8; - } /* end for */ + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, ((actual_name_length + 8) / 8) * 8, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, + "ran off end of input buffer while decoding"); + *pp += ((actual_name_length + 8) / 8) * 8; + } + } + if (dt->shared->u.enumer.nmembs != nmembs) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, "incorrect number of enum members decoded"); /* Values */ - H5MM_memcpy(dt->shared->u.enumer.value, *pp, - dt->shared->u.enumer.nmembs * dt->shared->parent->shared->size); - *pp += dt->shared->u.enumer.nmembs * dt->shared->parent->shared->size; - break; + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, nmembs * dt->shared->parent->shared->size, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); + H5MM_memcpy(dt->shared->u.enumer.value, *pp, nmembs * dt->shared->parent->shared->size); + *pp += nmembs * dt->shared->parent->shared->size; + } break; - case H5T_VLEN: /* Variable length datatypes... */ + case H5T_VLEN: + /* + * Variable length datatypes... + */ /* Set the type of VL information, either sequence or string */ dt->shared->u.vlen.type = (H5T_vlen_type_t)(flags & 0x0f); if (dt->shared->u.vlen.type == H5T_VLEN_STRING) { dt->shared->u.vlen.pad = (H5T_str_t)((flags >> 4) & 0x0f); dt->shared->u.vlen.cset = (H5T_cset_t)((flags >> 8) & 0x0f); - } /* end if */ + } /* Decode base type of VL information */ if (NULL == (dt->shared->parent = H5T__alloc())) HGOTO_ERROR(H5E_DATATYPE, H5E_NOSPACE, FAIL, "memory allocation failed") - if (H5O__dtype_decode_helper(ioflags, pp, dt->shared->parent) < 0) + if (H5O__dtype_decode_helper(ioflags, pp, dt->shared->parent, skip, p_end) < 0) HGOTO_ERROR(H5E_DATATYPE, H5E_CANTDECODE, FAIL, "unable to decode VL parent type") /* Check if the parent of this vlen has a version greater than the @@ -526,8 +709,13 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t HGOTO_ERROR(H5E_DATATYPE, H5E_CANTINIT, FAIL, "invalid datatype location") break; - case H5T_ARRAY: /* Array datatypes */ + case H5T_ARRAY: + /* + * Array datatypes... + */ /* Decode the number of dimensions */ + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, 1, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); dt->shared->u.array.ndims = *(*pp)++; /* Double-check the number of dimensions */ @@ -535,23 +723,32 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t HGOTO_ERROR(H5E_DATATYPE, H5E_CANTLOAD, FAIL, "too many dimensions for array datatype") /* Skip reserved bytes, if version has them */ - if (version < H5O_DTYPE_VERSION_3) + if (version < H5O_DTYPE_VERSION_3) { + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, 3, p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); *pp += 3; + } /* Decode array dimension sizes & compute number of elements */ - for (i = 0, dt->shared->u.array.nelem = 1; i < (unsigned)dt->shared->u.array.ndims; i++) { - UINT32DECODE(*pp, dt->shared->u.array.dim[i]); - dt->shared->u.array.nelem *= dt->shared->u.array.dim[i]; - } /* end for */ + dt->shared->u.array.nelem = 1; + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, (dt->shared->u.array.ndims * 4), p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); + for (unsigned u = 0; u < dt->shared->u.array.ndims; u++) { + UINT32DECODE(*pp, dt->shared->u.array.dim[u]); + dt->shared->u.array.nelem *= dt->shared->u.array.dim[u]; + } /* Skip array dimension permutations, if version has them */ - if (version < H5O_DTYPE_VERSION_3) + if (version < H5O_DTYPE_VERSION_3) { + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *pp, (dt->shared->u.array.ndims * 4), p_end)) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding"); *pp += dt->shared->u.array.ndims * 4; + } /* Decode base type of array */ if (NULL == (dt->shared->parent = H5T__alloc())) HGOTO_ERROR(H5E_DATATYPE, H5E_NOSPACE, FAIL, "memory allocation failed") - if (H5O__dtype_decode_helper(ioflags, pp, dt->shared->parent) < 0) + if (H5O__dtype_decode_helper(ioflags, pp, dt->shared->parent, skip, p_end) < 0) HGOTO_ERROR(H5E_DATATYPE, H5E_CANTDECODE, FAIL, "unable to decode array parent type") /* Check if the parent of this array has a version greater than the @@ -561,8 +758,7 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t /* There should be no array datatypes with version < 2. */ H5O_DTYPE_CHECK_VERSION(dt, version, H5O_DTYPE_VERSION_2, ioflags, "array", FAIL) - /* - * Set the "force conversion" flag if a VL base datatype is used or + /* Set the "force conversion" flag if a VL base datatype is used or * or if any components of the base datatype are VL types. */ if (dt->shared->parent->shared->force_conv == TRUE) @@ -573,17 +769,15 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t case H5T_NCLASSES: default: HGOTO_ERROR(H5E_DATATYPE, H5E_UNSUPPORTED, FAIL, "unknown datatype class found") - } /* end switch */ + } done: + /* Cleanup on error */ if (ret_value < 0) - if (dt != NULL) { - if (dt->shared != NULL) { - HDassert(!dt->shared->owned_vol_obj); - dt->shared = H5FL_FREE(H5T_shared_t, dt->shared); - } /* end if */ - dt = H5FL_FREE(H5T_t, dt); - } /* end if */ + /* Release (reset) dt but do not free it - leave it as an empty datatype as was the case on + * function entry */ + if (H5T__free(dt) < 0) + HDONE_ERROR(H5E_DATATYPE, H5E_CANTRELEASE, FAIL, "can't release datatype info") FUNC_LEAVE_NOAPI(ret_value) } /* end H5O__dtype_decode_helper() */ @@ -613,7 +807,7 @@ H5O__dtype_encode_helper(uint8_t **pp, const H5T_t *dt) size_t n, z; herr_t ret_value = SUCCEED; /* Return value */ - FUNC_ENTER_STATIC + FUNC_ENTER_PACKAGE /* check args */ HDassert(pp && *pp); @@ -1113,33 +1307,48 @@ done: Pointer to the new message in native order on success, NULL on failure DESCRIPTION This function decodes the "raw" disk form of a simple datatype message - into a struct in memory native format. The struct is allocated within this - function using malloc() and is returned to the caller. + into a struct in memory native format. The struct is allocated within this + function using malloc() and is returned to the caller. --------------------------------------------------------------------------*/ static void * H5O__dtype_decode(H5F_t H5_ATTR_UNUSED *f, H5O_t H5_ATTR_UNUSED *open_oh, unsigned H5_ATTR_UNUSED mesg_flags, - unsigned *ioflags /*in,out*/, size_t H5_ATTR_UNUSED p_size, const uint8_t *p) + unsigned *ioflags /*in,out*/, size_t p_size, const uint8_t *p) { - H5T_t *dt = NULL; - void *ret_value = NULL; /* Return value */ + hbool_t skip; + H5T_t *dt = NULL; + const uint8_t *p_end = p + p_size - 1; + void *ret_value = NULL; - FUNC_ENTER_STATIC + FUNC_ENTER_PACKAGE - /* check args */ + HDassert(f); HDassert(p); /* Allocate datatype message */ if (NULL == (dt = H5T__alloc())) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed") + /* If we are decoding a buffer from H5Tdecode(), we won't have the size + * of the buffer and bounds checking will be impossible. In this case, + * the library will have set p_size to SIZE_MAX and we can use that + * as a signal to skip bounds checking. + */ + skip = (p_size == SIZE_MAX ? TRUE : FALSE); + /* Perform actual decode of message */ - if (H5O__dtype_decode_helper(ioflags, &p, dt) < 0) + if (H5O__dtype_decode_helper(ioflags, &p, dt, skip, p_end) < 0) HGOTO_ERROR(H5E_DATATYPE, H5E_CANTDECODE, NULL, "can't decode type") /* Set return value */ ret_value = dt; done: + /* Cleanup on error */ + if (!ret_value) + /* Free dt */ + if (H5T_close_real(dt) < 0) + HDONE_ERROR(H5E_DATATYPE, H5E_CANTRELEASE, NULL, "can't release datatype info") + FUNC_LEAVE_NOAPI(ret_value) } /* end H5O__dtype_decode() */ @@ -1166,7 +1375,7 @@ H5O__dtype_encode(H5F_t H5_ATTR_UNUSED *f, uint8_t *p, const void *mesg) const H5T_t *dt = (const H5T_t *)mesg; herr_t ret_value = SUCCEED; /* Return value */ - FUNC_ENTER_STATIC + FUNC_ENTER_PACKAGE /* check args */ HDassert(f); @@ -1205,7 +1414,7 @@ H5O__dtype_copy(const void *_src, void *_dst) H5T_t *dst; void *ret_value = NULL; /* Return value */ - FUNC_ENTER_STATIC + FUNC_ENTER_PACKAGE /* check args */ HDassert(src); @@ -1251,7 +1460,7 @@ H5O__dtype_size(const H5F_t *f, const void *_mesg) unsigned u; /* Local index variable */ size_t ret_value = 0; /* Return value */ - FUNC_ENTER_STATIC_NOERR + FUNC_ENTER_PACKAGE_NOERR HDassert(f); HDassert(dt); @@ -1379,7 +1588,7 @@ H5O__dtype_reset(void *_mesg) { H5T_t *dt = (H5T_t *)_mesg; - FUNC_ENTER_STATIC_NOERR + FUNC_ENTER_PACKAGE_NOERR if (dt) H5T__free(dt); @@ -1404,7 +1613,7 @@ H5O__dtype_free(void *mesg) { herr_t ret_value = SUCCEED; /* Return value */ - FUNC_ENTER_STATIC + FUNC_ENTER_PACKAGE /* Sanity check */ HDassert(mesg); @@ -1435,7 +1644,7 @@ H5O__dtype_set_share(void *_mesg /*in,out*/, const H5O_shared_t *sh) H5T_t *dt = (H5T_t *)_mesg; herr_t ret_value = SUCCEED; - FUNC_ENTER_STATIC + FUNC_ENTER_PACKAGE HDassert(dt); HDassert(sh); @@ -1491,7 +1700,7 @@ H5O__dtype_can_share(const void *_mesg) htri_t tri_ret; htri_t ret_value = TRUE; - FUNC_ENTER_STATIC + FUNC_ENTER_PACKAGE HDassert(mesg); @@ -1534,7 +1743,7 @@ H5O__dtype_pre_copy_file(H5F_t *file_src, const void *mesg_src, hbool_t H5_ATTR_ H5D_copy_file_ud_t *udata = (H5D_copy_file_ud_t *)_udata; /* Dataset copying user data */ herr_t ret_value = SUCCEED; /* Return value */ - FUNC_ENTER_STATIC + FUNC_ENTER_PACKAGE /* check args */ HDassert(file_src); @@ -1589,7 +1798,7 @@ H5O__dtype_copy_file(H5F_t H5_ATTR_UNUSED *file_src, const H5O_msg_class_t *mesg H5T_t *dst_mesg; /* Destination datatype */ void *ret_value = NULL; /* Return value */ - FUNC_ENTER_STATIC + FUNC_ENTER_PACKAGE /* Perform a normal copy of the object header message */ if (NULL == (dst_mesg = (H5T_t *)H5O__dtype_copy(native_src, NULL))) @@ -1629,7 +1838,7 @@ H5O__dtype_shared_post_copy_upd(const H5O_loc_t H5_ATTR_UNUSED *src_oloc, const H5T_t *dt_dst = (H5T_t *)mesg_dst; /* Destination datatype */ herr_t ret_value = SUCCEED; /* Return value */ - FUNC_ENTER_STATIC + FUNC_ENTER_PACKAGE if (dt_dst->sh_loc.type == H5O_SHARE_TYPE_COMMITTED) { HDassert(H5T_is_named(dt_dst)); @@ -1673,7 +1882,7 @@ H5O__dtype_debug(H5F_t *f, const void *mesg, FILE *stream, int indent, int fwidt unsigned i; size_t k; - FUNC_ENTER_STATIC_NOERR + FUNC_ENTER_PACKAGE_NOERR /* check args */ HDassert(f); @@ -2139,4 +2348,4 @@ H5O__dtype_debug(H5F_t *f, const void *mesg, FILE *stream, int indent, int fwidt } /* end else */ FUNC_LEAVE_NOAPI(SUCCEED) -} /* end H5O__dtype_debug() */ +} /* end H5O__dtype_debug() */
\ No newline at end of file diff --git a/src/H5Olayout.c b/src/H5Olayout.c index ed69fb1..0c5c993 100644 --- a/src/H5Olayout.c +++ b/src/H5Olayout.c @@ -639,13 +639,27 @@ H5O__layout_decode(H5F_t *f, H5O_t H5_ATTR_UNUSED *open_oh, unsigned H5_ATTR_UNU heap_block_p += tmp_size; /* Source selection */ - if (H5S_SELECT_DESERIALIZE(&mesg->storage.u.virt.list[i].source_select, - &heap_block_p) < 0) + avail_buffer_space = heap_block_p_end - heap_block_p + 1; + + if (avail_buffer_space <= 0) + HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, NULL, + "buffer overflow while decoding layout") + + if (H5S_SELECT_DESERIALIZE(&mesg->storage.u.virt.list[i].source_select, &heap_block_p, + (size_t)(avail_buffer_space)) < 0) HGOTO_ERROR(H5E_OHDR, H5E_CANTDECODE, NULL, "can't decode source space selection") /* Virtual selection */ + + /* Buffer space must be updated after previous deserialization */ + avail_buffer_space = heap_block_p_end - heap_block_p + 1; + + if (avail_buffer_space <= 0) + HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, NULL, + "buffer overflow while decoding layout") + if (H5S_SELECT_DESERIALIZE(&mesg->storage.u.virt.list[i].source_dset.virtual_select, - &heap_block_p) < 0) + &heap_block_p, (size_t)(avail_buffer_space)) < 0) HGOTO_ERROR(H5E_OHDR, H5E_CANTDECODE, NULL, "can't decode virtual space selection") diff --git a/src/H5Rint.c b/src/H5Rint.c index 656df7b..d46bbe5 100644 --- a/src/H5Rint.c +++ b/src/H5Rint.c @@ -1252,6 +1252,7 @@ static herr_t H5R__decode_region(const unsigned char *buf, size_t *nbytes, H5S_t **space_ptr) { const uint8_t *p = (const uint8_t *)buf; + const uint8_t *p_end = p + *nbytes - 1; size_t buf_size = 0; unsigned rank; H5S_t *space; @@ -1284,7 +1285,11 @@ H5R__decode_region(const unsigned char *buf, size_t *nbytes, H5S_t **space_ptr) HGOTO_ERROR(H5E_REFERENCE, H5E_CANTDECODE, FAIL, "Buffer size is too small") if (H5S_set_extent_simple(space, rank, NULL, NULL) < 0) HGOTO_ERROR(H5E_REFERENCE, H5E_CANTSET, FAIL, "can't set extent rank for selection") - if (H5S_SELECT_DESERIALIZE(&space, &p) < 0) + + if (p - 1 > p_end) + HGOTO_ERROR(H5E_REFERENCE, H5E_CANTDECODE, FAIL, "Ran off end of buffer while decoding") + + if (H5S_SELECT_DESERIALIZE(&space, &p, (size_t)(p_end - p + 1)) < 0) HGOTO_ERROR(H5E_REFERENCE, H5E_CANTDECODE, FAIL, "can't deserialize selection") *nbytes = buf_size; @@ -1547,7 +1552,8 @@ H5R__decode_token_region_compat(H5F_t *f, const unsigned char *buf, size_t *nbyt unsigned char *data = NULL; H5O_token_t token = {0}; size_t data_size; - const uint8_t *p; + const uint8_t *p = NULL; + const uint8_t *p_end = NULL; H5S_t *space = NULL; herr_t ret_value = SUCCEED; @@ -1563,7 +1569,8 @@ H5R__decode_token_region_compat(H5F_t *f, const unsigned char *buf, size_t *nbyt HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier") /* Get object address */ - p = (const uint8_t *)data; + p = (const uint8_t *)data; + p_end = p + data_size - 1; H5MM_memcpy(&token, p, token_size); p += token_size; @@ -1583,7 +1590,11 @@ H5R__decode_token_region_compat(H5F_t *f, const unsigned char *buf, size_t *nbyt HGOTO_ERROR(H5E_REFERENCE, H5E_NOTFOUND, FAIL, "not found") /* Unserialize the selection */ - if (H5S_SELECT_DESERIALIZE(&space, &p) < 0) + + if (p - 1 >= p_end) + HGOTO_ERROR(H5E_REFERENCE, H5E_CANTDECODE, FAIL, "Ran off end of buffer while deserializing") + + if (H5S_SELECT_DESERIALIZE(&space, &p, (size_t)(p_end - p + 1)) < 0) HGOTO_ERROR(H5E_REFERENCE, H5E_CANTDECODE, FAIL, "can't deserialize selection") *space_ptr = space; @@ -1695,9 +1695,10 @@ H5S_decode(const unsigned char **p) if (H5S_select_all(ds, FALSE) < 0) HGOTO_ERROR(H5E_DATASPACE, H5E_CANTSET, NULL, "unable to set all selection") - /* Decode the select part of dataspace. I believe this part always exists. */ + /* Decode the select part of dataspace. + * Because size of buffer is unknown, assume arbitrarily large buffer to allow decoding. */ *p = pp; - if (H5S_SELECT_DESERIALIZE(&ds, p) < 0) + if (H5S_SELECT_DESERIALIZE(&ds, p, SIZE_MAX) < 0) HGOTO_ERROR(H5E_DATASPACE, H5E_CANTDECODE, NULL, "can't decode space selection") /* Set return value */ diff --git a/src/H5Sall.c b/src/H5Sall.c index a1ae04e..00ab1e2 100644 --- a/src/H5Sall.c +++ b/src/H5Sall.c @@ -50,7 +50,7 @@ static herr_t H5S__all_release(H5S_t *space); static htri_t H5S__all_is_valid(const H5S_t *space); static hssize_t H5S__all_serial_size(H5S_t *space); static herr_t H5S__all_serialize(H5S_t *space, uint8_t **p); -static herr_t H5S__all_deserialize(H5S_t **space, const uint8_t **p); +static herr_t H5S__all_deserialize(H5S_t **space, const uint8_t **p, const size_t p_size, hbool_t skip); static herr_t H5S__all_bounds(const H5S_t *space, hsize_t *start, hsize_t *end); static herr_t H5S__all_offset(const H5S_t *space, hsize_t *off); static int H5S__all_unlim_dim(const H5S_t *space); @@ -637,13 +637,13 @@ H5S__all_serialize(H5S_t *space, uint8_t **p) REVISION LOG --------------------------------------------------------------------------*/ static herr_t -H5S__all_deserialize(H5S_t **space, const uint8_t **p) +H5S__all_deserialize(H5S_t **space, const uint8_t **p, const size_t p_size, hbool_t skip) { - uint32_t version; /* Version number */ - H5S_t *tmp_space = NULL; /* Pointer to actual dataspace to use, - either *space or a newly allocated one */ - herr_t ret_value = SUCCEED; /* return value */ - + uint32_t version; /* Version number */ + H5S_t *tmp_space = NULL; /* Pointer to actual dataspace to use, + either *space or a newly allocated one */ + 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_STATIC HDassert(p); @@ -663,12 +663,16 @@ H5S__all_deserialize(H5S_t **space, const uint8_t **p) tmp_space = *space; /* Decode version */ + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *p, sizeof(uint32_t), p_end)) + HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, "buffer overflow while decoding selection version") UINT32DECODE(*p, version); if (version < H5S_ALL_VERSION_1 || version > H5S_ALL_VERSION_LATEST) HGOTO_ERROR(H5E_DATASPACE, H5E_BADVALUE, FAIL, "bad version number for all selection") /* Skip over the remainder of the header */ + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *p, 8, p_end)) + HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, "buffer overflow while decoding header") *p += 8; /* Change to "all" selection */ diff --git a/src/H5Shyper.c b/src/H5Shyper.c index 1f614ba..db8d573 100644 --- a/src/H5Shyper.c +++ b/src/H5Shyper.c @@ -174,7 +174,7 @@ static htri_t H5S__hyper_is_valid(const H5S_t *space); static hsize_t H5S__hyper_span_nblocks(H5S_hyper_span_info_t *spans); static hssize_t H5S__hyper_serial_size(H5S_t *space); static herr_t H5S__hyper_serialize(H5S_t *space, uint8_t **p); -static herr_t H5S__hyper_deserialize(H5S_t **space, const uint8_t **p); +static herr_t H5S__hyper_deserialize(H5S_t **space, const uint8_t **p, const size_t p_size, hbool_t skip); static herr_t H5S__hyper_bounds(const H5S_t *space, hsize_t *start, hsize_t *end); static herr_t H5S__hyper_offset(const H5S_t *space, hsize_t *offset); static int H5S__hyper_unlim_dim(const H5S_t *space); @@ -4220,21 +4220,21 @@ done: REVISION LOG --------------------------------------------------------------------------*/ static herr_t -H5S__hyper_deserialize(H5S_t **space, const uint8_t **p) +H5S__hyper_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 */ - hsize_t start[H5S_MAX_RANK]; /* hyperslab start information */ - hsize_t block[H5S_MAX_RANK]; /* hyperslab block information */ - uint32_t version; /* Version number */ - uint8_t flags = 0; /* Flags */ - uint8_t enc_size = 0; /* Encoded size of selection info */ - unsigned rank; /* rank of points */ - const uint8_t *pp; /* Local pointer for decoding */ - unsigned u; /* Local counting variable */ - herr_t ret_value = FAIL; /* 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 */ + hsize_t start[H5S_MAX_RANK]; /* hyperslab start information */ + hsize_t block[H5S_MAX_RANK]; /* hyperslab block information */ + uint32_t version; /* Version number */ + uint8_t flags = 0; /* Flags */ + uint8_t enc_size = 0; /* Encoded size of selection info */ + unsigned rank; /* rank of points */ + const uint8_t *pp; /* Local pointer for decoding */ + unsigned u; /* Local counting variable */ + herr_t ret_value = FAIL; /* return value */ + const uint8_t *p_end = *p + p_size - 1; /* Pointer to last valid byte in buffer */ FUNC_ENTER_STATIC /* Check args */ @@ -4255,6 +4255,8 @@ H5S__hyper_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_HYPER_VERSION_1 || version > H5S_HYPER_VERSION_LATEST) @@ -4262,13 +4264,22 @@ H5S__hyper_deserialize(H5S_t **space, const uint8_t **p) if (version >= (uint32_t)H5S_HYPER_VERSION_2) { /* Decode flags */ + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, pp, 1, p_end)) + HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, "buffer overflow while decoding selection flags") flags = *(pp)++; - if (version >= (uint32_t)H5S_HYPER_VERSION_3) + if (version >= (uint32_t)H5S_HYPER_VERSION_3) { /* decode size of offset info */ + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, pp, 1, p_end)) + HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, + "buffer overflow while decoding selection encoding size") enc_size = *(pp)++; + } else { /* Skip over the remainder of the header */ + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, pp, 4, p_end)) + HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, + "buffer overflow while decoding selection header") pp += 4; enc_size = H5S_SELECT_INFO_ENC_SIZE_8; } /* end else */ @@ -4279,6 +4290,8 @@ H5S__hyper_deserialize(H5S_t **space, const uint8_t **p) } 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 header") pp += 8; enc_size = H5S_SELECT_INFO_ENC_SIZE_4; } /* end else */ @@ -4288,6 +4301,8 @@ H5S__hyper_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) { @@ -4314,6 +4329,10 @@ H5S__hyper_deserialize(H5S_t **space, const uint8_t **p) switch (enc_size) { case H5S_SELECT_INFO_ENC_SIZE_2: for (u = 0; u < tmp_space->extent.rank; u++) { + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, pp, 4 * sizeof(uint16_t), p_end)) + HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, + "buffer overflow while decoding selection ranks") + UINT16DECODE(pp, start[u]); UINT16DECODE(pp, stride[u]); @@ -4329,6 +4348,10 @@ H5S__hyper_deserialize(H5S_t **space, const uint8_t **p) case H5S_SELECT_INFO_ENC_SIZE_4: for (u = 0; u < tmp_space->extent.rank; u++) { + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, pp, 4 * sizeof(uint32_t), p_end)) + HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, + "buffer overflow while decoding selection ranks") + UINT32DECODE(pp, start[u]); UINT32DECODE(pp, stride[u]); @@ -4344,6 +4367,10 @@ H5S__hyper_deserialize(H5S_t **space, const uint8_t **p) case H5S_SELECT_INFO_ENC_SIZE_8: for (u = 0; u < tmp_space->extent.rank; u++) { + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, pp, 4 * sizeof(uint64_t), p_end)) + HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, + "buffer overflow while decoding selection ranks") + UINT64DECODE(pp, start[u]); UINT64DECODE(pp, stride[u]); @@ -4379,14 +4406,23 @@ H5S__hyper_deserialize(H5S_t **space, const uint8_t **p) /* Decode the number of blocks */ 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 selection blocks") 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 selection blocks") 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 selection blocks") UINT64DECODE(pp, num_elem); break; @@ -4403,6 +4439,10 @@ H5S__hyper_deserialize(H5S_t **space, const uint8_t **p) /* Decode the starting and ending points */ switch (enc_size) { case H5S_SELECT_INFO_ENC_SIZE_2: + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, pp, rank * 2 * sizeof(uint16_t), p_end)) + HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, + "buffer overflow while decoding selection coordinates") + for (tstart = start, v = 0; v < rank; v++, tstart++) UINT16DECODE(pp, *tstart); for (tend = end, v = 0; v < rank; v++, tend++) @@ -4410,6 +4450,10 @@ H5S__hyper_deserialize(H5S_t **space, const uint8_t **p) break; case H5S_SELECT_INFO_ENC_SIZE_4: + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, pp, rank * 2 * sizeof(uint32_t), p_end)) + HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, + "buffer overflow while decoding selection coordinates") + for (tstart = start, v = 0; v < rank; v++, tstart++) UINT32DECODE(pp, *tstart); for (tend = end, v = 0; v < rank; v++, tend++) @@ -4417,6 +4461,10 @@ H5S__hyper_deserialize(H5S_t **space, const uint8_t **p) break; case H5S_SELECT_INFO_ENC_SIZE_8: + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, pp, rank * 2 * sizeof(uint64_t), p_end)) + HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, + "buffer overflow while decoding selection coordinates") + for (tstart = start, v = 0; v < rank; v++, tstart++) UINT64DECODE(pp, *tstart); for (tend = end, v = 0; v < rank; v++, tend++) diff --git a/src/H5Snone.c b/src/H5Snone.c index de52370..116396b 100644 --- a/src/H5Snone.c +++ b/src/H5Snone.c @@ -50,7 +50,7 @@ static herr_t H5S__none_release(H5S_t *space); static htri_t H5S__none_is_valid(const H5S_t *space); static hssize_t H5S__none_serial_size(H5S_t *space); static herr_t H5S__none_serialize(H5S_t *space, uint8_t **p); -static herr_t H5S__none_deserialize(H5S_t **space, const uint8_t **p); +static herr_t H5S__none_deserialize(H5S_t **space, const uint8_t **p, const size_t p_size, hbool_t skip); static herr_t H5S__none_bounds(const H5S_t *space, hsize_t *start, hsize_t *end); static herr_t H5S__none_offset(const H5S_t *space, hsize_t *off); static int H5S__none_unlim_dim(const H5S_t *space); @@ -593,12 +593,13 @@ H5S__none_serialize(H5S_t *space, uint8_t **p) REVISION LOG --------------------------------------------------------------------------*/ static herr_t -H5S__none_deserialize(H5S_t **space, const uint8_t **p) +H5S__none_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 */ - uint32_t version; /* Version number */ - 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 */ + uint32_t version; /* Version number */ + 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_STATIC @@ -618,12 +619,16 @@ H5S__none_deserialize(H5S_t **space, const uint8_t **p) tmp_space = *space; /* Decode version */ + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *p, sizeof(uint32_t), p_end)) + HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, "buffer overflow while decoding selection version") UINT32DECODE(*p, version); if (version < H5S_NONE_VERSION_1 || version > H5S_NONE_VERSION_LATEST) HGOTO_ERROR(H5E_DATASPACE, H5E_BADVALUE, FAIL, "bad version number for none selection") /* Skip over the remainder of the header */ + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *p, 8, p_end)) + HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, "buffer overflow while decoding selection header") *p += 8; /* Change to "none" selection */ diff --git a/src/H5Spkg.h b/src/H5Spkg.h index 5bd8219..46631f6 100644 --- a/src/H5Spkg.h +++ b/src/H5Spkg.h @@ -245,7 +245,8 @@ typedef hssize_t (*H5S_sel_serial_size_func_t)(H5S_t *space); /* Method to store current selection in "serialized" form (a byte sequence suitable for storing on disk) */ typedef herr_t (*H5S_sel_serialize_func_t)(H5S_t *space, uint8_t **p); /* Method to create selection from "serialized" form (a byte sequence suitable for storing on disk) */ -typedef herr_t (*H5S_sel_deserialize_func_t)(H5S_t **space, const uint8_t **p); +typedef herr_t (*H5S_sel_deserialize_func_t)(H5S_t **space, const uint8_t **p, const size_t p_size, + hbool_t skip); /* Method to determine smallest n-D bounding box containing the current selection */ typedef herr_t (*H5S_sel_bounds_func_t)(const H5S_t *space, hsize_t *start, hsize_t *end); /* Method to determine linear offset of initial element in selection within dataspace */ diff --git a/src/H5Spoint.c b/src/H5Spoint.c index c6fdecd..7ef7c38 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); @@ -1350,20 +1350,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_STATIC /* Check args */ @@ -1384,16 +1384,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; } @@ -1403,6 +1410,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) { @@ -1420,12 +1429,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: @@ -1437,6 +1458,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++) @@ -1444,11 +1488,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; diff --git a/src/H5Sprivate.h b/src/H5Sprivate.h index db627fb..c2fd224 100644 --- a/src/H5Sprivate.h +++ b/src/H5Sprivate.h @@ -190,7 +190,7 @@ typedef struct H5S_sel_iter_op_t { #define H5S_SELECT_SHAPE_SAME(S1, S2) (H5S_select_shape_same(S1, S2)) #define H5S_SELECT_INTERSECT_BLOCK(S, START, END) (H5S_select_intersect_block(S, START, END)) #define H5S_SELECT_RELEASE(S) (H5S_select_release(S)) -#define H5S_SELECT_DESERIALIZE(S, BUF) (H5S_select_deserialize(S, BUF)) +#define H5S_SELECT_DESERIALIZE(S, BUF, BUF_SIZE) (H5S_select_deserialize(S, BUF, BUF_SIZE)) /* Forward declaration of structs used below */ struct H5O_t; @@ -229,7 +229,7 @@ H5_DLL htri_t H5S_extent_equal(const H5S_t *ds1, const H5S_t *ds2); H5_DLL herr_t H5S_extent_copy(H5S_t *dst, const H5S_t *src); /* Operations on selections */ -H5_DLL herr_t H5S_select_deserialize(H5S_t **space, const uint8_t **p); +H5_DLL herr_t H5S_select_deserialize(H5S_t **space, const uint8_t **p, const size_t p_size); H5_DLL H5S_sel_type H5S_get_select_type(const H5S_t *space); H5_DLL herr_t H5S_select_iterate(void *buf, const H5T_t *type, H5S_t *space, const H5S_sel_iter_op_t *op, void *op_data); diff --git a/src/H5Sselect.c b/src/H5Sselect.c index 70f4cef..2a485f5 100644 --- a/src/H5Sselect.c +++ b/src/H5Sselect.c @@ -527,11 +527,12 @@ H5S_select_valid(const H5S_t *space) REVISION LOG --------------------------------------------------------------------------*/ herr_t -H5S_select_deserialize(H5S_t **space, const uint8_t **p) +H5S_select_deserialize(H5S_t **space, const uint8_t **p, const size_t p_size) { - uint32_t sel_type; /* Pointer to the selection type */ - herr_t ret_value = FAIL; /* Return value */ - + uint32_t sel_type; /* Pointer to the selection type */ + herr_t ret_value = FAIL; /* Return value */ + const uint8_t *p_end = *p + p_size - 1; /* Pointer to last valid byte in buffer */ + hbool_t skip = (p_size == SIZE_MAX ? TRUE : FALSE); /* If p_size is unknown, skip buffer checks */ FUNC_ENTER_NOAPI(FAIL) HDassert(space); @@ -539,24 +540,26 @@ H5S_select_deserialize(H5S_t **space, const uint8_t **p) /* Selection-type specific coding is moved to the callbacks. */ /* Decode selection type */ + if (H5_IS_KNOWN_BUFFER_OVERFLOW(skip, *p, sizeof(uint32_t), p_end)) + HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, "buffer overflow while decoding selection type") UINT32DECODE(*p, sel_type); /* Make routine for selection type */ switch (sel_type) { case H5S_SEL_POINTS: /* Sequence of points selected */ - ret_value = (*H5S_sel_point->deserialize)(space, p); + ret_value = (*H5S_sel_point->deserialize)(space, p, p_size - sizeof(uint32_t), skip); break; case H5S_SEL_HYPERSLABS: /* Hyperslab selection defined */ - ret_value = (*H5S_sel_hyper->deserialize)(space, p); + ret_value = (*H5S_sel_hyper->deserialize)(space, p, p_size - sizeof(uint32_t), skip); break; case H5S_SEL_ALL: /* Entire extent selected */ - ret_value = (*H5S_sel_all->deserialize)(space, p); + ret_value = (*H5S_sel_all->deserialize)(space, p, p_size - sizeof(uint32_t), skip); break; case H5S_SEL_NONE: /* Nothing selected */ - ret_value = (*H5S_sel_none->deserialize)(space, p); + ret_value = (*H5S_sel_none->deserialize)(space, p, p_size - sizeof(uint32_t), skip); break; default: diff --git a/src/H5private.h b/src/H5private.h index 7d15312..36a40ad 100644 --- a/src/H5private.h +++ b/src/H5private.h @@ -403,6 +403,15 @@ */ #define H5_IS_BUFFER_OVERFLOW(ptr, size, buffer_end) (((ptr) + (size)-1) > (buffer_end)) +/* Variant of H5_IS_BUFFER_OVERFLOW, used with functions such as H5Tdecode() + * that don't take a size parameter, where we need to skip the bounds checks. + * + * This is a separate macro since we don't want to inflict that behavior on + * the entire library. + */ +#define H5_IS_KNOWN_BUFFER_OVERFLOW(skip, ptr, size, buffer_end) \ + (skip ? FALSE : ((ptr) + (size)-1) > (buffer_end)) + /* * HDF Boolean type. */ |