From 2247737db20555d10742831c20ecc27b8571b28d Mon Sep 17 00:00:00 2001 From: willvale <66674079+willvale@users.noreply.github.com> Date: Mon, 29 Jun 2026 23:06:30 +1200 Subject: [PATCH 1/3] Stop managed_array from accidentally changing C++ types Storing non-POD types in char buffers is unsafe. * Ensure alignments are compatible with desired type. * Ensure raw data is copied with memcpy Otherwise, the list entries are created raw and placement-constructed as one type, which is changed when they're copied by the copy constructor of T (different type). For POD data this wouldn't matter, but here it changes the vtable pointer and indirectly causes an OOB write. --- inkcpp/array.h | 21 ++++++++++++++++----- inkcpp/list_table.cpp | 1 + 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/inkcpp/array.h b/inkcpp/array.h index 5d02dac2..845741ed 100644 --- a/inkcpp/array.h +++ b/inkcpp/array.h @@ -35,6 +35,7 @@ class managed_array : public snapshot_interface if constexpr (dynamic) { if constexpr (simple) { _dynamic_data = reinterpret_cast(new char[sizeof(T) * initialCapacity]); + inkAssert((::size_t)_dynamic_data % alignof(T) == 0); } else { _dynamic_data = new T[initialCapacity]; } @@ -113,6 +114,7 @@ class managed_array : public snapshot_interface inkAssert(_size <= _capacity, "Try to append to a full array!"); // TODO(JBenda): Silent fail? } + inkAssert(_size < _capacity); return data()[_size++]; } @@ -197,8 +199,8 @@ class managed_array : public snapshot_interface private: T* _dynamic_data = nullptr; - size_t _capacity; - size_t _size; + size_t _capacity = 0; + size_t _size = 0; if_t _static_data[dynamic ? 1 : initialCapacity]; }; @@ -273,13 +275,22 @@ void managed_array::extend(size_t capacity) } T* new_data = nullptr; if constexpr (simple) { + // Warning: Allocating typed data in a char* container is potentially unsafe. We need to be sure the + // alignment is compatible with the destination type... new_data = reinterpret_cast(new char[sizeof(T) * new_capacity]); + inkAssert((::size_t)new_data % alignof(T) == 0); + + // ...and we have to copy the contents byte-by-byte, since client code (_list_handouts) type-puns + // between two classes with different vtbls here. Copying these elementwise would change the stored + // C++ type. + memcpy(new_data, _dynamic_data, sizeof(T) * _capacity); } else { + // Allocate and copy typed data normally new_data = new T[new_capacity]; - } - for (size_t i = 0; i < _capacity; ++i) { - new_data[i] = _dynamic_data[i]; + for (size_t i = 0; i < _capacity; ++i) { + new_data[i] = _dynamic_data[i]; + } } if constexpr (simple) { diff --git a/inkcpp/list_table.cpp b/inkcpp/list_table.cpp index d4236983..b4c2d143 100644 --- a/inkcpp/list_table.cpp +++ b/inkcpp/list_table.cpp @@ -825,6 +825,7 @@ list_table::list list_table::redefine(list lh, list rh) list_interface* list_table::handout_list(list l) { static_assert(sizeof(list_interface) == sizeof(list_impl)); + static_assert(alignof(list_interface) == alignof(list_impl)); auto& res = _list_handouts.push(); new (&res) list_impl(*this, l.lid); return &res; From 862ff99f754aea18098c679e73378961767de78a Mon Sep 17 00:00:00 2001 From: willvale <66674079+willvale@users.noreply.github.com> Date: Mon, 29 Jun 2026 23:23:02 +1200 Subject: [PATCH 2/3] Fix clangformat issues --- inkcpp/array.h | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/inkcpp/array.h b/inkcpp/array.h index 845741ed..1449c423 100644 --- a/inkcpp/array.h +++ b/inkcpp/array.h @@ -35,7 +35,7 @@ class managed_array : public snapshot_interface if constexpr (dynamic) { if constexpr (simple) { _dynamic_data = reinterpret_cast(new char[sizeof(T) * initialCapacity]); - inkAssert((::size_t)_dynamic_data % alignof(T) == 0); + inkAssert(( ::size_t ) _dynamic_data % alignof(T) == 0); } else { _dynamic_data = new T[initialCapacity]; } @@ -43,8 +43,8 @@ class managed_array : public snapshot_interface } config::statistics::container statistics() const - { - return {static_cast(_capacity), static_cast(_size)}; + { + return {static_cast(_capacity), static_cast(_size)}; } virtual ~managed_array() @@ -199,8 +199,8 @@ class managed_array : public snapshot_interface private: T* _dynamic_data = nullptr; - size_t _capacity = 0; - size_t _size = 0; + size_t _capacity = 0; + size_t _size = 0; if_t _static_data[dynamic ? 1 : initialCapacity]; }; @@ -275,14 +275,14 @@ void managed_array::extend(size_t capacity) } T* new_data = nullptr; if constexpr (simple) { - // Warning: Allocating typed data in a char* container is potentially unsafe. We need to be sure the - // alignment is compatible with the destination type... + // Warning: Allocating typed data in a char* container is potentially unsafe. We need to be sure + // the alignment is compatible with the destination type... new_data = reinterpret_cast(new char[sizeof(T) * new_capacity]); - inkAssert((::size_t)new_data % alignof(T) == 0); + inkAssert(( ::size_t ) new_data % alignof(T) == 0); - // ...and we have to copy the contents byte-by-byte, since client code (_list_handouts) type-puns - // between two classes with different vtbls here. Copying these elementwise would change the stored - // C++ type. + // ...and we have to copy the contents byte-by-byte, since client code (_list_handouts) + // type-puns between two classes with different vtbls here. Copying these elementwise would + // change the stored C++ type. memcpy(new_data, _dynamic_data, sizeof(T) * _capacity); } else { // Allocate and copy typed data normally @@ -376,8 +376,8 @@ class basic_restorable_array : public snapshot_interface private: inline void check_index(size_t index) const - { - inkAssert(index < capacity(), "Index out of range!"); + { + inkAssert(index < capacity(), "Index out of range!"); } void clear_temp(); @@ -501,8 +501,8 @@ class fixed_restorable_array : public basic_restorable_array public: fixed_restorable_array(const T& initial, const T& nullValue) : basic_restorable_array(_buffer, SIZE * 2, nullValue) - { - basic_restorable_array::clear(initial); + { + basic_restorable_array::clear(initial); } const unsigned char* @@ -575,8 +575,8 @@ class allocated_restorable_array : public basic_restorable_array template inline bool basic_restorable_array::can_be_migrated() const -{ - return ! _saved; +{ + return ! _saved; } template @@ -596,7 +596,7 @@ inline size_t basic_restorable_array::snap(unsigned char* data, const snapper template inline const unsigned char* basic_restorable_array::impl_snap_load_meta(const unsigned char* data -) + ) { auto ptr = data; ptr = snap_read(ptr, _saved); @@ -625,7 +625,7 @@ inline const unsigned char* template inline const unsigned char* fixed_restorable_array< - T, SIZE>::snap_load(const unsigned char* data, const snapshot_interface::loader&) + T, SIZE>::snap_load(const unsigned char* data, const snapshot_interface::loader&) { auto ptr = data; ptr = base::impl_snap_load_meta(ptr); @@ -635,7 +635,7 @@ inline const unsigned char* fixed_restorable_array< template inline const unsigned char* allocated_restorable_array< - T>::snap_load(const unsigned char* data, const snapshot_interface::loader&) + T>::snap_load(const unsigned char* data, const snapshot_interface::loader&) { auto ptr = data; ptr = base::impl_snap_load_meta(ptr); From 3791229f0925784d8294f9f14df2c8c40cfcd66a Mon Sep 17 00:00:00 2001 From: willvale <66674079+willvale@users.noreply.github.com> Date: Mon, 29 Jun 2026 23:31:36 +1200 Subject: [PATCH 3/3] Restore whitespace. Hnng. --- inkcpp/array.h | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/inkcpp/array.h b/inkcpp/array.h index 1449c423..c93db1f4 100644 --- a/inkcpp/array.h +++ b/inkcpp/array.h @@ -43,8 +43,8 @@ class managed_array : public snapshot_interface } config::statistics::container statistics() const - { - return {static_cast(_capacity), static_cast(_size)}; + { + return {static_cast(_capacity), static_cast(_size)}; } virtual ~managed_array() @@ -376,8 +376,8 @@ class basic_restorable_array : public snapshot_interface private: inline void check_index(size_t index) const - { - inkAssert(index < capacity(), "Index out of range!"); + { + inkAssert(index < capacity(), "Index out of range!"); } void clear_temp(); @@ -501,8 +501,8 @@ class fixed_restorable_array : public basic_restorable_array public: fixed_restorable_array(const T& initial, const T& nullValue) : basic_restorable_array(_buffer, SIZE * 2, nullValue) - { - basic_restorable_array::clear(initial); + { + basic_restorable_array::clear(initial); } const unsigned char* @@ -575,8 +575,8 @@ class allocated_restorable_array : public basic_restorable_array template inline bool basic_restorable_array::can_be_migrated() const -{ - return ! _saved; +{ + return ! _saved; } template @@ -596,7 +596,7 @@ inline size_t basic_restorable_array::snap(unsigned char* data, const snapper template inline const unsigned char* basic_restorable_array::impl_snap_load_meta(const unsigned char* data - ) +) { auto ptr = data; ptr = snap_read(ptr, _saved); @@ -625,7 +625,7 @@ inline const unsigned char* template inline const unsigned char* fixed_restorable_array< - T, SIZE>::snap_load(const unsigned char* data, const snapshot_interface::loader&) + T, SIZE>::snap_load(const unsigned char* data, const snapshot_interface::loader&) { auto ptr = data; ptr = base::impl_snap_load_meta(ptr); @@ -635,7 +635,7 @@ inline const unsigned char* fixed_restorable_array< template inline const unsigned char* allocated_restorable_array< - T>::snap_load(const unsigned char* data, const snapshot_interface::loader&) + T>::snap_load(const unsigned char* data, const snapshot_interface::loader&) { auto ptr = data; ptr = base::impl_snap_load_meta(ptr);