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

[libc++][hardening] Use bounded iterators in std::vector and std::string #78929

Merged
merged 9 commits into from
Jul 23, 2024
2 changes: 2 additions & 0 deletions libcxx/include/__iterator/bounded_iter.h
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,8 @@ struct __bounded_iter {
private:
var-const marked this conversation as resolved.
Show resolved Hide resolved
template <class>
friend struct pointer_traits;
template <class, class>
friend struct __bounded_iter;
_Iterator __current_; // current iterator
_Iterator __begin_, __end_; // valid range represented as [begin, end]
};
Expand Down
26 changes: 25 additions & 1 deletion libcxx/include/string
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,7 @@ basic_string<char32_t> operator""s( const char32_t *str, size_t len );
#include <__functional/unary_function.h>
#include <__fwd/string.h>
#include <__ios/fpos.h>
#include <__iterator/bounded_iter.h>
#include <__iterator/distance.h>
#include <__iterator/iterator_traits.h>
#include <__iterator/reverse_iterator.h>
Expand Down Expand Up @@ -786,9 +787,16 @@ public:
"[allocator.requirements] states that rebinding an allocator to the same type should result in the "
"original allocator");

// TODO: Implement iterator bounds checking without requiring the global database.
#ifdef _LIBCPP_ABI_BOUNDED_ITERATORS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#ifdef _LIBCPP_ABI_BOUNDED_ITERATORS
#ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_STRING

I think we should introduce this under its own ABI macro, and the same for vector. And we should rename the existing macro to explicitly mention the containers they enable. I think that's the only way to pretend that these ABI macros yield a somewhat stable ABI.

In this patch, please introduce _LIBCPP_ABI_BOUNDED_ITERATORS_IN_STRING and _LIBCPP_ABI_BOUNDED_ITERATORS_IN_VECTOR and I can handle the related cleanup as a follow-up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done (leaving the comment open as a reminder that we need to do some further cleanup).

// The pointer must be passed through __wrap_iter because
// __alloc_traits::pointer may not be detected as a continguous iterator on
// its own.
var-const marked this conversation as resolved.
Show resolved Hide resolved
typedef __bounded_iter<__wrap_iter<pointer>> iterator;
typedef __bounded_iter<__wrap_iter<const_pointer>> const_iterator;
#else
typedef __wrap_iter<pointer> iterator;
typedef __wrap_iter<const_pointer> const_iterator;
#endif
typedef std::reverse_iterator<iterator> reverse_iterator;
typedef std::reverse_iterator<const_iterator> const_reverse_iterator;

Expand Down Expand Up @@ -918,11 +926,27 @@ private:
__init_with_sentinel(std::move(__first), std::move(__last));
}

#ifdef _LIBCPP_ABI_BOUNDED_ITERATORS
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 iterator __make_iterator(pointer __p) {
return __make_bounded_iter(
var-const marked this conversation as resolved.
Show resolved Hide resolved
__wrap_iter<pointer>(__p),
__wrap_iter<pointer>(__get_pointer()),
__wrap_iter<pointer>(__get_pointer() + size()));
var-const marked this conversation as resolved.
Show resolved Hide resolved
}

_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 const_iterator __make_const_iterator(const_pointer __p) const {
return __make_bounded_iter(
var-const marked this conversation as resolved.
Show resolved Hide resolved
__wrap_iter<const_pointer>(__p),
__wrap_iter<const_pointer>(__get_pointer()),
__wrap_iter<const_pointer>(__get_pointer() + size()));
}
#else
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 iterator __make_iterator(pointer __p) { return iterator(__p); }

_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 const_iterator __make_const_iterator(const_pointer __p) const {
return const_iterator(__p);
}
#endif

