Skip to content

Commit

Permalink
Adds bounds-checking to the second range of absl container algorithms (…
Browse files Browse the repository at this point in the history 
…#813)

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
derekmauro authored Oct 9, 2020
1 parent fcb1045 commit 49bd3cc
Show file tree
Hide file tree
Showing 2 changed files with 168 additions and 39 deletions.
74 changes: 55 additions & 19 deletions absl/algorithm/container.h
View file
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
View file
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

0 comments on commit 49bd3cc

Please sign in to comment.