diff options
author | mattjala <124107509+mattjala@users.noreply.github.com> | 2023-05-12 20:22:55 (GMT) |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-05-12 20:22:55 (GMT) |
commit | 364145f144cb68a5635ad9f7dad0e4210e3d513a (patch) | |
tree | 5a5a7b4f2a343c8e96bc486abbbbe4d94fb0b094 /src | |
parent | 0d4a12d7cd0f0c10b385533365fc1f3ebeef8e74 (diff) | |
download | hdf5-364145f144cb68a5635ad9f7dad0e4210e3d513a.zip hdf5-364145f144cb68a5635ad9f7dad0e4210e3d513a.tar.gz hdf5-364145f144cb68a5635ad9f7dad0e4210e3d513a.tar.bz2 |
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.
Diffstat (limited to 'src')
-rw-r--r-- | src/H5Odtype.c | 67 | ||||
-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, 233 insertions, 104 deletions
diff --git a/src/H5Odtype.c b/src/H5Odtype.c index 977e4b1..cab2eb2 100644 --- a/src/H5Odtype.c +++ b/src/H5Odtype.c @@ -24,15 +24,6 @@ #include "H5Tpkg.h" /* Datatypes */ #include "H5VMprivate.h" /* Vectors and arrays */ -/* Variant boundary-checking macro, used here since H5Tdecode() doesn't take a - * size parameter so we need to ignore the bounds checks. - * - * This is a separate macro since we don't want to inflict that behavior on - * the rest of the library. - */ -#define H5_DTYPE_IS_BUFFER_OVERFLOW(skip, ptr, size, buffer_end) \ - (skip ? FALSE : ((ptr) + (size)-1) > (buffer_end)) - /* PRIVATE PROTOTYPES */ static herr_t H5O__dtype_encode(H5F_t *f, uint8_t *p, const void *mesg); static void *H5O__dtype_decode(H5F_t *f, H5O_t *open_oh, unsigned mesg_flags, unsigned *ioflags, @@ -146,7 +137,7 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t * that case is impossible. * * Instead of using our normal H5_IS_BUFFER_OVERFLOW macro, use - * H5_DTYPE_IS_BUFFER_OVERFLOW, which will skip the check when the + * 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 @@ -155,7 +146,7 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t */ /* Version, class & flags */ - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, 4, p_end)) + 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; @@ -166,7 +157,7 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t flags >>= 8; /* Size */ - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, 4, p_end)) + 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); @@ -183,7 +174,7 @@ 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_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, 2 + 2, p_end)) + 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); @@ -224,26 +215,26 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t } dt->shared->u.atomic.u.f.sign = (flags >> 8) & 0xff; - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, 2 + 2, p_end)) + 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_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, 1 + 1, p_end)) + 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)++; if (dt->shared->u.atomic.u.f.esize == 0) HGOTO_ERROR(H5E_DATATYPE, H5E_BADVALUE, FAIL, "exponent size can't be zero") - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, 1 + 1, p_end)) + 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)++; if (dt->shared->u.atomic.u.f.msize == 0) HGOTO_ERROR(H5E_DATATYPE, H5E_BADVALUE, FAIL, "mantissa size can't be zero") - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, 4, p_end)) + 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; @@ -253,7 +244,7 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t * Time datatypes... */ dt->shared->u.atomic.order = (flags & 0x1) ? H5T_ORDER_BE : H5T_ORDER_LE; - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, 2, p_end)) + 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; @@ -279,7 +270,7 @@ 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_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, 2 + 2, p_end)) + 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); @@ -300,7 +291,7 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t if (NULL == (dt->shared->u.opaque.tag = (char *)H5MM_malloc(z + 1))) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed") - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, z, p_end)) + 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'; @@ -361,7 +352,7 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t actual_name_length = HDstrlen((const char *)*pp); } - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, actual_name_length, p_end)) + 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 */ @@ -373,14 +364,14 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t /* Version 3 of the datatype message eliminated the padding to multiple of 8 bytes */ if (version >= H5O_DTYPE_VERSION_3) { /* Advance past name, including null terminator */ - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, actual_name_length + 1, p_end)) + 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 */ - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, ((actual_name_length + 8) / 8) * 8, p_end)) + 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; @@ -389,14 +380,14 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t /* Decode the field offset */ /* (starting with version 3 of the datatype message, use the minimum # of bytes required) */ if (version >= H5O_DTYPE_VERSION_3) { - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, offset_nbytes, p_end)) + 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_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, 4, p_end)) + 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) @@ -407,7 +398,7 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t * use the separate array datatypes. */ if (version == H5O_DTYPE_VERSION_1) { /* Decode the number of dimensions */ - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, 1, p_end)) + 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)++; @@ -420,25 +411,25 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t } /* Skip reserved bytes */ - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, 3, p_end)) + 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_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, 4, p_end)) + 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_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, 4, p_end)) + 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 */ - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, (4 * 4), p_end)) + 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++) @@ -657,7 +648,7 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t actual_name_length = HDstrlen((const char *)*pp); } - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, actual_name_length, p_end)) + 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))) @@ -666,14 +657,14 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t /* Version 3 of the datatype message eliminated the padding to multiple of 8 bytes */ if (version >= H5O_DTYPE_VERSION_3) { /* Advance past name, including null terminator */ - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, actual_name_length + 1, p_end)) + 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 */ - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, ((actual_name_length + 8) / 8) * 8, p_end)) + 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; @@ -683,7 +674,7 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, FAIL, "incorrect number of enum members decoded"); /* Values */ - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, nmembs * dt->shared->parent->shared->size, p_end)) + 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; @@ -723,7 +714,7 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t * Array datatypes... */ /* Decode the number of dimensions */ - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, 1, p_end)) + 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)++; @@ -733,14 +724,14 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t /* Skip reserved bytes, if version has them */ if (version < H5O_DTYPE_VERSION_3) { - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, 3, p_end)) + 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 */ dt->shared->u.array.nelem = 1; - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, (dt->shared->u.array.ndims * 4), p_end)) + 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]); @@ -749,7 +740,7 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t /* Skip array dimension permutations, if version has them */ if (version < H5O_DTYPE_VERSION_3) { - if (H5_DTYPE_IS_BUFFER_OVERFLOW(skip, *pp, (dt->shared->u.array.ndims * 4), p_end)) + 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; } diff --git a/src/H5Olayout.c b/src/H5Olayout.c index f784f24..645ad73 100644 --- a/src/H5Olayout.c +++ b/src/H5Olayout.c @@ -634,13 +634,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 4b3d692..7b22150 100644 --- a/src/H5Rint.c +++ b/src/H5Rint.c @@ -1177,6 +1177,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; @@ -1209,7 +1210,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; @@ -1472,7 +1477,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; @@ -1488,7 +1494,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; @@ -1508,7 +1515,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; @@ -1655,9 +1655,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 20c9a20..7f5633f 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_PACKAGE 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 e749ee9..9f567d3 100644 --- a/src/H5Shyper.c +++ b/src/H5Shyper.c @@ -175,7 +175,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); @@ -4239,21 +4239,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_PACKAGE /* Check args */ @@ -4274,6 +4274,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) @@ -4281,13 +4283,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 */ @@ -4298,6 +4309,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 */ @@ -4307,6 +4320,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) { @@ -4333,6 +4348,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]); @@ -4348,6 +4367,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]); @@ -4363,6 +4386,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]); @@ -4398,14 +4425,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; @@ -4422,6 +4458,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++) @@ -4429,6 +4469,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++) @@ -4436,6 +4480,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 b32ac12..c6e862c 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_PACKAGE @@ -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 1163484..4367a4d 100644 --- a/src/H5Spkg.h +++ b/src/H5Spkg.h @@ -242,7 +242,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 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; diff --git a/src/H5Sprivate.h b/src/H5Sprivate.h index 4303eee..ac46d9e 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 02889f7..28c30ea 100644 --- a/src/H5Sselect.c +++ b/src/H5Sselect.c @@ -521,11 +521,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); @@ -533,24 +534,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 eac2cba..91a4780 100644 --- a/src/H5private.h +++ b/src/H5private.h @@ -327,6 +327,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. */ |