[vector] Fix use-after-free when appending from own storage (#6393)

push (v[i]) reads its argument after alloc() may have moved the
vector's backing storage. Array extension has the same problem.
This is reachable through item_variations_t::duplicate_row() when
subsetting avar2 axes that share a variation row.

Construct a temporary before growth when a push argument aliases our
storage, and recover aliased source arrays by offset after allocation.
Keep the alias check on the push growth path, and reject already-failed
vectors before moving from their arguments.

Share the array extension implementation and route direct vector
extension through it, making v.extend (v) safe as well. General iterator
adapters into the same vector still cannot survive reallocation.

Add regressions for trivial and non-trivial copies, aliased moves,
constructor arguments, mutable and const arrays, direct self-extension,
and already-failed vectors. Reproduced push and both extension failures
under ASan before the fix. The updated tests pass under ASan+UBSan and
GCC with HB_TINY; injected allocation failures are clean under the
sanitizers. The full Meson suite passes (292 passed, 1 skipped).

Assisted-by: Claude
Assisted-by: Codex

Co-authored-by: Mar <mardgez13@gmail.com>

src/hb-vector.hh

diff --git a/src/hb-vector.hh b/src/hb-vector.hh
index 6ef75a6..036a1df 100644
--- a/src/hb-vector.hh
+++ b/src/hb-vector.hh
@@ -139,18 +139,23 @@
   }
   HB_ALWAYS_INLINE_VECTOR_ALLOCS
   void extend (array_t o, bool exact=false)
-  {
-    alloc (length + o.length, exact);
-    if (unlikely (in_error ())) return;
-    copy_array (o);
-  }
+  { extend (c_array_t (o), exact); }
   HB_ALWAYS_INLINE_VECTOR_ALLOCS
   void extend (c_array_t o, bool exact=false)
   {
+    /* o may point into our own storage, eg. v.extend (v.as_array ()).
+     * alloc() may move that storage, so find o again afterwards. */
+    bool aliased = o.length && in_storage (*o.arrayZ);
+    unsigned offset = aliased ? o.arrayZ - arrayZ : 0;
     alloc (length + o.length, exact);
     if (unlikely (in_error ())) return;
+    if (unlikely (aliased))
+      o = {arrayZ + offset, o.length};
     copy_array (o);
   }
+  HB_ALWAYS_INLINE_VECTOR_ALLOCS
+  void extend (const hb_vector_t &o, bool exact=false)
+  { extend (o.as_array (), exact); }
 
   public:
   int allocated = 0; /* < 0 means allocation failed. */
@@ -318,11 +323,26 @@
   HB_ALWAYS_INLINE_VECTOR_ALLOCS
   Type *push (Args&&... args)
   {
-    if (unlikely ((int) length >= allocated && !alloc (length + 1)))
-      // If push failed to allocate then don't copy v, since this may cause
-      // the created copy to leak memory since we won't have stored a
-      // reference to it.
-      return std::addressof (Crap (Type));
+    if (unlikely ((int) length >= allocated))
+    {
+      if (unlikely (in_error ()))
+	return std::addressof (Crap (Type));
+
+      /* An argument may live in our own storage, eg. v.push (v[i]).
+       * alloc() frees that storage, so copy it out first. */
+      if (unlikely (in_storage (args...)))
+      {
+	Type copy (std::forward<Args> (args)...);
+	if (unlikely (!alloc (length + 1)))
+	  return std::addressof (Crap (Type));
+	return push_has_room (std::move (copy));
+      }
+      if (unlikely (!alloc (length + 1)))
+	// If push failed to allocate then don't copy v, since this may cause
+	// the created copy to leak memory since we won't have stored a
+	// reference to it.
+	return std::addressof (Crap (Type));
+    }
 
     return push_has_room (std::forward<Args> (args)...);
   }
@@ -341,6 +361,16 @@
     return new (p) Type (std::forward<Args> (args)...);
   }
 
