Skip to content

Memory safety / robustness: replace_range_at buffer overflow, index_range int overflow, abort() on misuse, mutable global #29

@jkalias

Description

@jkalias

Summary

The library has no I/O, parsing, or network surface, so classic exploit exposure is low — but there are genuine memory-safety / robustness defects, mostly stemming from assert-only validation (compiled out under NDEBUG) and from int-based indexing in index_range.


1. replace_range_at / replacing_range_at can silently overflow the buffer in release builds

Location: include/vector.h:2017-2027 (and replacing_range_at_imp, 2035-2046)

const auto vec_size = std::distance(vec_begin, vec_end);
assert(index + vec_size >= vec_size && index + vec_size <= size());
std::copy(vec_begin, vec_end, m_vector.begin() + index);

The only bounds guard is assert, which disappears under NDEBUG. Unlike operator[] (which deliberately mirrors std::vector's documented unchecked behavior), there is no standard analogue here: if index + vec_size > size(), std::copy writes past the end of the vector → heap buffer overflow / undefined behavior. If index or the source length can derive from untrusted input, this is exploitable.

Suggested fix: validate at runtime regardless of build type — either clamp the copy to the available space, or throw std::out_of_range, or document it loudly as strictly unchecked (consistent with operator[]).


2. index_range uses int for indices → signed overflow and signed/unsigned mixing

Location: include/index_range.h:49-75, include/vector.h:1287, 1305

index_range stores start/end/count as int, while the containers use size_t. In remove_range / removing_range:

if (!range.is_valid || size() < range.end + 1)
  • range.end + 1 is signed int arithmetic — UB if end == INT_MAX.
  • size() < range.end + 1 mixes size_t and int. (The project currently suppresses MSVC warnings 4018/4389 in tests/warnings.h, which hides the conversion rather than fixing it.)
  • Vectors larger than INT_MAX cannot be addressed through index_range at all.

Suggested fix: use size_t consistently in index_range (with a separate validity flag for "no range"), which removes the overflow and the whole class of sign-conversion warnings.


3. assert(false); std::abort(); hard-crashes on recoverable misuse

Location: include/vector.h:215-225, 271-282 (lazy zip); include/set.h:390-402, 416-428 (lazy zip)

if (index >= vector_copy.size()) {
    assert(false);
    std::abort();
}

On a size mismatch (a recoverable precondition violation), the entire process is terminated. Under NDEBUG the assert is a no-op but std::abort() still fires, so library misuse takes down the whole program with no chance to recover or report context.

Suggested fix: throw std::invalid_argument / std::logic_error with a descriptive message instead of aborting.


4. index_range::invalid is a mutable, externally writable global

Location: include/index_range.h:30, src/index_range.cc:25

static index_range invalid;   // non-const, public

Any code can mutate index_range::invalid, corrupting it for every other user, and it is not thread-safe to do so.

Suggested fix: make it const (e.g. static const index_range invalid;) and/or expose it via a function returning a fresh value.


5. (C++11/14 fallback only) consider runtime-checked accessors

operator[] on vector/set/map is assert-only (documented, mirrors std::vector). For users who want safety in release builds, an at() that throws std::out_of_range would be a useful, low-risk addition. Optional / nice-to-have.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions