Invalid WriteRange inputs (inverted begin/end or negative lengths) crash instead of being rejected #56

Closed
opened 2026-06-28 22:57:25 +00:00 by weaselbot · 1 comment
Member

The radix-tree implementation trusts callers to supply well-formed ConflictSet_Key / ConflictSet_WriteRange values and does not validate them at the public entry points. Passing an inverted range (begin > end) or a negative key length causes an out-of-bounds read or a segmentation fault, even in debug builds.

Locations

  • ConflictSet.cpp around lines 5045-5055: ConflictSet::addWrites converts w.begin / w.end into TrivialSpans without checking len >= 0 or (for ranges) begin < end.
  • ConflictSet.cpp around lines 2991-3008: addWriteRange assumes the caller has already guaranteed begin < end.
  • ConflictSet.cpp around lines 2971-2984: eraseInRange walks the tree with nextLogical(beginNode) until it reaches endNode; when beginNode is after endNode it walks off the right edge of the tree and dereferences a null pointer.
  • Internal.h around lines 36-44: TrivialSpan stores int len and uses it directly in operator[] and in memcmp via std::min<int>(...). A negative length is implicitly converted to a huge size_t in memcmp, causing an out-of-bounds read in release builds (in debug builds the operator[] assert fires first).

Reproductions

Inverted range write

#include "ConflictSet.h"
int main(void) {
    ConflictSet *cs = ConflictSet_create(0);
    ConflictSet_WriteRange w;
    uint8_t a = 'b', b = 'a';
    w.begin.p = &a; w.begin.len = 1;
    w.end.p   = &b; w.end.len   = 1;  // begin > end
    ConflictSet_addWrites(cs, &w, 1, 1);
    ConflictSet_destroy(cs);
    return 0;
}

Built against radix_tree/libconflict-set.so this crashes with SIGSEGV during eraseInRange.

Negative key length

#include "ConflictSet.h"
#include <stdio.h>
int main(void) {
    ConflictSet *cs = ConflictSet_create(0);
    ConflictSet_ReadRange r;
    uint8_t k = 'x';
    r.begin.p = &k;
    r.begin.len = -1;  // invalid negative length
    r.end.len = 0;
    r.readVersion = 0;
    ConflictSet_Result res;
    ConflictSet_check(cs, &r, &res, 1);
    printf("result=%d\n", res);
    ConflictSet_destroy(cs);
    return 0;
}

In a debug build this aborts at TrivialSpan::operator[] (Internal.h:44). In a release build (-DNDEBUG) the negative length flows into memcmp and produces a massive size_t, resulting in an out-of-bounds read.

Expected vs actual

  • Expected: The library detects malformed inputs at the API boundary and either returns an error or, consistent with the rest of the codebase, abort()s with a clear precondition assertion.
  • Actual: The malformed length/range is propagated through TrivialSpan and the radix-tree insertion path, eventually causing a segfault or out-of-bounds memory access.

Impact

Any language binding or C caller that accidentally produces an inverted range or a negative length (e.g. a bug in a wrapper, a signed/unsigned conversion mistake, or fuzzing) can crash the process or read memory past the supplied buffer. This affects both the C and C++ public APIs and any scripting language wrappers built on top of them.

Suggested fix

Add explicit validation at the start of ConflictSet::addWrites (and ideally ConflictSet::check) before constructing TrivialSpans:

assert(w.begin.len >= 0);
assert(w.end.len >= 0);
if (w.end.len > 0) {
    assert(TrivialSpan(w.begin.p, w.begin.len) <
           TrivialSpan(w.end.p, w.end.len));
}

For a non-asserting API, returning an error or treating the inputs as an empty batch would also prevent the crash, but an assertion matches the existing style used for version monotonicity and other invariants.

The radix-tree implementation trusts callers to supply well-formed `ConflictSet_Key` / `ConflictSet_WriteRange` values and does not validate them at the public entry points. Passing an inverted range (`begin > end`) or a negative key length causes an out-of-bounds read or a segmentation fault, even in debug builds. ## Locations - `ConflictSet.cpp` around lines 5045-5055: `ConflictSet::addWrites` converts `w.begin` / `w.end` into `TrivialSpan`s without checking `len >= 0` or (for ranges) `begin < end`. - `ConflictSet.cpp` around lines 2991-3008: `addWriteRange` assumes the caller has already guaranteed `begin < end`. - `ConflictSet.cpp` around lines 2971-2984: `eraseInRange` walks the tree with `nextLogical(beginNode)` until it reaches `endNode`; when `beginNode` is after `endNode` it walks off the right edge of the tree and dereferences a null pointer. - `Internal.h` around lines 36-44: `TrivialSpan` stores `int len` and uses it directly in `operator[]` and in `memcmp` via `std::min<int>(...)`. A negative length is implicitly converted to a huge `size_t` in `memcmp`, causing an out-of-bounds read in release builds (in debug builds the `operator[]` assert fires first). ## Reproductions ### Inverted range write ```c #include "ConflictSet.h" int main(void) { ConflictSet *cs = ConflictSet_create(0); ConflictSet_WriteRange w; uint8_t a = 'b', b = 'a'; w.begin.p = &a; w.begin.len = 1; w.end.p = &b; w.end.len = 1; // begin > end ConflictSet_addWrites(cs, &w, 1, 1); ConflictSet_destroy(cs); return 0; } ``` Built against `radix_tree/libconflict-set.so` this crashes with `SIGSEGV` during `eraseInRange`. ### Negative key length ```c #include "ConflictSet.h" #include <stdio.h> int main(void) { ConflictSet *cs = ConflictSet_create(0); ConflictSet_ReadRange r; uint8_t k = 'x'; r.begin.p = &k; r.begin.len = -1; // invalid negative length r.end.len = 0; r.readVersion = 0; ConflictSet_Result res; ConflictSet_check(cs, &r, &res, 1); printf("result=%d\n", res); ConflictSet_destroy(cs); return 0; } ``` In a debug build this aborts at `TrivialSpan::operator[]` (`Internal.h:44`). In a release build (`-DNDEBUG`) the negative length flows into `memcmp` and produces a massive `size_t`, resulting in an out-of-bounds read. ## Expected vs actual - **Expected:** The library detects malformed inputs at the API boundary and either returns an error or, consistent with the rest of the codebase, `abort()`s with a clear precondition assertion. - **Actual:** The malformed length/range is propagated through `TrivialSpan` and the radix-tree insertion path, eventually causing a segfault or out-of-bounds memory access. ## Impact Any language binding or C caller that accidentally produces an inverted range or a negative length (e.g. a bug in a wrapper, a signed/unsigned conversion mistake, or fuzzing) can crash the process or read memory past the supplied buffer. This affects both the C and C++ public APIs and any scripting language wrappers built on top of them. ## Suggested fix Add explicit validation at the start of `ConflictSet::addWrites` (and ideally `ConflictSet::check`) before constructing `TrivialSpan`s: ```cpp assert(w.begin.len >= 0); assert(w.end.len >= 0); if (w.end.len > 0) { assert(TrivialSpan(w.begin.p, w.begin.len) < TrivialSpan(w.end.p, w.end.len)); } ``` For a non-asserting API, returning an error or treating the inputs as an empty batch would also prevent the crash, but an assertion matches the existing style used for version monotonicity and other invariants.
Owner

An input that violates a precondition does not constitute a bug

An input that violates a precondition does not constitute a bug
Sign in to join this conversation.
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: weaselab/conflict-set#56