Fix svector use-after-free when the argument aliases an element

include/xtensor/containers/xstorage.hpp

@@ -897,12 +897,15 @@ namespace xt
     template <class T, std::size_t N, class A, bool Init>
     inline void svector<T, N, A, Init>::assign(size_type n, const value_type& v)
     {
+        // v may refer to an element of this vector, and grow() may free the
+        // buffer that holds it.
+        const value_type value(v);
         if (n > N && n > capacity())
         {
             grow(n);
         }
         m_end = m_begin + n;
-        std::fill(begin(), end(), v);
+        std::fill(begin(), end(), value);
     }
 
     template <class T, std::size_t N, class A, bool Init>
@@ -1022,19 +1025,32 @@ namespace xt
     {
         if (m_end >= m_capacity)
         {
+            // elt may refer to an element of this vector, and grow() may free
+            // the buffer that holds it.
+            T value(elt);
             grow();
+            *(m_end++) = std::move(value);
+        }
+        else
+        {
+            *(m_end++) = elt;
         }
-        *(m_end++) = elt;
     }
 
     template <class T, std::size_t N, class A, bool Init>
     void svector<T, N, A, Init>::push_back(T&& elt)
     {
         if (m_end >= m_capacity)
         {
+            // Same as above: take the value out before grow() runs.
+            T value(std::move(elt));
             grow();
+            *(m_end++) = std::move(value);
+        }
+        else
+        {
+            *(m_end++) = std::move(elt);
         }
-        *(m_end++) = std::move(elt);
     }
 
     template <class T, std::size_t N, class A, bool Init>
@@ -1196,31 +1212,20 @@ namespace xt
     template <class T, std::size_t N, class A, bool Init>
     inline auto svector<T, N, A, Init>::insert(const_iterator cit, const T& elt) -> iterator
     {
+        // elt may refer to an element of this vector: the shift below moves it
+        // and grow() may free the buffer that holds it.
+        T value(elt);
         auto it = const_cast<pointer>(cit);
-        if (it == m_end)
-        {
-            push_back(elt);
-            return m_end - 1;
-        }
-
         if (m_end >= m_capacity)
         {
             std::ptrdiff_t elt_no = it - m_begin;
             grow();
             it = m_begin + elt_no;
         }
 
-        (*m_end) = back();
-        std::move_backward(it, m_end - 1, m_end);
+        std::move_backward(it, m_end, m_end + 1);
         ++m_end;
-
-        // Update ref if element moved
-        const T* elt_ptr = &elt;
-        bool cond = it <= elt_ptr && elt_ptr < m_end;
-        // More complicated than incrementing elt_ptr, but this avoids
-        // false positive array-bounds warning on GCC 10
-        const T* src_ptr = cond ? it + (elt_ptr - it) + std::ptrdiff_t(1) : elt_ptr;
-        *it = *src_ptr;
+        *it = std::move(value);
         return it;
     }
 

test/test_xstorage.cpp

@@ -145,6 +145,69 @@ namespace xt
         EXPECT_TRUE(std::equal(s.begin(), s.end(), v.begin()));
     }
 
+    // A full heap buffer: the next insertion reallocates and frees the old
+    // buffer, the one that holds the aliased argument in the tests below.
+    using small_svector_type = svector<std::size_t, 1>;
+
+    TEST(svector, insert_aliased_element)
+    {
+        {
+            small_svector_type s = {1, 2, 3};
+            ASSERT_FALSE(s.on_stack());
+            ASSERT_EQ(s.size(), s.capacity());
+            s.insert(s.begin(), s[0]);
+            EXPECT_EQ(s, small_svector_type({1, 1, 2, 3}));
+        }
+
+        {
+            small_svector_type s = {1, 2, 3};
+            ASSERT_EQ(s.size(), s.capacity());
+            s.insert(s.begin() + 1, s[1]);
+            EXPECT_EQ(s, small_svector_type({1, 2, 2, 3}));
+        }
+
+        {
+            small_svector_type s = {1, 2, 3};
+            ASSERT_EQ(s.size(), s.capacity());
+            s.insert(s.end(), s[0]);
+            EXPECT_EQ(s, small_svector_type({1, 2, 3, 1}));
+        }
+
+        {
+            // No reallocation: the shift moves the aliased element.
+            small_svector_type s = {1, 2, 3};
+            s.reserve(8);
+            s.insert(s.begin(), s[1]);
+            EXPECT_EQ(s, small_svector_type({2, 1, 2, 3}));
+        }
+    }
+
+    TEST(svector, push_back_aliased_element)
+    {
+        {
+            small_svector_type s = {1, 2, 3};
+            ASSERT_FALSE(s.on_stack());
+            ASSERT_EQ(s.size(), s.capacity());
+            s.push_back(s[0]);
+            EXPECT_EQ(s, small_svector_type({1, 2, 3, 1}));
+        }
+
+        {
+            small_svector_type s = {1, 2, 3};
+            ASSERT_EQ(s.size(), s.capacity());
+            s.push_back(std::move(s[1]));
+            EXPECT_EQ(s, small_svector_type({1, 2, 3, 2}));
+        }
+    }
+
+    TEST(svector, assign_aliased_element)
+    {
+        small_svector_type s = {7, 2, 3};
+        ASSERT_FALSE(s.on_stack());
+        s.assign(5, s[0]);
+        EXPECT_EQ(s, small_svector_type({7, 7, 7, 7, 7}));
+    }
+
     TEST(svector, insert_range)
     {
         svector_type s0 = {1, 2, 3, 4};