Adds bounds-checking to the second range of absl container algorithms (#812)20190808.1

The APIs for the two-range `absl::c_mismatch`, `absl::c_swap_ranges`,
and `absl::c_transform` are misleading as they do not check the bounds
of the second range against the first one.

This commit cleans up ensures that buggy calls are not exploitable;
non-buggy calls are unaffected.

This is consistent with both C++14's two-range `std::` equivalents and
C++20's `std::ranges::` equivalents.
http://wg21.link/mismatch
http://wg21.link/alg.swap
http://wg21.link/alg.transform

2 files changed, 168 insertions, 39 deletions

diff --git a/absl/algorithm/container.h b/absl/algorithm/container.h
index 16389be7..e784099a 100644
--- a/absl/algorithm/container.h
+++ b/absl/algorithm/container.h
@@ -327,24 +327,45 @@ container_algorithm_internal::ContainerDifferenceType<const C> c_count_if(
 // c_mismatch()
 //
 // Container-based version of the <algorithm> `std::mismatch()` function to
-// return the first element where two ordered containers differ.
+// return the first element where two ordered containers differ. Applies `==` to
+// the first N elements of `c1` and `c2`, where N = min(size(c1), size(c2)).
 template <typename C1, typename C2>
 container_algorithm_internal::ContainerIterPairType<C1, C2>
 c_mismatch(C1& c1, C2& c2) {
-  return std::mismatch(container_algorithm_internal::c_begin(c1),
-                       container_algorithm_internal::c_end(c1),
-                       container_algorithm_internal::c_begin(c2));
+  auto first1 = container_algorithm_internal::c_begin(c1);
+  auto last1 = container_algorithm_internal::c_end(c1);
+  auto first2 = container_algorithm_internal::c_begin(c2);
+  auto last2 = container_algorithm_internal::c_end(c2);
+
+  for (; first1 != last1 && first2 != last2; ++first1, (void)++first2) {
+    // Negates equality because Cpp17EqualityComparable doesn't require clients
+    // to overload both `operator==` and `operator!=`.
+    if (!(*first1 == *first2)) {
+      break;
+    }
+  }
+
+  return std::make_pair(first1, first2);
 }
 
 // Overload of c_mismatch() for using a predicate evaluation other than `==` as
-// the function's test condition.
+// the function's test condition. Applies `pred`to the first N elements of `c1`
+// and `c2`, where N = min(size(c1), size(c2)).
 template <typename C1, typename C2, typename BinaryPredicate>
 container_algorithm_internal::ContainerIterPairType<C1, C2>
-c_mismatch(C1& c1, C2& c2, BinaryPredicate&& pred) {
-  return std::mismatch(container_algorithm_internal::c_begin(c1),
-                       container_algorithm_internal::c_end(c1),
-                       container_algorithm_internal::c_begin(c2),
-                       std::forward<BinaryPredicate>(pred));
+c_mismatch(C1& c1, C2& c2, BinaryPredicate pred) {
+  auto first1 = container_algorithm_internal::c_begin(c1);
+  auto last1 = container_algorithm_internal::c_end(c1);
+  auto first2 = container_algorithm_internal::c_begin(c2);
+  auto last2 = container_algorithm_internal::c_end(c2);
+
+  for (; first1 != last1 && first2 != last2; ++first1, (void)++first2) {
+    if (!pred(*first1, *first2)) {
+      break;
+    }
+  }
+
+  return std::make_pair(first1, first2);
 }
 
 // c_equal()
@@ -524,12 +545,20 @@ BidirectionalIterator c_move_backward(C&& src, BidirectionalIterator dest) {
 // c_swap_ranges()
 //
 // Container-based version of the <algorithm> `std::swap_ranges()` function to
-// swap a container's elements with another container's elements.
+// swap a container's elements with another container's elements. Swaps the
+// first N elements of `c1` and `c2`, where N = min(size(c1), size(c2)).
 template <typename C1, typename C2>
 container_algorithm_internal::ContainerIter<C2> c_swap_ranges(C1& c1, C2& c2) {
-  return std::swap_ranges(container_algorithm_internal::c_begin(c1),
-                          container_algorithm_internal::c_end(c1),
-                          container_algorithm_internal::c_begin(c2));
+  auto first1 = container_algorithm_internal::c_begin(c1);
+  auto last1 = container_algorithm_internal::c_end(c1);
+  auto first2 = container_algorithm_internal::c_begin(c2);
+  auto last2 = container_algorithm_internal::c_end(c2);
+
+  using std::swap;
+  for (; first1 != last1 && first2 != last2; ++first1, (void)++first2) {
+    swap(*first1, *first2);
+  }
+  return first2;
 }
 
 // c_transform()
@@ -547,16 +576,23 @@ OutputIterator c_transform(const InputSequence& input, OutputIterator output,
 }
 
 // Overload of c_transform() for performing a transformation using a binary
-// predicate.
+// predicate. Applies `binary_op` to the first N elements of `c1` and `c2`,
+// where N = min(size(c1), size(c2)).
 template <typename InputSequence1, typename InputSequence2,
           typename OutputIterator, typename BinaryOp>
 OutputIterator c_transform(const InputSequence1& input1,
                            const InputSequence2& input2, OutputIterator output,
                            BinaryOp&& binary_op) {
-  return std::transform(container_algorithm_internal::c_begin(input1),
-                        container_algorithm_internal::c_end(input1),
-                        container_algorithm_internal::c_begin(input2), output,
-                        std::forward<BinaryOp>(binary_op));
+  auto first1 = container_algorithm_internal::c_begin(input1);
+  auto last1 = container_algorithm_internal::c_end(input1);
+  auto first2 = container_algorithm_internal::c_begin(input2);
+  auto last2 = container_algorithm_internal::c_end(input2);
+  for (; first1 != last1 && first2 != last2;
+       ++first1, (void)++first2, ++output) {
+    *output = binary_op(*first1, *first2);
+  }
+
+  return output;
 }
 
 // c_replace()
diff --git a/absl/algorithm/container_test.cc b/absl/algorithm/container_test.cc
index 86bf9d3e..9320aa2e 100644
--- a/absl/algorithm/container_test.cc
+++ b/absl/algorithm/container_test.cc
@@ -57,9 +57,7 @@ class NonMutatingTest : public testing::Test {
 };
 
 struct AccumulateCalls {
-  void operator()(int value) {
-    calls.push_back(value);
-  }
+  void operator()(int value) { calls.push_back(value); }
   std::vector<int> calls;
 };
 
@@ -68,7 +66,6 @@ bool BinPredicate(int v1, int v2) { return v1 < v2; }
 bool Equals(int v1, int v2) { return v1 == v2; }
 bool IsOdd(int x) { return x % 2 != 0; }
 
-
 TEST_F(NonMutatingTest, Distance) {
   EXPECT_EQ(container_.size(), absl::c_distance(container_));
   EXPECT_EQ(sequence_.size(), absl::c_distance(sequence_));
@@ -151,13 +148,90 @@ TEST_F(NonMutatingTest, CountIf) {
 }
 
 TEST_F(NonMutatingTest, Mismatch) {
-  absl::c_mismatch(container_, sequence_);
-  absl::c_mismatch(sequence_, container_);
+  // Testing necessary as absl::c_mismatch executes logic.
+  {
+    auto result = absl::c_mismatch(vector_, sequence_);
+    EXPECT_EQ(result.first, vector_.end());
+    EXPECT_EQ(result.second, sequence_.end());
+  }
+  {
+    auto result = absl::c_mismatch(sequence_, vector_);
+    EXPECT_EQ(result.first, sequence_.end());
+    EXPECT_EQ(result.second, vector_.end());
+  }
+
+  sequence_.back() = 5;
+  {
+    auto result = absl::c_mismatch(vector_, sequence_);
+    EXPECT_EQ(result.first, std::prev(vector_.end()));
+    EXPECT_EQ(result.second, std::prev(sequence_.end()));
+  }
+  {
+    auto result = absl::c_mismatch(sequence_, vector_);
+    EXPECT_EQ(result.first, std::prev(sequence_.end()));
+    EXPECT_EQ(result.second, std::prev(vector_.end()));
+  }
+
+  sequence_.pop_back();
+  {
+    auto result = absl::c_mismatch(vector_, sequence_);
+    EXPECT_EQ(result.first, std::prev(vector_.end()));
+    EXPECT_EQ(result.second, sequence_.end());
+  }
+  {
+    auto result = absl::c_mismatch(sequence_, vector_);
+    EXPECT_EQ(result.first, sequence_.end());
+    EXPECT_EQ(result.second, std::prev(vector_.end()));
+  }
+  {
+    struct NoNotEquals {
+      constexpr bool operator==(NoNotEquals) const { return true; }
+      constexpr bool operator!=(NoNotEquals) const = delete;
+    };
+    std::vector<NoNotEquals> first;
+    std::list<NoNotEquals> second;
+
+    // Check this still compiles.
+    absl::c_mismatch(first, second);
+  }
 }
 
 TEST_F(NonMutatingTest, MismatchWithPredicate) {
-  absl::c_mismatch(container_, sequence_, BinPredicate);
-  absl::c_mismatch(sequence_, container_, BinPredicate);
+  // Testing necessary as absl::c_mismatch executes logic.
+  {
+    auto result = absl::c_mismatch(vector_, sequence_, BinPredicate);
+    EXPECT_EQ(result.first, vector_.begin());
+    EXPECT_EQ(result.second, sequence_.begin());
+  }
+  {
+    auto result = absl::c_mismatch(sequence_, vector_, BinPredicate);
+    EXPECT_EQ(result.first, sequence_.begin());
+    EXPECT_EQ(result.second, vector_.begin());
+  }
+
+  sequence_.front() = 0;
+  {
+    auto result = absl::c_mismatch(vector_, sequence_, BinPredicate);
+    EXPECT_EQ(result.first, vector_.begin());
+    EXPECT_EQ(result.second, sequence_.begin());
+  }
+  {
+    auto result = absl::c_mismatch(sequence_, vector_, BinPredicate);
+    EXPECT_EQ(result.first, std::next(sequence_.begin()));
+    EXPECT_EQ(result.second, std::next(vector_.begin()));
+  }
+
+  sequence_.clear();
+  {
+    auto result = absl::c_mismatch(vector_, sequence_, BinPredicate);
+    EXPECT_EQ(result.first, vector_.begin());
+    EXPECT_EQ(result.second, sequence_.end());
+  }
+  {
+    auto result = absl::c_mismatch(sequence_, vector_, BinPredicate);
+    EXPECT_EQ(result.first, sequence_.end());
+    EXPECT_EQ(result.second, vector_.begin());
+  }
 }
 
 TEST_F(NonMutatingTest, Equal) {
@@ -513,11 +587,9 @@ TEST_F(SortingTest, IsSortedUntil) {
 TEST_F(SortingTest, NthElement) {
   std::vector<int> unsorted = {2, 4, 1, 3};
   absl::c_nth_element(unsorted, unsorted.begin() + 2);
-  EXPECT_THAT(unsorted,
-              ElementsAre(Lt(3), Lt(3), 3, Gt(3)));
+  EXPECT_THAT(unsorted, ElementsAre(Lt(3), Lt(3), 3, Gt(3)));
   absl::c_nth_element(unsorted, unsorted.begin() + 2, std::greater<int>());
-  EXPECT_THAT(unsorted,
-              ElementsAre(Gt(2), Gt(2), 2, Lt(2)));
+  EXPECT_THAT(unsorted, ElementsAre(Gt(2), Gt(2), 2, Lt(2)));
 }
 
 TEST(MutatingTest, IsPartitioned) {
@@ -670,6 +742,15 @@ TEST(MutatingTest, SwapRanges) {
   absl::c_swap_ranges(odds, evens);
   EXPECT_THAT(odds, ElementsAre(1, 3, 5));
   EXPECT_THAT(evens, ElementsAre(2, 4, 6));
+
+  odds.pop_back();
+  absl::c_swap_ranges(odds, evens);
+  EXPECT_THAT(odds, ElementsAre(2, 4));
+  EXPECT_THAT(evens, ElementsAre(1, 3, 6));
+
+  absl::c_swap_ranges(evens, odds);
+  EXPECT_THAT(odds, ElementsAre(1, 3));
+  EXPECT_THAT(evens, ElementsAre(2, 4, 6));
 }
 
 TEST_F(NonMutatingTest, Transform) {
@@ -684,6 +765,20 @@ TEST_F(NonMutatingTest, Transform) {
   EXPECT_EQ(std::vector<int>({1, 5, 4}), z);
   *end = 7;
   EXPECT_EQ(std::vector<int>({1, 5, 4, 7}), z);
+
+  z.clear();
+  y.pop_back();
+  end = absl::c_transform(x, y, std::back_inserter(z), std::plus<int>());
+  EXPECT_EQ(std::vector<int>({1, 5}), z);
+  *end = 7;
+  EXPECT_EQ(std::vector<int>({1, 5, 7}), z);
+
+  z.clear();
+  std::swap(x, y);
+  end = absl::c_transform(x, y, std::back_inserter(z), std::plus<int>());
+  EXPECT_EQ(std::vector<int>({1, 5}), z);
+  *end = 7;
+  EXPECT_EQ(std::vector<int>({1, 5, 7}), z);
 }
 
 TEST(MutatingTest, Replace) {
@@ -749,10 +844,9 @@ MATCHER_P2(IsElement, key, value, "") {
 TEST(MutatingTest, StableSort) {
   std::vector<Element> test_vector = {{1, 1}, {2, 1}, {2, 0}, {1, 0}, {2, 2}};
   absl::c_stable_sort(test_vector);
-  EXPECT_THAT(
-      test_vector,
-      ElementsAre(IsElement(1, 1), IsElement(1, 0), IsElement(2, 1),
-                  IsElement(2, 0), IsElement(2, 2)));
+  EXPECT_THAT(test_vector,
+              ElementsAre(IsElement(1, 1), IsElement(1, 0), IsElement(2, 1),
+                          IsElement(2, 0), IsElement(2, 2)));
 }
 
 TEST(MutatingTest, StableSortWithPredicate) {
@@ -760,10 +854,9 @@ TEST(MutatingTest, StableSortWithPredicate) {
   absl::c_stable_sort(test_vector, [](const Element& e1, const Element& e2) {
     return e2 < e1;
   });
-  EXPECT_THAT(
-      test_vector,
-      ElementsAre(IsElement(2, 1), IsElement(2, 0), IsElement(2, 2),
-                  IsElement(1, 1), IsElement(1, 0)));
+  EXPECT_THAT(test_vector,
+              ElementsAre(IsElement(2, 1), IsElement(2, 0), IsElement(2, 2),
+                          IsElement(1, 1), IsElement(1, 0)));
 }
 
 TEST(MutatingTest, ReplaceCopyIf) {