From c5675d248be333f9550a5ac7feaf2c175868929b Mon Sep 17 00:00:00 2001 From: "Brian T.N. Gunney" Date: Wed, 18 Jan 2023 13:51:47 -0800 Subject: [PATCH 1/4] Reduce size of ArrayView returned by subspan. The size was allowing access of potentially invalid memory. --- RELEASE-NOTES.md | 1 + src/axom/core/ArrayView.hpp | 10 +++++++--- src/axom/core/tests/core_array.hpp | 14 ++++++++++++++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index cbfff7d180..25861bbfcc 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -42,6 +42,7 @@ The Axom project release numbers follow [Semantic Versioning](http://semver.org/ - Updates uberenv submodule to HEAD of main on 28Dec2022 - Updates blt submodule to HEAD of develop on 28Dec2022 - Adds `vcpkg` ports for `RAJA`, `Umpire` with optional `OpenMP` feature for automated Windows build +- Reduce size of `ArrayView::subspan` to prevent accessing invalid memory. ## [Version 0.7.0] - Release date 2022-08-30 diff --git a/src/axom/core/ArrayView.hpp b/src/axom/core/ArrayView.hpp index c91e991ae4..c142c18e1a 100644 --- a/src/axom/core/ArrayView.hpp +++ b/src/axom/core/ArrayView.hpp @@ -135,8 +135,8 @@ class ArrayView : public ArrayBase> * \param [in] count The number of elements to include in the subspan, or -1 * to take all elements after offset (default). * - * \return A subspan ArrayView that spans the indices [offsets, offsets + count), - * or [offsets, num_elements) if count == -1. + * \return A subspan ArrayView that spans the indices [offset, offset + count), + * or [offset, num_elements) if count == -1. * * \pre offset + count <= m_num_elements if count != -1 */ @@ -146,7 +146,11 @@ class ArrayView : public ArrayBase> assert(offset + count <= m_num_elements); ArrayView slice = *this; slice.m_data += offset; - if(count != -1) + if(count == -1) + { + slice.m_num_elements -= offset; + } + else { slice.m_num_elements = count; } diff --git a/src/axom/core/tests/core_array.hpp b/src/axom/core/tests/core_array.hpp index d492dd258b..a990910cb3 100644 --- a/src/axom/core/tests/core_array.hpp +++ b/src/axom/core/tests/core_array.hpp @@ -1931,4 +1931,18 @@ TEST(core_array, checkVariadicCtors) Array arr12(s, s, s); } +//------------------------------------------------------------------------------ + +TEST(core_array, check_subspan_range) +{ + int m = 10; + int n = 3; + Array arr(m); + ArrayView arrv1(arr); + ArrayView arrv2 = arrv1.subspan(n); + EXPECT_EQ(arrv2.size() + n, arrv1.size()); + EXPECT_GE(&arrv2[0], &arr[0]); + EXPECT_LE(&arrv2[arrv2.size()-1], &arr[arr.size()-1]); +} + } /* end namespace axom */ From 8494769a58c44be141841cc4e72ba0904c821853 Mon Sep 17 00:00:00 2001 From: "Brian T.N. Gunney" Date: Wed, 18 Jan 2023 14:12:42 -0800 Subject: [PATCH 2/4] Run auto-formatter. --- src/axom/core/tests/core_array.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/axom/core/tests/core_array.hpp b/src/axom/core/tests/core_array.hpp index a990910cb3..aa5da00e85 100644 --- a/src/axom/core/tests/core_array.hpp +++ b/src/axom/core/tests/core_array.hpp @@ -1942,7 +1942,7 @@ TEST(core_array, check_subspan_range) ArrayView arrv2 = arrv1.subspan(n); EXPECT_EQ(arrv2.size() + n, arrv1.size()); EXPECT_GE(&arrv2[0], &arr[0]); - EXPECT_LE(&arrv2[arrv2.size()-1], &arr[arr.size()-1]); + EXPECT_LE(&arrv2[arrv2.size() - 1], &arr[arr.size() - 1]); } } /* end namespace axom */ From ae1eeb08531ab4d4cf8cd89e20a3aa1f22e2d4d3 Mon Sep 17 00:00:00 2001 From: "Brian T.N. Gunney" Date: Wed, 18 Jan 2023 15:15:27 -0800 Subject: [PATCH 3/4] More precise checks on range of subspan object. --- src/axom/core/tests/core_array.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/axom/core/tests/core_array.hpp b/src/axom/core/tests/core_array.hpp index aa5da00e85..3c0d9c96c2 100644 --- a/src/axom/core/tests/core_array.hpp +++ b/src/axom/core/tests/core_array.hpp @@ -1941,8 +1941,8 @@ TEST(core_array, check_subspan_range) ArrayView arrv1(arr); ArrayView arrv2 = arrv1.subspan(n); EXPECT_EQ(arrv2.size() + n, arrv1.size()); - EXPECT_GE(&arrv2[0], &arr[0]); - EXPECT_LE(&arrv2[arrv2.size() - 1], &arr[arr.size() - 1]); + EXPECT_EQ(&arrv2[0], &arr[n]); + EXPECT_EQ(&arrv2[arrv2.size() - 1], &arr[arr.size() - 1]); } } /* end namespace axom */ From 846d9e814f409cbb7064ed379970a3daed65da1f Mon Sep 17 00:00:00 2001 From: "Brian T.N. Gunney" Date: Wed, 18 Jan 2023 22:32:50 -0800 Subject: [PATCH 4/4] Add a few more checks on the subspan range. --- src/axom/core/ArrayView.hpp | 15 ++++++++++----- src/axom/core/tests/core_array.hpp | 9 +++++++++ 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/axom/core/ArrayView.hpp b/src/axom/core/ArrayView.hpp index c142c18e1a..28d8bf3dbf 100644 --- a/src/axom/core/ArrayView.hpp +++ b/src/axom/core/ArrayView.hpp @@ -135,18 +135,23 @@ class ArrayView : public ArrayBase> * \param [in] count The number of elements to include in the subspan, or -1 * to take all elements after offset (default). * - * \return A subspan ArrayView that spans the indices [offset, offset + count), - * or [offset, num_elements) if count == -1. + * \return An ArrayView that spans the indices [offset, offset + count), + * or [offset, num_elements) if count < 0. * - * \pre offset + count <= m_num_elements if count != -1 + * \pre offset + count <= m_num_elements if count < 0 */ template ::type> AXOM_HOST_DEVICE ArrayView subspan(IndexType offset, IndexType count = -1) const { - assert(offset + count <= m_num_elements); + assert(offset >= 0 && offset < m_num_elements); + if(count >= 0) + { + assert(offset + count <= m_num_elements); + } + ArrayView slice = *this; slice.m_data += offset; - if(count == -1) + if(count < 0) { slice.m_num_elements -= offset; } diff --git a/src/axom/core/tests/core_array.hpp b/src/axom/core/tests/core_array.hpp index 3c0d9c96c2..0f4373f07e 100644 --- a/src/axom/core/tests/core_array.hpp +++ b/src/axom/core/tests/core_array.hpp @@ -1937,12 +1937,21 @@ TEST(core_array, check_subspan_range) { int m = 10; int n = 3; + int l = 5; Array arr(m); + ArrayView arrv1(arr); + EXPECT_EQ(arrv1.size(), arr.size()); + ArrayView arrv2 = arrv1.subspan(n); EXPECT_EQ(arrv2.size() + n, arrv1.size()); EXPECT_EQ(&arrv2[0], &arr[n]); EXPECT_EQ(&arrv2[arrv2.size() - 1], &arr[arr.size() - 1]); + + ArrayView arrv3 = arrv1.subspan(n, l); + EXPECT_EQ(arrv3.size(), l); + EXPECT_EQ(&arrv3[0], &arr[n]); + EXPECT_EQ(&arrv3[arrv3.size() - 1], &arr[n + l - 1]); } } /* end namespace axom */