public:
_LIBCPP_TEMPLATE_DATA_VIS static const size_type npos = -1;
Expand Down
30 changes: 29 additions & 1 deletion libcxx/include/vector
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@ template<class T, class charT> requires is-vector-bool-reference<T> // Since C++
#include <__functional/unary_function.h>
#include <__fwd/vector.h>
#include <__iterator/advance.h>
#include <__iterator/bounded_iter.h>
#include <__iterator/distance.h>
#include <__iterator/iterator_traits.h>
#include <__iterator/reverse_iterator.h>
Expand Down Expand Up @@ -401,9 +402,16 @@ public:
typedef typename __alloc_traits::difference_type difference_type;
typedef typename __alloc_traits::pointer pointer;
typedef typename __alloc_traits::const_pointer const_pointer;
// TODO: Implement iterator bounds checking without requiring the global database.
#ifdef _LIBCPP_ABI_BOUNDED_ITERATORS
// The pointer must be passed through __wrap_iter because
// __alloc_traits::pointer may not be detected as a continguous iterator on
// its own.
var-const marked this conversation as resolved.
Show resolved Hide resolved
typedef __bounded_iter<__wrap_iter<pointer>> iterator;
typedef __bounded_iter<__wrap_iter<const_pointer>> const_iterator;
#else
typedef __wrap_iter<pointer> iterator;
typedef __wrap_iter<const_pointer> const_iterator;
#endif
typedef std::reverse_iterator<iterator> reverse_iterator;
typedef std::reverse_iterator<const_iterator> const_reverse_iterator;

Expand Down Expand Up @@ -798,10 +806,30 @@ private:
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void __append(size_type __n);
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void __append(size_type __n, const_reference __x);
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI iterator __make_iter(pointer __p) _NOEXCEPT {
#ifdef _LIBCPP_ABI_BOUNDED_ITERATORS
// Bound the iterator according to the capacity, rather than the size.
// Resizing a vector up to the capacity will not invalidate iterators, so,
// Without a way to update all live iterators on resize, we must
var-const marked this conversation as resolved.
Show resolved Hide resolved
// conservatively bound the iterator by the capacity rather than the size.
return __make_bounded_iter(
__wrap_iter<pointer>(__p), __wrap_iter<pointer>(this->__begin_), __wrap_iter<pointer>(this->__end_cap()));
#else
return iterator(__p);
#endif
}
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI const_iterator __make_iter(const_pointer __p) const _NOEXCEPT {
#ifdef _LIBCPP_ABI_BOUNDED_ITERATORS
// Bound the iterator according to the capacity, rather than the size.
// Resizing a vector up to the capacity will not invalidate iterators, so,
// Without a way to update all live iterators on resize, we must
// conservatively bound the iterator by the capacity rather than the size.
return __make_bounded_iter(
__wrap_iter<const_pointer>(__p),
__wrap_iter<const_pointer>(this->__begin_),
__wrap_iter<const_pointer>(this->__end_cap()));
#else
return const_iterator(__p);
#endif
}
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
__swap_out_circular_buffer(__split_buffer<value_type, allocator_type&>& __v);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,36 +10,48 @@

var-const marked this conversation as resolved.
Show resolved Hide resolved
// Add to iterator out of bounds.

// REQUIRES: has-unix-headers
// UNSUPPORTED: !libcpp-has-legacy-debug-mode, c++03
// REQUIRES: has-unix-headers, libcpp-has-abi-bounded-iterators
// UNSUPPORTED: libcpp-hardening-mode=none, c++03

#include <vector>
#include <cassert>

#include "check_assertion.h"
#include "min_allocator.h"

template <typename T, typename A>
void fill_to_capacity(std::vector<T, A>& vec) {
// Fill vec up to its capacity. Our bounded iterators currently unable to
// catch accesses between size and capacity due to iterator stability
// guarantees. This function clears those effects.
while (vec.size() < vec.capacity()) {
vec.push_back(T());
}
}

