From 1e40ea72c1b8e2d8b7f1f11b5569aad3e0201a6d Mon Sep 17 00:00:00 2001 From: cruzzil <1434309+cruzzil@users.noreply.github.com> Date: Sun, 18 Oct 2026 17:37:51 +0800 Subject: [PATCH] fix: heap buffer overflow on ndarray shapes whose size overflows asdf_ndarray_size and asdf_ndarray_nbytes wrapped, so a file could pass the data size check with a tiny byte count while the conversion loop wrote the true element count into a buffer sized from the wrapped one. Saturate both at UINT64_MAX instead. check_bounds had the same problem with origin + shape, letting a tile or asdf_ndarray_read_at index read outside the array. --- changes/281.bugfix | 5 +++ include/asdf/core/ndarray.h | 4 +- src/core/ndarray.c | 27 +++++++++--- tests/Makefile.am | 1 + tests/fixtures/ndarray-size-overflow.asdf | Bin 0 -> 319 bytes tests/test-ndarray.c | 51 +++++++++++++++++++++- 6 files changed, 80 insertions(+), 8 deletions(-) create mode 100644 changes/281.bugfix create mode 100644 tests/fixtures/ndarray-size-overflow.asdf diff --git a/changes/281.bugfix b/changes/281.bugfix new file mode 100644 index 00000000..173b735b --- /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 19ecf246..5f0edfe8 100644 --- a/include/asdf/core/ndarray.h +++ b/include/asdf/core/ndarray.h @@ -297,7 +297,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); @@ -307,7 +307,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 918aa7ca..41651b30 100644 --- a/src/core/ndarray.c +++ b/src/core/ndarray.c @@ -1630,11 +1630,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]; - for (uint32_t idx = 0; idx < ndarray->ndim; idx++) - size *= ndarray->shape[idx]; + // An empty dimension makes the whole array empty + if (dim == 0) + return 0; - return size; + if (size > UINT64_MAX / dim) + overflow = true; + else + size *= dim; + } + + // Saturate rather than wrap, so that size checks against the result fail + return overflow ? UINT64_MAX : size; } @@ -1644,7 +1656,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; } @@ -1846,7 +1863,7 @@ static inline bool check_bounds( 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 bf9b5146..ea160159 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -434,6 +434,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/parse-minimal.asdf \ diff --git a/tests/fixtures/ndarray-size-overflow.asdf b/tests/fixtures/ndarray-size-overflow.asdf new file mode 100644 index 0000000000000000000000000000000000000000..a23e1cdda94974beed884607be7b78c3a1943953 GIT binary patch literal 319 zcmZ9E!A`?45JY=K{)(-tN)Tk%Nm{4P%_xEcg#+ycA%xu6P)jAL?5$9~vp>|0luOym z&YPW`iQZI?kP0azoAml?v(>Maeyhk-WKQ4p%bIk`rd$04V?gs$MvS!=&YnsW?e~mv z&S6})!OeQ+l(JNCgisD` zjU_PMPzVs)Q>a}Dk^xwQ2D>AXZ!lHl^!079j@}O*@!-gdd(%fgR#}$kMJ98#T+EAN av6QP%CWK(;hxPMFj;`ls!2kO?MRWmsl}O?M literal 0 HcmV?d00001 diff --git a/tests/test-ndarray.c b/tests/test-ndarray.c index 3e9e2c67..acbfd878 100644 --- a/tests/test-ndarray.c +++ b/tests/test-ndarray.c @@ -1283,6 +1283,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), @@ -1299,7 +1346,9 @@ MU_TEST_SUITE( MU_RUN_TEST(heap_use_after_free_issue_63), MU_RUN_TEST(ndarray_data_alloc_storage_set_ordering), MU_RUN_TEST(ndarray_read_at), - 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) );