[libcxx-commits] [libcxx] [libc++] Fix search_n with an invalid count (PR #223747)

[via libcxx-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

<details>
<summary>Changes</summary>

search_n would incorrectly compute pointer offsets based on the user-provided count, which wasn't checked for being negative. This patch adds checks for negativity to prevent computing and accessing potentially out-of-bounds pointers.

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


3 Files Affected:

- (modified) libcxx/include/__algorithm/search_n.h (+11-5) 
- (modified) libcxx/test/std/algorithms/alg.nonmodifying/alg.search/ranges.search_n.pass.cpp (+16) 
- (modified) libcxx/test/std/algorithms/alg.nonmodifying/alg.search/search_n.pass.cpp (+12) 


``````````diff
diff --git a/libcxx/include/__algorithm/search_n.h b/libcxx/include/__algorithm/search_n.h
index 0962542e134cd..b978e12ee2dd0 100644
--- a/libcxx/include/__algorithm/search_n.h
+++ b/libcxx/include/__algorithm/search_n.h
@@ -18,6 +18,7 @@
 #include <__type_traits/enable_if.h>
 #include <__type_traits/invoke.h>
 #include <__type_traits/is_callable.h>
+#include <__type_traits/make_unsigned.h>
 #include <__utility/convert_to_integral.h>
 #include <__utility/pair.h>
 
@@ -79,14 +80,19 @@ __find_longest_suffix(_RAIter __first, _RAIter __last, const _ValueT& __value, _
 template <class _AlgPolicy, class _Pred, class _Iter, class _SizeT, class _Type, class _Proj, class _DiffT>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 std::pair<_Iter, _Iter> __search_n_random_access_impl(
     _Iter __first, _SizeT __count_in, const _Type& __value, _Pred& __pred, _Proj& __proj, _DiffT __size) {
-  auto __last  = __first + __size;
-  auto __count = static_cast<_DiffT>(__count_in);
-
-  if (__count == 0)
+  // Trivially match if count is non-positive. This must be checked before we perform arithmetic on the count.
+  if (__count_in <= 0)
     return std::make_pair(__first, __first);
-  if (__size < __count)
+
+  auto __last = __first + __size;
+
+  // If count is larger than the sequence size, no match is possible. Compare as unsigned to prevent
+  // truncation between _DiffT and _SizeT.
+  if (std::__to_unsigned_like(__size) < std::__to_unsigned_like(__count_in))
     return std::make_pair(__last, __last);
 
+  auto __count = static_cast<_DiffT>(__count_in); // representable as _DiffT since `0 < __count_in <= __size`
+
   // [__match_start, __match_start + __count) is the subrange which we currently check whether it only contains matching
   // elements. This subrange is returned in case all the elements match.
   // [__match_start, __matched_until) is the longest subrange where all elements are known to match at any given point
diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.search/ranges.search_n.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.search/ranges.search_n.pass.cpp
index 6686e8de733bd..84a92fbd60f6c 100644
--- a/libcxx/test/std/algorithms/alg.nonmodifying/alg.search/ranges.search_n.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.search/ranges.search_n.pass.cpp
@@ -170,6 +170,22 @@ constexpr void test_iterators() {
     }
   }
 
+  { // pattern has negative length. See https://wg21.link/LWG426.
+    {
+      int a[]  = {6, 7, 8};
+      auto ret = std::ranges::search_n(Iter(a), Sent(Iter(a + 3)), -1, 7);
+      assert(base(ret.begin()) == a);
+      assert(base(ret.end()) == a);
+    }
+    {
+      int a[]    = {6, 7, 8};
+      auto range = std::ranges::subrange(Iter(a), Sent(Iter(a + 3)));
+      auto ret   = std::ranges::search_n(range, -1, 7);
+      assert(base(ret.begin()) == a);
+      assert(base(ret.end()) == a);
+    }
+  }
+
   { // range has zero length
     {
       std::array<int, 0> a = {};
diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.search/search_n.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.search/search_n.pass.cpp
index bbe9f0ec51b3d..68678d73bb340 100644
--- a/libcxx/test/std/algorithms/alg.nonmodifying/alg.search/search_n.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.search/search_n.pass.cpp
@@ -16,6 +16,8 @@
 #include <algorithm>
 #include <array>
 #include <cassert>
+#include <cstddef>
+#include <limits>
 
 #include "test_macros.h"
 #include "test_iterators.h"
@@ -79,6 +81,16 @@ TEST_CONSTEXPR_CXX20 bool test() {
     auto ret = std::search_n(Iter(a), Iter(a + 3), 0, 7);
     assert(base(ret) == a);
   }
+  { // pattern has negative length. See https://wg21.link/LWG426.
+    int a[]  = {6, 7, 8};
+    auto ret = std::search_n(Iter(a), Iter(a + 3), -1, 7);
+    assert(base(ret) == a);
+  }
+  { // pattern is longer than the range and doesn't fit in the iterator's difference_type
+    int a[]  = {7, 7, 7};
+    auto ret = std::search_n(Iter(a), Iter(a + 3), std::numeric_limits<std::size_t>::max(), 7);
+    assert(base(ret) == a + 3);
+  }
   { // range has zero length
     std::array<int, 0> a = {};
     auto ret             = std::search_n(Iter(a.data()), Iter(a.data()), 1, 1);

``````````

</details>


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