diff --git a/changes/281.bugfix b/changes/281.bugfix new file mode 100644 index 0000000..173b735 --- /dev/null +++ b/changes/281.bugfix @@ -0,0 +1,5 @@ +Fixed a heap buffer overflow reading an ndarray whose shape's byte size +overflows 64 bits. `asdf_ndarray_size` and `asdf_ndarray_nbytes` now return +``UINT64_MAX`` rather than wrapping, so the read fails with +``ASDF_NDARRAY_ERR_OUT_OF_BOUNDS``. Tile reads whose origin plus shape +overflows are also out of bounds now. diff --git a/include/asdf/core/ndarray.h b/include/asdf/core/ndarray.h index 2e8c70b..623f642 100644 --- a/include/asdf/core/ndarray.h +++ b/include/asdf/core/ndarray.h @@ -321,7 +321,7 @@ ASDF_EXPORT const void *asdf_ndarray_data_raw(asdf_ndarray_t *ndarray, size_t *s * * :param ndarray: An `asdf_ndarray_t *` * :return: Total number of elements in the array (just the product of its - * shape) + * shape), or ``UINT64_MAX`` if the product does not fit in 64 bits */ ASDF_EXPORT uint64_t asdf_ndarray_size(const asdf_ndarray_t *ndarray); @@ -331,7 +331,7 @@ ASDF_EXPORT uint64_t asdf_ndarray_size(const asdf_ndarray_t *ndarray); * * :param ndarray: An `asdf_ndarray_t *` * :return: The byte size of the ndarray (this is just its size times the - * datatype nbytes) + * datatype nbytes), or ``UINT64_MAX`` if that does not fit in 64 bits */ ASDF_EXPORT uint64_t asdf_ndarray_nbytes(const asdf_ndarray_t *ndarray); diff --git a/src/core/ndarray.c b/src/core/ndarray.c index d09fc3b..6060dfc 100644 --- a/src/core/ndarray.c +++ b/src/core/ndarray.c @@ -1816,11 +1816,23 @@ uint64_t asdf_ndarray_size(const asdf_ndarray_t *ndarray) { return 0; uint64_t size = 1; + bool overflow = false; + + for (uint32_t idx = 0; idx < ndarray->ndim; idx++) { + uint64_t dim = ndarray->shape[idx]; + + // An empty dimension makes the whole array empty + if (dim == 0) + return 0; - for (uint32_t idx = 0; idx < ndarray->ndim; idx++) - size *= ndarray->shape[idx]; + if (size > UINT64_MAX / dim) + overflow = true; + else + size *= dim; + } - return size; + // Saturate rather than wrap, so that size checks against the result fail + return overflow ? UINT64_MAX : size; } @@ -1830,7 +1842,12 @@ uint64_t asdf_ndarray_nbytes(const asdf_ndarray_t *ndarray) { if (UNLIKELY(size == 0)) return size; - return size * asdf_datatype_size((asdf_datatype_t *)&ndarray->datatype); + uint64_t elsize = asdf_datatype_size((asdf_datatype_t *)&ndarray->datatype); + + if (elsize > 0 && size > UINT64_MAX / elsize) + return UINT64_MAX; + + return size * elsize; } @@ -2106,7 +2123,7 @@ static inline bool asdf_ndarray_bounds_check( uint32_t ndim = ndarray->ndim; for (uint32_t idx = 0; idx < ndim; idx++) { - if (origin[idx] + shape[idx] > array_shape[idx]) + if (shape[idx] > array_shape[idx] || origin[idx] > array_shape[idx] - shape[idx]) return false; } diff --git a/tests/Makefile.am b/tests/Makefile.am index 719d4dd..c18b08c 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -439,6 +439,7 @@ EXTRA_DIST += \ fixtures/multiple_hdu.asdf \ fixtures/nested.asdf \ fixtures/ndarray-inline.asdf \ + fixtures/ndarray-size-overflow.asdf \ fixtures/ndarray-out.asdf \ fixtures/numeric.asdf \ fixtures/offset.asdf \ diff --git a/tests/fixtures/ndarray-size-overflow.asdf b/tests/fixtures/ndarray-size-overflow.asdf new file mode 100644 index 0000000..a23e1cd Binary files /dev/null and b/tests/fixtures/ndarray-size-overflow.asdf differ diff --git a/tests/test-ndarray.c b/tests/test-ndarray.c index 42045ce..7fdc814 100644 --- a/tests/test-ndarray.c +++ b/tests/test-ndarray.c @@ -1791,6 +1791,53 @@ MU_TEST(ndarray_extension_embedded_ndarray) { } +/* A shape whose byte size overflows 64 bits is refused rather than read into + * an undersized buffer */ +MU_TEST(ndarray_read_size_overflow) { + const char *path = get_fixture_file_path("ndarray-size-overflow.asdf"); + asdf_file_t *file = asdf_open(path, "r"); + assert_not_null(file); + + asdf_ndarray_t *ndarray = NULL; + assert_int(asdf_get_ndarray(file, "arr", &ndarray), ==, ASDF_VALUE_OK); + assert_not_null(ndarray); + assert_uint64(asdf_ndarray_nbytes(ndarray), ==, UINT64_MAX); + + void *data = NULL; + assert_int( + asdf_ndarray_read_all(ndarray, ASDF_DATATYPE_INT32, &data), + ==, + ASDF_NDARRAY_ERR_OUT_OF_BOUNDS); + assert_null(data); + asdf_ndarray_destroy(ndarray); + asdf_close(file); + return MUNIT_OK; +} + + +/* A tile whose origin plus shape overflows is out of bounds */ +MU_TEST(ndarray_read_tile_bounds_overflow) { + const char *path = get_fixture_file_path("tiles.asdf"); + asdf_file_t *file = asdf_open(path, "r"); + assert_not_null(file); + + asdf_ndarray_t *ndarray = NULL; + assert_int(asdf_get_ndarray(file, "1d", &ndarray), ==, ASDF_VALUE_OK); + assert_not_null(ndarray); + uint64_t origin[] = {UINT64_MAX}; + uint64_t shape[] = {2}; + void *tile = NULL; + assert_int( + asdf_ndarray_read_tile_ndim(ndarray, origin, shape, ASDF_DATATYPE_SOURCE, &tile), + ==, + ASDF_NDARRAY_ERR_OUT_OF_BOUNDS); + assert_null(tile); + asdf_ndarray_destroy(ndarray); + asdf_close(file); + return MUNIT_OK; +} + + MU_TEST_SUITE( ndarray, MU_RUN_TEST(ndarray_read_1d_tile_contiguous), @@ -1813,7 +1860,9 @@ MU_TEST_SUITE( MU_RUN_TEST(ndarray_strides_trivial), MU_RUN_TEST(ndarray_strides_out_of_bounds), MU_RUN_TEST(ndarray_strides_inline_roundtrip), - MU_RUN_TEST(ndarray_extension_embedded_ndarray) + MU_RUN_TEST(ndarray_extension_embedded_ndarray), + MU_RUN_TEST(ndarray_read_size_overflow), + MU_RUN_TEST(ndarray_read_tile_bounds_overflow) );