Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds bounds-checking to the second range of absl container algorithms #813

Merged
merged 1 commit into from
Oct 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Adds bounds-checking to the second range of absl container algorithms
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
  • Loading branch information
derekmauro committed Oct 8, 2020
commit aa0bdc5de31fd17241a6f229367b00822ce06efe
74 changes: 55 additions & 19 deletions absl/algorithm/container.h
Original file line number Diff line number Diff line change
Expand Up @@ -328,24 +328,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()
Expand Down Expand Up @@ -515,12 +536,20 @@ OutputIterator c_move(C&& src, OutputIterator 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()
Expand All @@ -538,16 +567,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()
Expand Down
133 changes: 113 additions & 20 deletions absl/algorithm/container_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand All @@ -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_));
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -657,6 +729,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) {
Expand All @@ -671,6 +752,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) {
Expand Down Expand Up @@ -736,21 +831,19 @@ 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) {
std::vector<Element> test_vector = {{1, 1}, {2, 1}, {2, 0}, {1, 0}, {2, 2}};
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) {
Expand Down