fix: heap buffer overflow on ndarray shapes whose size overflows

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.

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);
 

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;
     }
 

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 \

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)
 );