-
Notifications
You must be signed in to change notification settings - Fork 1.5k
STL Hardening
- CDL is
_CONTAINER_DEBUG_LEVEL
.- This was an earlier attempt at something resembling hardening which was never formally designed, and which was never adopted in practice. It is not enabled by default.
- IDL is
_ITERATOR_DEBUG_LEVEL
.- Release mode defaults to IDL=0 (no checking).
- Debug mode defaults to IDL=2 (exhaustive correctness checking, extremely expensive).
- Attempting to enable IDL=2 in release mode is flatly forbidden with a hard
#error
. - Release mode IDL=1 (formerly known as
_SECURE_SCL
in VS 2005/2008) was the first attempt at something resembling a security-hardened mode. This is an ABI-breaking option, changing the representations of containers and iterators, and introduces dynamic memory allocations. It is quite expensive (2x perf penalty in the worst case), which users rejected. IDL=1 remains technically supported, but it is never enabled by default, virtually no users are aware of it, and we strongly discourage its use. It will be eliminated when we can break ABI in vNext.
- Review:
- P3471R1 Standard Library Hardening
- Existing CDL checks (below)
- libc++'s hardening modes
- Design a consistent policy for what should be hardened
- Bonus: Should
unique_ptr
,shared_ptr
, andfunction
destructors set their state to null, to defend against use-after-frees?- Should we instead use a bogus sentinel value, like pointer values forbidden by the x64 ISA, instead of trying to make dtors idempotent?
- Comment product code clearly.
- Update
VSO_0000000_wcfb01_idempotent_container_destructors
accordingly. - Does the optimizer skip such assignments?
- Consider adding an optimized configuration to the test.
- Design how this will be controlled, and its defaults
- Opt-in means virtually no users will benefit, but has the least performance requirements
- Implied-by-
/sdl
means some users will benefit automatically - Opt-out means all users will benefit automatically, but has the most stringent performance requirements
- Have a coarse-grained macro plus fine-grained macros per class?
- Design a termination mechanism:
- Is CDL's termination mechanism ideal?
- Should violations terminate the program differently?
- Should it be customizable?
- Some users may slightly prefer
abort
soSIGABRT
can be intercepted, but this is not absolutely necessary
- With a design, make the code changes (should be the easy part as 90% of the work is already present)
- Optimizations to mitigate perf impact, once we have examples of where the perf impact will be
- Loop in the compiler back-end
- Should we use
[[unlikely]]
? - Additional library changes may also be necessary
Class | Member | Notes |
---|---|---|
vector |
operator[] |
✅ P3471R1's proposed wording |
vector |
front |
✅ P3471R1's design |
vector |
back |
✅ P3471R1's design |
vector |
pop_back |
✅ P3471R1's design |
vector<bool> |
operator[] |
✅ P3471R1's proposed wording |
vector<bool> |
front |
✅ P3471R1's design |
vector<bool> |
back |
✅ P3471R1's design |
vector<bool> |
pop_back |
✅ P3471R1's design |
deque |
operator[] |
✅ P3471R1's proposed wording |
deque |
front |
✅ P3471R1's design |
deque |
back |
✅ P3471R1's design |
deque |
pop_front |
✅ P3471R1's design |
deque |
pop_back |
✅ P3471R1's design |
list |
front |
✅ P3471R1's design |
list |
back |
✅ P3471R1's design |
list |
pop_front |
✅ P3471R1's design |
list |
pop_back |
✅ P3471R1's design |
forward_list |
front |
✅ P3471R1's design |
forward_list |
pop_front |
✅ P3471R1's design |
array |
operator[] |
✅ P3471R1's proposed wording |
array<T, 0> |
operator[] |
✅ P3471R1's proposed wording |
array<T, 0> |
front |
✅ P3471R1's design |
array<T, 0> |
back |
✅ P3471R1's design |
Class | Member | Notes |
---|---|---|
basic_string |
operator[] |
✅ P3471R1's proposed wording |
basic_string |
front |
✅ P3471R1's design |
basic_string |
back |
✅ P3471R1's design |
basic_string |
resize_and_overwrite |
❌ Should be excluded - not hardened by libc++, difficult to mess up the operation's returned size |
basic_string |
pop_back |
✅ P3471R1's design |
basic_string_view |
basic_string_view(P, N) |
❌ Should be excluded - checking for (null, non-zero) is not critical |
basic_string_view |
operator[] |
✅ P3471R1's proposed wording |
basic_string_view |
front |
✅ P3471R1's design |
basic_string_view |
back |
✅ P3471R1's design |
basic_string_view |
remove_prefix |
✅ Morally equivalent to pop_front
|
basic_string_view |
remove_suffix |
✅ Morally equivalent to pop_back
|
Class | Member | Notes |
---|---|---|
span |
span(ARGS) |
✅ Limited to spans with static extents, verifying valid construction is important to verify that elements actually exist |
span |
first |
✅ Similar to pop_back
|
span |
last |
✅ Similar to pop_front
|
span |
subspan |
✅ Similar to pop_front and pop_back
|
span |
size_bytes |
❌ Should be excluded - size would have to be extremely bogus for size_bytes to overflow, i.e. the span is already doomed |
span |
operator[] |
✅ P3471R1's proposed wording |
span |
front |
✅ P3471R1's design |
span |
back |
✅ P3471R1's design |
mdspan |
static_extent |
❌ Should be excluded - not checking all library preconditions |
mdspan |
mdspan(OTHER) |
❌ Should be excluded - not checking all library preconditions |
mdspan |
operator[] |
✅ P3471R1's design |
mdspan |
size |
❌ Should be excluded - checking for a pathological overflow case |
Class | Member | Notes |
---|---|---|
optional |
operator* |
✅ P3471R1's proposed wording |
optional |
operator-> |
✅ P3471R1's proposed wording |
expected<T, E> |
operator-> |
✅ P3471R1's proposed wording |
expected<T, E> |
operator* |
✅ P3471R1's proposed wording |
expected<T, E> |
error |
✅ Logically follows from P3471R1's design |
expected<void, E> |
operator* |
✅ Precondition with no behavior; when has_value() is false , then an operation has failed |
expected<void, E> |
error |
✅ Logically follows from P3471R1's design |
Class | Member | Notes |
---|---|---|
ranges::view_interface |
operator[] |
✅ P3471R1's design |
ranges::view_interface |
front |
✅ P3471R1's design |
ranges::view_interface |
back |
✅ P3471R1's design |
iota_view |
iota_view(ARGS) |
❌ Should be excluded - not checking all library preconditions |
repeat_view::iterator |
operator++ |
❌ Should be excluded - not checking all library preconditions |
repeat_view::iterator |
operator-- |
❌ Should be excluded - not checking all library preconditions |
repeat_view::iterator |
operator+= |
❌ Should be excluded - not checking all library preconditions |
repeat_view::iterator |
operator-= |
❌ Should be excluded - not checking all library preconditions |
repeat_view |
repeat_view(ARGS) |
❌ Should be excluded - not checking all library preconditions |
filter_view |
pred |
❌ Should be excluded - not checking all library preconditions |
filter_view |
begin |
❌ Should be excluded - not checking all library preconditions |
take_view |
take_view(ARGS) |
❌ Should be excluded - not checking all library preconditions |
take_while_view |
pred |
❌ Should be excluded - not checking all library preconditions |
take_while_view |
end |
❌ Should be excluded - not checking all library preconditions |
drop_view |
drop_view(ARGS) |
❌ Should be excluded - not checking all library preconditions |
drop_while_view |
pred |
❌ Should be excluded - not checking all library preconditions |
drop_while_view |
begin |
❌ Should be excluded - not checking all library preconditions |
views::counted |
operator() |
❌ Should be excluded - not checking all library preconditions |
chunk_view |
chunk_view(ARGS) |
❌ Should be excluded - not checking all library preconditions |
slide_view |
slide_view(ARGS) |
❌ Should be excluded - not checking all library preconditions |
chunk_by_view |
pred |
❌ Should be excluded - not checking all library preconditions |
chunk_by_view |
begin |
❌ Should be excluded - not checking all library preconditions |
stride_view |
stride_view(ARGS) |
❌ Should be excluded - not checking all library preconditions |
cartesian_product_view |
size |
❌ Should be excluded - not checking all library preconditions |
Class | Member | Notes |
---|---|---|
generator::iterator |
operator* |
❌ Should be excluded - not checking all library preconditions |
generator::iterator |
operator++ |
❌ Should be excluded - not checking all library preconditions |
generator |
begin |
❌ Should be excluded - not checking all library preconditions |
Class | Member | Notes |
---|---|---|
valarray |
operator[] |
✅ P3471R1's design ❔ valarray is an unusual, rarely-used, high-performance data structure. It's unclear whether it should be hardened. |
valarray |
28 binary ops | ❌ Should be excluded - checking that the valarray s are the same size is not relevant for memory safety, as long as operator[] is hardened |
Class | Member | Notes |
---|---|---|
extents |
extents(ARGS) |
❌ Should be excluded - not checking all library preconditions |
extents |
static_extent |
❌ Should be excluded - not checking all library preconditions |
extents |
extent |
❌ Should be excluded - not checking all library preconditions |
layout_left::mapping |
mapping(ARGS) |
❌ Should be excluded - not checking all library preconditions |
layout_left::mapping |
stride |
❌ Should be excluded - not checking all library preconditions |
layout_left::mapping |
operator() |
❌ Should be excluded - not checking all library preconditions |
layout_right::mapping |
mapping(ARGS) |
❌ Should be excluded - not checking all library preconditions |
layout_right::mapping |
stride |
❌ Should be excluded - not checking all library preconditions |
layout_right::mapping |
operator() |
❌ Should be excluded - not checking all library preconditions |
layout_stride::mapping |
mapping(ARGS) |
❌ Should be excluded - not checking all library preconditions |
layout_stride::mapping |
stride |
❌ Should be excluded - not checking all library preconditions |
layout_stride::mapping |
operator() |
❌ Should be excluded - not checking all library preconditions |
Class | Member | Notes |
---|---|---|
condition_variable |
wait_until(ARGS) |
❌ Should be excluded - not relevant for memory safety |
-
VSO-1556181
gsl::span
CQ deficiency: predicate inference weakness #1 -
VSO-1556194
gsl::span
CQ deficiency: useless multibyte copy -
VSO-1556195
gsl::span
CQ deficiency: predicate inference weakness #2
- #5090 "Implement a hardened mode"
P3471R1 Standard Library Hardening
Stephan's highlights:
Hardening needs to be lightweight enough to be used in production environments. In our experience, a debug-only approach is not sufficient to really move the needle security-wise. Thus, the goal is to identify the minimal set of checks that would deliver the most value to users while requiring minimal overhead, with a particular focus on memory safety as a major source of security vulnerabilities. Checking all preconditions in the library is an explicit non-goal, and would be impossible for many semantic requirements anyway.
To reiterate the last point, an important design principle is that hardening needs to be lightweight enough for production use by a wide variety of real-world programs. In our experience in libc++, a small set of checks that is widely used delivers far more value than a more extensive set of checks that is only enabled by select few users. Thankfully, many of the most valuable checks, such as checking for out-of-bounds access in standard containers, also happen to be relatively cheap.
In the initial proposal, we decide to focus on hardened preconditions that prevent out-of-bounds memory access, i.e., compromise the memory safety of the program. These are some of the most valuable for the user since they help prevent potential security vulnerabilities; many of them are also relatively cheap to implement. More hardened preconditions can potentially be added in the future, but the intent is for their number to be limited to keep hardening viable for production use. Specifically, the proposal is to add hardened preconditions to:
- Accessors of sequence containers,
std::span
,std::mdspan
,std::string
,std::string_view
and other similar classes that might attempt to access non-existent elements (e.g.back()
on an empty container oroperator[]
with an invalid index).- Modifiers of sequence containers and string classes that assume the container to be non-empty (e.g.
pop_back()
).- Accessors of
optional
andexpected
that expect the object to be non-empty.
In our experience, hardening all of these operations is trivial to implement and provides significant security value.