2026-10-27 19:04:11 +01:00
# Safe C++ in detz: the playbook
detz is written in C++ and will stay that way. The language gives us speed and
control; it does not give us safety for free. This document is how we get both:
fast, object-oriented C++ that a hostile or buggy peer can waste our time with
but can never crash, corrupt, or silently mis-handle. It is a standing contract,
not a suggestion. **We do not accept regressions in it.**
It exists because of incident WA-2026-10-27: a single malformed WhatsApp frame
aborted the whole process with `std::vector larger than max_size()` . The
research behind this playbook is archived under
[`../../research/safe-cpp-practices/` ](../../research/safe-cpp-practices/ )
(OpenSSF compiler hardening, the C++ Core Guidelines bounds profile, integer
overflow practice, libFuzzer/sanitizer CI, the sanitizer overhead ladder,
clang-tidy ratchets). The full incident write-up is in
[whatsapp.md section 13 ](whatsapp.md ).
## The principle
A bug that a tool can catch must be caught by a tool, not by a careful reader.
Careful review found this class of bug twice and missed it both times; an
AddressSanitizer build found a second instance of it in seconds. So our safety
does not rest on discipline alone. It rests on four layers, cheapest first, each
backed by something that fails loudly when it is violated.
## Layer 1: design out the bug (zero runtime cost)
This is where most safety is won. The rules below are mandatory for any code
that touches bytes we did not produce (network frames, files, user input, IPC):
1. **Size arithmetic never overflows a bounds check.** Never write
`pos + len <= size` or `pos + len > size` : if `len` comes off the wire, the
sum can wrap `size_t` and the check passes on a garbage value. Always write it
as a subtraction against an established invariant: with `pos <= size` known,
check `len > size - pos` . For products use `a != 0 && b > SIZE_MAX / a` .
2. **A length is a claim, not a fact.** Validate every wire-supplied length,
count, and index against the bytes actually present (and against a static
application cap) *before* it drives an allocation, a `resize` /`reserve` , a
range constructor, or a slice. A caller must never be able to make us ask for
a four-gigabyte vector.
3. **Read lengths unsigned, reject negatives.** A 4-byte length read into a
signed `int` can sign-extend to negative and then become a huge `size_t` .
Read into an unsigned type wide enough to hold the value; reject negatives at
the boundary. Make every narrowing conversion explicit and checked, never an
implicit `int` ->`size_t` .
4. **Guard every shift and loop bound.** A varint decoder must stop before it
shifts by >= 64 bits (that is undefined behavior); a repeat count must be
bounded. This is the bug the sanitizer caught after review missed it.
5. **No exception escapes a top-level loop.** Any loop that reads and dispatches
untrusted input wraps the per-item work in `try/catch (const std::exception&)`
and turns a failure into a logged, skipped item. An uncaught `std::exception`
in a parser or dispatcher is a bug even when the throw "cannot happen".
6. **Parse at the boundary into a checked reader, then work on trusted types.**
Concentrate all the raw pointer/length/offset math in one small, reviewed,
bounds-checked reader (detz's `WaBinary` decoder and `ProtoReader` are this).
Above it, code works on decoded structs and `std::string` /`std::vector` , never
on raw offsets. This is the std::span / "parse don't validate" discipline;
note that `std::span::operator[]` and `subspan` do **not** bounds-check in
C++20, so the reader validates, not the view.
2026-10-27 19:25:50 +01:00
7. **Never take iterators from a temporary you rebuild.** `Bytes((c ? *o : "").begin(), (c ? *o : "").end())` evaluates the expression twice, so `begin()` and `end()` come from two different, already-destroyed temporaries: undefined behavior, and the garbage length aborted the paired-device save. Bind the value to a named variable first, then take its iterators. More generally, a range `[a, b)` must come from one live object. This is the object-lifetime class, distinct from size overflow; both are caught by the sanitizer net only if a test exercises the populated path.
8. **RAII and the rule of zero.** Resources are owned by objects that release
2026-10-27 19:04:11 +01:00
them; types that manage no raw resource declare no special members. A bug
that cannot exist needs no test. This is also our object-orientation: small
classes with invariants established in the constructor and preserved by every
method, not manual acquire/release.
## Layer 2: release hardening (cheap, always on)
The shipping binary keeps the Layer 1 checks plus the OpenSSF "fast" hardening
set, which is constant-time and meant for production:
- `_GLIBCXX_ASSERTIONS` (libstdc++ precondition checks: container bounds, valid
iterators) so a residual bad index is a defined, located abort, not undefined
behavior that corrupts silently.
- `_FORTIFY_SOURCE=3` , `-fstack-protector-strong` , `-fstack-clash-protection` ,
RELRO, PIE, noexecstack.
These are cheap enough that Linux distributions ship them by default. GCC's
`-fhardened` (and Clang's, as it lands) bundles the whole set behind one flag,
and is the intended future simplification once the toolchain baseline supports
it. Hardening catches memory and fortifiable bugs; it does not replace Layer 1.
## Layer 3: the sanitizer test net (free at dev time, run in CI)
Sanitizers are too expensive for release (AddressSanitizer is ~2-3x, ThreadSan
~5-15x; UBSan ~1.25x), so they are the **test** configuration, not the shipping
one. Every parser of untrusted input has:
- a **hostile-input test** : the exact malformed shapes that have bitten us or
could (oversized lengths, truncations, overlong varints), asserting each
throws a *handled* exception and specifically never `std::length_error` or a
crash; plus a **random fuzz** of thousands of inputs where surviving the loop
is the proof.
- a **sanitizer build** of that test under
`-fsanitize=address,undefined -fno-sanitize-recover=all -D_GLIBCXX_ASSERTIONS` ,
which aborts at the exact line of any over-read, over-allocation, or UB.
For detz::wa this is `tests/whatsapp_test.cpp` (`test_hostile` ) plus
[`../tools/san-whatsapp.sh` ](../tools/san-whatsapp.sh ). Run the sanitizer script
in CI and before any change to a parser. A new parser is **not finished** until
it has both. Vendored third-party C with intentional, portable UB idioms (the
ed25519 ref10 signed shifts) is excluded from UBSan explicitly and only there.
## Layer 4: capture, so an unreproducible bug is a one-shot fix
A wire bug that only a specific peer or timing triggers cannot be fixed by
staring at code. So a frame that breaks a parser has its exact bytes recorded
(bounded) to a capture file, and the dispatcher logs the node shape. The next
occurrence yields the precise input, which goes straight into the hostile-input
corpus as a permanent regression test. This is the "keep a corpus of crashing
inputs" rule, scaled to a single-process client.
## What we deliberately did not do
- **Adopt GSL** (`gsl::span` , `gsl::narrow` , `gsl::not_null` ) wholesale: it is a
new dependency, and CLAUDE.md favors fewer moving parts. `std::span` -style
checked views plus a small reviewed reader cover the need. We still follow the
Core Guidelines *rules* (span over pointer+length, explicit checked narrowing).
- **Run sanitizers in release**: too costly; they are the net, not the product.
- **Rewrite the wire layer in another language**: the goal is safe C++, and the
crypto/wire core is proven byte-for-byte against golden vectors.
## The next ratchet (named, not yet wired)
A `clang-tidy` gate (`bugprone-*` , selected `cppcoreguidelines-*` , especially
`bugprone-narrowing-conversions` ) would flag an implicit `int` ->`size_t`
narrowing at author time, before a sanitizer run. The industry pattern is a
ratchet: advisory on changed files first, then warnings-as-errors, enabling one
parked check at a time with its fixes. This is the planned next layer; it is
recorded here so it is not forgotten, and so nobody believes the current state
is the finished state.
## The one-line rule for a reviewer
If a line computes a size or index from data we did not produce, it is wrong
unless the check is a subtraction against a known bound, the value is unsigned
and range-checked before use, and a thrown exception upstream is caught. If a
parser has no hostile-input test and no sanitizer run, it is unfinished.