[libcxx-commits] [libcxx] [libc++] Assert that iterator ranges are valid in string insert, append and assign (PR #234566)

[via libcxx-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: Ilyas Mallah (ilyas-mallah)

<details>
<summary>Changes</summary>

With forward iterators, `basic_string::insert(pos, first, last)`, `append(first, last)` and `assign(first, last)` cast `std::distance(first, last)` straight to `size_type`, so `first > last` becomes a huge count. `insert` then reads past the source range. With `s` a 10-character string, `s.insert(s.begin() + 2, s.begin() + 8, s.begin() + 3)` is a stack-buffer-overflow read under ASan on `main`, in every hardening mode. `erase(first, last)` already checks this.

This adds a `_LIBCPP_ASSERT_VALID_INPUT_RANGE` where the distance is already computed, so nothing new is computed. In `assign` only the branch for trivial iterators computes the distance, so that's the only place `assign` checks.

AI usage: Claude Code helped with the test runs and test scaffolding. I traced the issue, made the fix, and validated every change.

---
Full diff: https://github.com/llvm/llvm-project/pull/234566.diff


2 Files Affected:

- (modified) libcxx/include/string (+9-4) 
- (added) libcxx/test/libcxx/strings/basic.string/string.modifiers/assert.invalid_range.pass.cpp (+52) 


``````````diff
diff --git a/libcxx/include/string b/libcxx/include/string
index 28c32cc60fe714..724d41728d6dea 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1362,7 +1362,9 @@ public:
   append(_ForwardIterator __first, _ForwardIterator __last) {
     size_type __sz  = size();
     size_type __cap = capacity();
-    size_type __n   = static_cast<size_type>(std::distance(__first, __last));
+    auto __dist     = std::distance(__first, __last);
+    _LIBCPP_ASSERT_VALID_INPUT_RANGE(__dist >= 0, "string::append(first, last) called with invalid range");
+    size_type __n = static_cast<size_type>(__dist);
     if (__n == 0)
       return *this;
 
@@ -1488,7 +1490,9 @@ public:
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string&
   assign(_ForwardIterator __first, _ForwardIterator __last) {
     if (__string_is_trivial_iterator_v<_ForwardIterator>) {
-      size_type __n = static_cast<size_type>(std::distance(__first, __last));
+      auto __dist = std::distance(__first, __last);
+      _LIBCPP_ASSERT_VALID_INPUT_RANGE(__dist >= 0, "string::assign(first, last) called with invalid range");
+      size_type __n = static_cast<size_type>(__dist);
       __assign_trivial(__first, __last, __n);
     } else {
       __assign_with_sentinel(__first, __last);
@@ -1582,8 +1586,9 @@ public:
   template <class _ForwardIterator, __enable_if_t<__has_forward_iterator_category<_ForwardIterator>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 iterator
   insert(const_iterator __pos, _ForwardIterator __first, _ForwardIterator __last) {
-    auto __n = static_cast<size_type>(std::distance(__first, __last));
-    return __insert_with_size(__pos, __first, __last, __n);
+    auto __dist = std::distance(__first, __last);
+    _LIBCPP_ASSERT_VALID_INPUT_RANGE(__dist >= 0, "string::insert(pos, first, last) called with invalid range");
+    return __insert_with_size(__pos, __first, __last, static_cast<size_type>(__dist));
   }
 
 #  ifndef _LIBCPP_CXX03_LANG
diff --git a/libcxx/test/libcxx/strings/basic.string/string.modifiers/assert.invalid_range.pass.cpp b/libcxx/test/libcxx/strings/basic.string/string.modifiers/assert.invalid_range.pass.cpp
new file mode 100644
index 00000000000000..cb79ac3ac451d1
--- /dev/null
+++ b/libcxx/test/libcxx/strings/basic.string/string.modifiers/assert.invalid_range.pass.cpp
@@ -0,0 +1,52 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: can-test-hardening-assertions-fast
+// UNSUPPORTED: libcpp-assertion-semantic={{ignore|observe}}
+
+// <string>
+
+// Make sure insert, append and assign catch an iterator range with first > last.
+
+#include <string>
+
+#include "check_assertion.h"
+#include "min_allocator.h"
+#include "test_iterators.h"
+
+template <class S>
+void test() {
+  S s("0123456789");
+  const char a[] = "abcdef";
+
+  TEST_LIBCPP_ASSERT_FAILURE(
+      s.insert(s.cbegin(), a + 3, a), "string::insert(pos, first, last) called with invalid range");
+  TEST_LIBCPP_ASSERT_FAILURE(
+      s.insert(s.cbegin(), s.begin() + 8, s.begin() + 3), "string::insert(pos, first, last) called with invalid range");
+  TEST_LIBCPP_ASSERT_FAILURE(
+      s.insert(s.cbegin(), random_access_iterator<const char*>(a + 3), random_access_iterator<const char*>(a)),
+      "string::insert(pos, first, last) called with invalid range");
+
+  TEST_LIBCPP_ASSERT_FAILURE(s.append(a + 3, a), "string::append(first, last) called with invalid range");
+  TEST_LIBCPP_ASSERT_FAILURE(
+      s.append(s.begin() + 8, s.begin() + 3), "string::append(first, last) called with invalid range");
+  TEST_LIBCPP_ASSERT_FAILURE(
+      s.append(random_access_iterator<const char*>(a + 3), random_access_iterator<const char*>(a)),
+      "string::append(first, last) called with invalid range");
+
+  TEST_LIBCPP_ASSERT_FAILURE(s.assign(a + 3, a), "string::assign(first, last) called with invalid range");
+  TEST_LIBCPP_ASSERT_FAILURE(
+      s.assign(s.data() + 8, s.data() + 3), "string::assign(first, last) called with invalid range");
+}
+
+int main(int, char**) {
+  test<std::string>();
+  test<std::basic_string<char, std::char_traits<char>, min_allocator<char> > >();
+
+  return 0;
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/234566