+  bool in_storage () const { return false; }
+  template <typename T, typename... Ts>
+  bool in_storage (const T &v, const Ts &...vs) const
+  {
+    uintptr_t p = (uintptr_t) std::addressof (v);
+    return (arrayZ &&
+	    (uintptr_t) arrayZ <= p && p < (uintptr_t) (arrayZ + length)) ||
+	   in_storage (vs...);
+  }
+
   bool is_owned () const
   {
     return allocated != 0 && allocated != -1;

src/test-vector.cc

diff --git a/src/test-vector.cc b/src/test-vector.cc
index 030e742..7c14c53 100644
--- a/src/test-vector.cc
+++ b/src/test-vector.cc
@@ -81,6 +81,80 @@
     hb_always_assert (v[1] == "two");
   }
 
+  /* Test pushing an element of the vector itself while it grows. */
+  {
+    hb_vector_t<int> v {7};
+    for (unsigned i = 0; i < 1000; i++)
+      v.push (v.arrayZ[0]);
+    hb_always_assert (v.length == 1001);
+    for (unsigned i = 0; i < v.length; i++)
+      hb_always_assert (v[i] == 7);
+  }
+  {
+    hb_vector_t<std::string> v;
+    v.push (std::string ("a string longer than the small-string buffer"));
+    for (unsigned i = 0; i < 1000; i++)
+      v.push (v.arrayZ[i]);
+    hb_always_assert (v.length == 1001);
+    for (unsigned i = 0; i < v.length; i++)
+      hb_always_assert (v[i] == "a string longer than the small-string buffer");
+  }
+  {
+    hb_vector_t<hb_vector_t<int>> v;
+    v.push (hb_vector_t<int> {1, 2, 3});
+    for (unsigned i = 0; i < 1000; i++)
+      v.push (v.arrayZ[0]);
+    hb_always_assert (v.length == 1001);
+    for (unsigned i = 0; i < v.length; i++)
+      hb_always_assert (v[i].length == 3 && v[i][2] == 3);
+  }
+  {
+    hb_vector_t<hb_vector_t<int>> v {hb_vector_t<int> {1, 2, 3}};
+    v.push (std::move (v[0]));
+    hb_always_assert (v.length == 2 && v[0].length == 0);
+    hb_always_assert (v[1].length == 3 && v[1][2] == 3);
+  }
+  {
+    hb_vector_t<std::string> v {"one"};
+    v.push (3, v[0][0]);
+    hb_always_assert (v.length == 2);
+    hb_always_assert (v[0] == "one" && v[1] == "ooo");
+  }
+  {
+    hb_vector_t<int> v {1, 2, 3};
+    v.extend (v.as_array ());
+    hb_always_assert (v.length == 6);
+    hb_always_assert (v[3] == 1 && v[4] == 2 && v[5] == 3);
+  }
+  {
+    hb_vector_t<int> v {1, 2, 3};
+    v.extend (hb_array_t<const int> (v.arrayZ + 1, 2));
+    hb_always_assert (v.length == 5);
+    hb_always_assert (v[3] == 2 && v[4] == 3);
+  }
+  {
+    hb_vector_t<std::string> v {"one", "two", "three"};
+    v.extend (v.as_array ());
+    hb_always_assert (v.length == 6);
+    for (unsigned i = 0; i < 3; i++)
+      hb_always_assert (v[i] == v[i + 3]);
+  }
+  {
+    hb_vector_t<std::string> v {"one", "two", "three"};
+    v.extend (v);
+    hb_always_assert (v.length == 6);
+    for (unsigned i = 0; i < 3; i++)
+      hb_always_assert (v[i] == v[i + 3]);
+  }
+
+  /* A failed vector must not move from an aliased argument. */
+  {
+    hb_vector_t<std::string> v {"one"};
+    v.set_error ();
+    hb_always_assert (!v.push_or_fail (std::move (v[0])));
+    hb_always_assert (v.length == 1 && v[0] == "one");
+  }
+
   /* Test move constructor. */
   {
     hb_vector_t<int> s {1, 2};