From bd0de71e754eb3280094e89c7ac35a14dac6d61c Mon Sep 17 00:00:00 2001 From: Derek Mauro <761129+derekmauro@users.noreply.github.com> Date: Fri, 9 Oct 2020 14:49:09 -0400 Subject: Adds bounds-checking to the second range of absl container algorithms (#810) 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 --- absl/algorithm/container_test.cc | 133 +++++++++++++++++++++++++++++++++------ 1 file changed, 113 insertions(+), 20 deletions(-) (limited to 'absl/algorithm/container_test.cc') diff --git a/absl/algorithm/container_test.cc b/absl/algorithm/container_test.cc index 0a4abe94..605afc80 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 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 first; + std::list 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) { @@ -519,11 +593,9 @@ TEST_F(SortingTest, IsSortedUntil) { TEST_F(SortingTest, NthElement) { std::vector 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()); - 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) { @@ -676,6 +748,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) { @@ -690,6 +771,20 @@ TEST_F(NonMutatingTest, Transform) { EXPECT_EQ(std::vector({1, 5, 4}), z); *end = 7; EXPECT_EQ(std::vector({1, 5, 4, 7}), z); + + z.clear(); + y.pop_back(); + end = absl::c_transform(x, y, std::back_inserter(z), std::plus()); + EXPECT_EQ(std::vector({1, 5}), z); + *end = 7; + EXPECT_EQ(std::vector({1, 5, 7}), z); + + z.clear(); + std::swap(x, y); + end = absl::c_transform(x, y, std::back_inserter(z), std::plus()); + EXPECT_EQ(std::vector({1, 5}), z); + *end = 7; + EXPECT_EQ(std::vector({1, 5, 7}), z); } TEST(MutatingTest, Replace) { @@ -755,10 +850,9 @@ MATCHER_P2(IsElement, key, value, "") { TEST(MutatingTest, StableSort) { std::vector 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) { @@ -766,10 +860,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) { -- cgit v1.2.3