int main(int, char**) {
{
typedef int T;
typedef std::vector<T> C;
C c(1);
fill_to_capacity(c);
C::iterator i = c.begin();
i += 1;
i += c.size();
assert(i == c.end());
i = c.begin();
TEST_LIBCPP_ASSERT_FAILURE(i + 2, "Attempted to add/subtract an iterator outside its valid range");
TEST_LIBCPP_ASSERT_FAILURE(i + 2, "__bounded_iter::operator+=: Attempt to advance an iterator past the end");
}

{
typedef int T;
typedef std::vector<T, min_allocator<T> > C;
C c(1);
fill_to_capacity(c);
C::iterator i = c.begin();
i += 1;
i += c.size();
assert(i == c.end());
i = c.begin();
TEST_LIBCPP_ASSERT_FAILURE(i + 2, "Attempted to add/subtract an iterator outside its valid range");
TEST_LIBCPP_ASSERT_FAILURE(i + 2, "__bounded_iter::operator+=: Attempt to advance an iterator past the end");
}

return 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@

// Decrement iterator prior to begin.

// REQUIRES: has-unix-headers
// UNSUPPORTED: !libcpp-has-legacy-debug-mode, c++03
// REQUIRES: has-unix-headers, libcpp-has-abi-bounded-iterators
// UNSUPPORTED: libcpp-hardening-mode=none, c++03

#include <vector>
#include <cassert>
Expand All @@ -27,7 +27,7 @@ int main(int, char**) {
C::iterator i = c.end();
--i;
assert(i == c.begin());
TEST_LIBCPP_ASSERT_FAILURE(--i, "Attempted to decrement a non-decrementable iterator");
TEST_LIBCPP_ASSERT_FAILURE(--i, "__bounded_iter::operator--: Attempt to rewind an iterator past the start");
}

{
Expand All @@ -37,7 +37,7 @@ int main(int, char**) {
C::iterator i = c.end();
--i;
assert(i == c.begin());
TEST_LIBCPP_ASSERT_FAILURE(--i, "Attempted to decrement a non-decrementable iterator");
TEST_LIBCPP_ASSERT_FAILURE(--i, "__bounded_iter::operator--: Attempt to rewind an iterator past the start");
}

return 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,29 +10,41 @@

// Dereference non-dereferenceable iterator.

// REQUIRES: has-unix-headers
// UNSUPPORTED: !libcpp-has-legacy-debug-mode, c++03
// REQUIRES: has-unix-headers, libcpp-has-abi-bounded-iterators
// UNSUPPORTED: libcpp-hardening-mode=none, c++03

#include <vector>

#include "check_assertion.h"
#include "min_allocator.h"

template <typename T, typename A>
void fill_to_capacity(std::vector<T, A>& vec) {
// Fill vec up to its capacity. Our bounded iterators currently unable to
// catch accesses between size and capacity due to iterator stability
// guarantees. This function clears those effects.
while (vec.size() < vec.capacity()) {
vec.push_back(T());
}
}

int main(int, char**) {
{
typedef int T;
typedef std::vector<T> C;
C c(1);
fill_to_capacity(c);
C::iterator i = c.end();
TEST_LIBCPP_ASSERT_FAILURE(*i, "Attempted to dereference a non-dereferenceable iterator");
TEST_LIBCPP_ASSERT_FAILURE(*i, "__bounded_iter::operator*: Attempt to dereference an iterator at the end");
}

{
typedef int T;
typedef std::vector<T, min_allocator<T> > C;
C c(1);
fill_to_capacity(c);
C::iterator i = c.end();
TEST_LIBCPP_ASSERT_FAILURE(*i, "Attempted to dereference a non-dereferenceable iterator");
TEST_LIBCPP_ASSERT_FAILURE(*i, "__bounded_iter::operator*: Attempt to dereference an iterator at the end");
}

return 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,34 +10,46 @@

// Increment iterator past end.

// REQUIRES: has-unix-headers
// UNSUPPORTED: !libcpp-has-legacy-debug-mode, c++03
// REQUIRES: has-unix-headers, libcpp-has-abi-bounded-iterators
// UNSUPPORTED: libcpp-hardening-mode=none, c++03

#include <vector>
#include <cassert>

#include "check_assertion.h"
#include "min_allocator.h"

template <typename T, typename A>
void fill_to_capacity(std::vector<T, A>& vec) {
// Fill vec up to its capacity. Our bounded iterators currently unable to
// catch accesses between size and capacity due to iterator stability
// guarantees. This function clears those effects.
while (vec.size() < vec.capacity()) {
vec.push_back(T());
}
}

int main(int, char**) {
{
typedef int T;
typedef std::vector<T> C;
C c(1);
fill_to_capacity(c);
C::iterator i = c.begin();
++i;
i += c.size();
assert(i == c.end());
TEST_LIBCPP_ASSERT_FAILURE(++i, "Attempted to increment a non-incrementable iterator");
TEST_LIBCPP_ASSERT_FAILURE(++i, "__bounded_iter::operator++: Attempt to advance an iterator past the end");
}

{
typedef int T;
typedef std::vector<T, min_allocator<T> > C;
C c(1);
fill_to_capacity(c);
C::iterator i = c.begin();
++i;
i += c.size();
assert(i == c.end());
TEST_LIBCPP_ASSERT_FAILURE(++i, "Attempted to increment a non-incrementable iterator");
TEST_LIBCPP_ASSERT_FAILURE(++i, "__bounded_iter::operator++: Attempt to advance an iterator past the end");
}

return 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,32 +10,50 @@

// Index iterator out of bounds.

// REQUIRES: has-unix-headers
// UNSUPPORTED: !libcpp-has-legacy-debug-mode, c++03
// REQUIRES: has-unix-headers, libcpp-has-abi-bounded-iterators
// UNSUPPORTED: libcpp-hardening-mode=none, c++03

#include <vector>
#include <cassert>

#include "check_assertion.h"
#include "min_allocator.h"

template <typename T, typename A>
void fill_to_capacity(std::vector<T, A>& vec) {
// Fill vec up to its capacity. Our bounded iterators currently unable to
// catch accesses between size and capacity due to iterator stability
// guarantees. This function clears those effects.
while (vec.size() < vec.capacity()) {
vec.push_back(T());
}
}

int main(int, char**) {
{
typedef int T;
typedef std::vector<T> C;
C c(1);
fill_to_capacity(c);
C::iterator i = c.begin();
assert(i[0] == 0);
TEST_LIBCPP_ASSERT_FAILURE(i[1], "Attempted to subscript an iterator outside its valid range");
TEST_LIBCPP_ASSERT_FAILURE(
i[c.size()], "__bounded_iter::operator[]: Attempt to index an iterator at or past the end");
TEST_LIBCPP_ASSERT_FAILURE(
i[c.size() + 1], "__bounded_iter::operator[]: Attempt to index an iterator at or past the end");
}

{
typedef int T;
typedef std::vector<T, min_allocator<T> > C;
C c(1);
fill_to_capacity(c);
C::iterator i = c.begin();
assert(i[0] == 0);
TEST_LIBCPP_ASSERT_FAILURE(i[1], "Attempted to subscript an iterator outside its valid range");
TEST_LIBCPP_ASSERT_FAILURE(
i[c.size()], "__bounded_iter::operator[]: Attempt to index an iterator at or past the end");
TEST_LIBCPP_ASSERT_FAILURE(
i[c.size() + 1], "__bounded_iter::operator[]: Attempt to index an iterator at or past the end");
}

return 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

// UNSUPPORTED: c++03

#include <iterator>
#include <string>

#include "test_macros.h"
Expand All @@ -18,6 +19,14 @@

template <class T>
class small_pointer {
public:
using value_type = T;
using difference_type = std::int16_t;
using pointer = small_pointer;
using reference = T&;
using iterator_category = std::random_access_iterator_tag;

private:
std::uint16_t offset;
};

Expand Down
Loading
Loading