Harden selection decoding (#6525)

* Harden serialized selection decoding

* Clang-format and expand test

* Guard against rank 0 hyperslab selection in serialization

* Add CHANGELOG entry

* Modify CHANGELOG

---------

Co-authored-by: Alb3e3 <74142887+Alb3e3@users.noreply.github.com>
This commit is contained in:
Matt L
2026-07-17 13:04:11 -05:00
committed by GitHub
co-authored by Alb3e3
parent 01b7084924
commit abbb48b9cb
7 changed files with 184 additions and 10 deletions
+4
View File
@@ -203,6 +203,10 @@ The `h5repack` tool now obtains its default low and high library version bounds
Fixed a bug where a flag in H5Cimage.c wasn't getting set correctly for release builds of HDF5, leading to incorrect error checking when reconstructing metadata cache entries.
### Hardened decoding of serialized dataspace selections against malformed buffers
`H5S_select_deserialize()` and the per-selection-type deserialize callbacks (all, hyperslab, none, and point) previously computed the pointer to the last valid buffer byte as `buffer + size - 1` without first checking the buffer size. A buffer shorter than the 4-byte selection-type header, or a zero-length selection-info buffer, would underflow this computation and produce an out-of-bounds end pointer, defeating subsequent overflow checks. The deserialize routines now reject a buffer that is too small to hold the selection type, and they reject an empty selection-info buffer before deriving the end pointer. Hyperslab decoding additionally now rejects a serialized rank of 0 or greater than `H5S_MAX_RANK`. As a companion fix, `H5S__hyper_serialize()` now returns an error when asked to serialize a hyperslab selection on a rank-0 (scalar or null) dataspace, a state that can arise when a dataspace extent is collapsed to a scalar after a hyperslab selection has already been made.
## Java Library
## Configuration
+2
View File
@@ -636,6 +636,8 @@ H5S__all_deserialize(H5S_t **space, const uint8_t **p, const size_t p_size, bool
if (skip)
p_end = *p;
else if (p_size == 0)
HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, "empty all selection buffer");
else
p_end = *p + p_size - 1; /* Pointer to last valid byte in buffer */
+13
View File
@@ -3989,6 +3989,14 @@ H5S__hyper_serialize(H5S_t *space, uint8_t **p)
pp = (*p);
assert(pp);
/* A hyperslab selection is meaningless on a scalar/null dataspace. This
* state can arise if the extent is collapsed to a scalar after the
* selection was made, since changing the extent does not reset an
* existing hyperslab selection. */
if (0 == space->extent.rank)
HGOTO_ERROR(H5E_DATASPACE, H5E_BADVALUE, FAIL,
"cannot serialize hyperslab selection with a rank of 0");
/* Set some convenience values */
ndims = space->extent.rank;
diminfo = space->select.sel_info.hslab->diminfo.opt;
@@ -4263,6 +4271,8 @@ H5S__hyper_deserialize(H5S_t **space, const uint8_t **p, const size_t p_size, bo
if (skip)
p_end = *p;
else if (p_size == 0)
HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, "empty hyperslab selection buffer");
else
p_end = *p + p_size - 1; /* Pointer to last valid byte in buffer */
@@ -4328,6 +4338,9 @@ H5S__hyper_deserialize(H5S_t **space, const uint8_t **p, const size_t p_size, bo
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 (0 == rank || rank > H5S_MAX_RANK)
HGOTO_ERROR(H5E_DATASPACE, H5E_BADVALUE, FAIL, "invalid rank (%u) for serialized hyperslab selection",
rank);
if (!*space) {
/* Patch the rank of the allocated dataspace */
+12 -5
View File
@@ -579,17 +579,24 @@ H5S__none_serialize(H5S_t *space, uint8_t **p)
static herr_t
H5S__none_deserialize(H5S_t **space, const uint8_t **p, const size_t p_size, bool 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 */
const uint8_t *p_end = *p + p_size - 1; /* Pointer to last valid byte in buffer */
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; /* Pointer to last valid byte in buffer */
FUNC_ENTER_PACKAGE
assert(p);
assert(*p);
if (skip)
p_end = *p;
else if (p_size == 0)
HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, "empty none selection buffer");
else
p_end = *p + p_size - 1;
/* As part of the efforts to push all selection-type specific coding
to the callbacks, the coding for the allocation of a null dataspace
is moved from H5S_select_deserialize() in H5Sselect.c to here.
+2
View File
@@ -1375,6 +1375,8 @@ H5S__point_deserialize(H5S_t **space, const uint8_t **p, const size_t p_size, bo
if (skip)
p_end = *p;
else if (p_size == 0)
HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, "empty point selection buffer");
else
p_end = *p + p_size - 1; /* Pointer to last valid byte in buffer */
+9 -5
View File
@@ -526,7 +526,8 @@ 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 */
const uint8_t *p_end = NULL; /* Pointer to last valid byte in buffer */
bool skip = false;
size_t sel_info_size; /* Size of selection-type-specific information */
bool skip = false;
FUNC_ENTER_NOAPI(FAIL)
@@ -536,6 +537,8 @@ H5S_select_deserialize(H5S_t **space, const uint8_t **p, const size_t p_size)
skip = (p_size == SIZE_MAX ? true : false);
if (skip)
p_end = *p;
else if (p_size < sizeof(uint32_t))
HGOTO_ERROR(H5E_DATASPACE, H5E_OVERFLOW, FAIL, "selection buffer too small");
else
p_end = *p + p_size - 1;
@@ -545,23 +548,24 @@ H5S_select_deserialize(H5S_t **space, const uint8_t **p, const size_t p_size)
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);
sel_info_size = (skip ? SIZE_MAX : p_size - sizeof(uint32_t));
/* Make routine for selection type */
switch (sel_type) {
case H5S_SEL_POINTS: /* Sequence of points selected */
ret_value = (*H5S_sel_point->deserialize)(space, p, p_size - sizeof(uint32_t), skip);
ret_value = (*H5S_sel_point->deserialize)(space, p, sel_info_size, skip);
break;
case H5S_SEL_HYPERSLABS: /* Hyperslab selection defined */
ret_value = (*H5S_sel_hyper->deserialize)(space, p, p_size - sizeof(uint32_t), skip);
ret_value = (*H5S_sel_hyper->deserialize)(space, p, sel_info_size, skip);
break;
case H5S_SEL_ALL: /* Entire extent selected */
ret_value = (*H5S_sel_all->deserialize)(space, p, p_size - sizeof(uint32_t), skip);
ret_value = (*H5S_sel_all->deserialize)(space, p, sel_info_size, skip);
break;
case H5S_SEL_NONE: /* Nothing selected */
ret_value = (*H5S_sel_none->deserialize)(space, p, p_size - sizeof(uint32_t), skip);
ret_value = (*H5S_sel_none->deserialize)(space, p, sel_info_size, skip);
break;
default:
+142
View File
@@ -16054,6 +16054,142 @@ test_h5s_set_extent_none(void)
} /* test_h5s_set_extent_none() */
/****************************************************************
**
** test_select_deserialize_invalid:
** Test malformed serialized selections that should fail cleanly.
**
****************************************************************/
static void
test_select_deserialize_invalid(void)
{
uint8_t hyper_bad_rank[14];
uint8_t point_bad_rank[13];
uint8_t short_selection_type[1] = {0};
uint8_t type_only[4];
uint8_t none_selection[16];
uint8_t *w;
const uint8_t *r;
H5S_t *space = NULL;
herr_t ret;
TESTING("deserialization of malformed serialized selections");
w = hyper_bad_rank;
UINT32ENCODE(w, (uint32_t)H5S_SEL_HYPERSLABS);
UINT32ENCODE(w, (uint32_t)H5S_HYPER_VERSION_3);
*w++ = 0;
*w++ = H5S_SELECT_INFO_ENC_SIZE_4;
UINT32ENCODE(w, (uint32_t)H5S_MAX_RANK + 1);
r = hyper_bad_rank;
H5E_BEGIN_TRY
{
ret = H5S_SELECT_DESERIALIZE(&space, &r, sizeof(hyper_bad_rank));
}
H5E_END_TRY
VERIFY(ret, FAIL, "H5S_SELECT_DESERIALIZE");
VERIFY(space, NULL, "H5S_SELECT_DESERIALIZE");
/* A point selection with a rank larger than H5S_MAX_RANK is rejected */
w = point_bad_rank;
UINT32ENCODE(w, (uint32_t)H5S_SEL_POINTS);
UINT32ENCODE(w, (uint32_t)H5S_POINT_VERSION_2);
*w++ = H5S_SELECT_INFO_ENC_SIZE_4;
UINT32ENCODE(w, (uint32_t)H5S_MAX_RANK + 1);
r = point_bad_rank;
H5E_BEGIN_TRY
{
ret = H5S_SELECT_DESERIALIZE(&space, &r, sizeof(point_bad_rank));
}
H5E_END_TRY
VERIFY(ret, FAIL, "H5S_SELECT_DESERIALIZE");
VERIFY(space, NULL, "H5S_SELECT_DESERIALIZE");
r = short_selection_type;
H5E_BEGIN_TRY
{
ret = H5S_SELECT_DESERIALIZE(&space, &r, 0);
}
H5E_END_TRY
VERIFY(ret, FAIL, "H5S_SELECT_DESERIALIZE");
VERIFY(space, NULL, "H5S_SELECT_DESERIALIZE");
/* A buffer holding only a valid selection type with no type-specific
payload passes the outer size check but is rejected by the concrete
deserializer's empty-payload guard */
w = type_only;
UINT32ENCODE(w, (uint32_t)H5S_SEL_HYPERSLABS);
r = type_only;
H5E_BEGIN_TRY
{
ret = H5S_SELECT_DESERIALIZE(&space, &r, sizeof(type_only));
}
H5E_END_TRY
VERIFY(ret, FAIL, "H5S_SELECT_DESERIALIZE");
VERIFY(space, NULL, "H5S_SELECT_DESERIALIZE");
w = none_selection;
UINT32ENCODE(w, (uint32_t)H5S_SEL_NONE);
UINT32ENCODE(w, (uint32_t)H5S_NONE_VERSION_1);
memset(w, 0, 8);
r = none_selection;
ret = H5S_SELECT_DESERIALIZE(&space, &r, SIZE_MAX);
CHECK(ret, FAIL, "H5S_SELECT_DESERIALIZE");
CHECK_PTR(space, "H5S_SELECT_DESERIALIZE");
ret = H5S_close(space);
CHECK(ret, FAIL, "H5S_close");
PASSED();
} /* test_select_deserialize_invalid() */
/****************************************************************
**
** test_select_hyper_serialize_scalar:
** Attempting to serialize a hyperslab selection whose dataspace has been
** collapsed to a scalar extent (rank 0) must fail cleanly.
**
****************************************************************/
static void
test_select_hyper_serialize_scalar(void)
{
hid_t sid = H5I_INVALID_HID;
hsize_t dims[1] = {10};
hsize_t start[1] = {0}, stride[1] = {2}, count[1] = {5}, block[1] = {1};
uint8_t buf[2048];
size_t buf_size = sizeof(buf);
herr_t ret;
TESTING("serialization of hyperslab selection on a scalar extent");
sid = H5Screate_simple(1, dims, NULL);
CHECK(sid, H5I_INVALID_HID, "H5Screate_simple");
ret = H5Sselect_hyperslab(sid, H5S_SELECT_SET, start, stride, count, block);
CHECK(ret, FAIL, "H5Sselect_hyperslab");
/* Collapse the extent to scalar, leaving the rank-mismatched
hyperslab selection in place */
ret = H5Sset_extent_simple(sid, 0, NULL, NULL);
CHECK(ret, FAIL, "H5Sset_extent_simple");
/* Encoding the mismatched selection must fail rather than crash */
H5E_BEGIN_TRY
{
ret = H5Sencode2(sid, buf, &buf_size, H5P_DEFAULT);
}
H5E_END_TRY
VERIFY(ret, FAIL, "H5Sencode2");
ret = H5Sclose(sid);
CHECK(ret, FAIL, "H5Sclose");
PASSED();
} /* test_select_hyper_serialize_scalar() */
/****************************************************************
**
** test_select(): Main H5S selection testing routine.
@@ -16260,6 +16396,12 @@ test_select(void H5_ATTR_UNUSED *params)
*/
test_h5s_set_extent_none();
/* Test malformed serialized selections */
test_select_deserialize_invalid();
/* Test serialization of a hyperslab selection on a scalar extent */
test_select_hyper_serialize_scalar();
} /* test_select() */
/*-------------------------------------------------------------------------