[libcxx-commits] [libcxx] [libc++] Fix use-after-free during self-assignment (PR #225296)

[Chris Kennelly via libcxx-commits]

https://github.com/ckennelly updated https://github.com/llvm/llvm-project/pull/225296

>From 31780e392234062e32eb407ff15d6a571bdc5902 Mon Sep 17 00:00:00 2001
From: Chris Kennelly <ckennelly at google.com>
Date: Mon, 21 Sep 2026 05:17:52 +0000
Subject: [PATCH] [libc++] Fix use-after-free during self-assignment

The range [data(), data() + size() + 1) points into the string's own buffer and
is one element longer than the capacity when the buffer is full, so assigning it
reallocates:

  std::string str;
  str.resize(str.capacity(), 'a');                      // size() == capacity()
  str.assign(str.data(), str.data() + str.size() + 1);  // reallocates

__assign_trivial called __grow_by_without_replace up front and read the range
afterwards, by which point the buffer holding it had been released: that is a
heap-use-after-free for a long string, and for a short string the long
representation has been written over the small buffer the range points into, so
the assignment silently stores garbage.

Allocate the new buffer and read the range into it before releasing the buffer
the string holds. That also keeps the range valid in the exotic case where it is
the byte representation of the string object itself, which the comment this
replaces called out. The capacity the assignment grows to is unchanged.

This regressed in baf6f91851ed ("[libc++][ranges] Implement the changes to
`basic_string` from P1206 (`ranges::to`)"), which moved assign(_ForwardIterator,
_ForwardIterator) into __assign_trivial and dropped the __addr_in_range check
that had routed self-referencing ranges through a temporary since e87479b00fcc;
the bug is present in every release since LLVM 17.

Constant evaluation diagnoses the read of the released buffer, so the
static_assert covering the new case rejects the unpatched header outright in
C++20 and later.

Assisted-by: Antigravity
---
 libcxx/include/string                         | 25 +++++++++++--------
 .../string_assign/iterator.pass.cpp           | 23 ++++++++++++++---
 2 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/libcxx/include/string b/libcxx/include/string
index 1f52bc02e0ea0..7d916e3330ee9 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -2917,16 +2917,21 @@ basic_string<_CharT, _Traits, _Allocator>::__assign_trivial(_Iterator __first, _
   size_type __old_size = size();
   size_type __cap      = capacity();
   if (__cap < __n) {
-    // Unlike `append` functions, if the input range points into the string itself, there is no case that the input
-    // range could get invalidated by reallocation:
-    // 1. If the input range is a subset of the string itself, its size cannot exceed the capacity of the string,
-    //    thus no reallocation would happen.
-    // 2. In the exotic case where the input range is the byte representation of the string itself, the string
-    //    object itself stays valid even if reallocation happens.
-    size_type __sz = size();
-    __grow_by_without_replace(__cap, __n - __cap, __sz, 0, __sz);
-    __annotate_increase(__n);
-  } else if (__n > __old_size)
+    // The input range may point into the string itself, so it is read into the new buffer before the buffer the
+    // string holds is released. That covers both a subset of the value, which is longer than the capacity when it
+    // includes the null terminator of a full buffer, and the exotic case where the range is the byte representation
+    // of the string object itself, which the long representation is written over.
+    __long __buffer = __allocate_long_buffer(__alloc_, __n, __get_amortized_growth_capacity(__n));
+    auto __end      = __copy_non_overlapping_range(__first, __last, std::__to_address(__buffer.__data_));
+    traits_type::assign(*__end, value_type());
+
+    __annotate_delete();
+    auto __guard = std::__make_scope_guard(__annotate_new_size(*this));
+    __reset_internal_buffer(__buffer);
+    return;
+  }
+
+  if (__n > __old_size)
     __annotate_increase(__n - __old_size);
   pointer __p = __get_pointer();
   for (; __first != __last; ++__p, (void)++__first)
diff --git a/libcxx/test/std/strings/basic.string/string.modifiers/string_assign/iterator.pass.cpp b/libcxx/test/std/strings/basic.string/string.modifiers/string_assign/iterator.pass.cpp
index b2306807fcf7b..768896be58bd0 100644
--- a/libcxx/test/std/strings/basic.string/string.modifiers/string_assign/iterator.pass.cpp
+++ b/libcxx/test/std/strings/basic.string/string.modifiers/string_assign/iterator.pass.cpp
@@ -28,6 +28,23 @@ TEST_CONSTEXPR_CXX20 void test(S s, It first, It last, S expected) {
   LIBCPP_ASSERT(is_string_asan_correct(s));
 }
 
+// [data(), data() + size() + 1) is a valid range that points into the string's own buffer -- data()[size()] is the
+// null terminator -- and it is one element longer than the capacity when the buffer is full, so assigning it
+// reallocates while the range still points into the buffer being replaced.
+template <class S>
+TEST_CONSTEXPR_CXX20 void test_self_referencing_range(typename S::size_type minimum_capacity) {
+  S s(minimum_capacity, 'a');
+  s.resize(s.capacity(), 'a');
+  assert(s.size() == s.capacity());
+  S expected = s + S(1, '\0');
+
+  // Note that the assignment must happen in place: copying the string first would make the range point elsewhere.
+  s.assign(s.data(), s.data() + s.size() + 1);
+  LIBCPP_ASSERT(s.__invariants());
+  assert(s == expected);
+  LIBCPP_ASSERT(is_string_asan_correct(s));
+}
+
 #ifndef TEST_HAS_NO_EXCEPTIONS
 struct Widget {
   operator char() const { throw 42; }
@@ -163,10 +180,8 @@ TEST_CONSTEXPR_CXX20 void test_string() {
   }
 
   { // regression-test assigning to self in sneaky ways
-    S sneaky = "hello";
-    sneaky.resize(sneaky.capacity(), 'x');
-    S expected = sneaky + S(1, '\0');
-    test(sneaky, sneaky.data(), sneaky.data() + sneaky.size() + 1, expected);
+    test_self_referencing_range<S>(0);  // the small buffer, which the long representation is written over
+    test_self_referencing_range<S>(64); // a heap buffer, which is released by the assignment
   }
 }