whatsapp: document the length_error crash and guard it with a hostile-input test

Record incident WA-2026-10-27 in docs/whatsapp.md section 13: the symptom (a
std::length_error core dump during --whatsapp-link after the QR refreshed), the
root cause (an overflowable `pos + len > size` bounds check plus an unguarded
dispatch loop), the fix (commit f16c5c6), and the standing rule for all code
that parses untrusted bytes: overflow-safe bounds checks, no unchecked
allocation from an attacker-controlled count, and no exception escaping a
top-level loop. The goal is C++ that a hostile peer can never crash.

Prevention is enforced, not just written: tests/whatsapp_test.cpp gains a
test_hostile section that feeds the parsers the exact shapes that crashed (a
4GB length field, an oversized protobuf varint, truncated frames, 2000 random
inputs) and fails if std::length_error is ever thrown again. Runs under
build/whatsapp_test / whatsapp.unit / make test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-10-27 18:36:14 +01:00
co-authored by Claude Opus 4.8
parent f16c5c6ede
commit 713f08dc31
2 changed files with 182 additions and 0 deletions
+105
View File
@@ -388,3 +388,108 @@ keys, signal and media total 5,231. The C++ size is not measured until it exists
battle tested. whatsmeow stays the referee.
- **Unverified here:** the full handshake (probe refused), XEdDSA verify via
OpenSSL Ed25519, the 20-day re-auth claim, the 405 behaviour.
## 13. Robustness against hostile input (incident WA-2026-10-27, and the standing rule)
Every byte this module parses comes off a network socket we do not control.
That makes the wire parsers (`WaBinary`, `WaProto`, and the Noise/Signal frame
readers) a security and stability boundary, not ordinary code. This section
records a crash that got through, exactly why, how it was fixed, and the rule
and test that keep it from ever coming back. **We do not accept regressions
here.**
### 13.1 What happened
Running `detz --whatsapp-link` on a real terminal, the wizard connected, showed
the QR, and after the code auto-refreshed it aborted:
```
terminate called after throwing an instance of 'std::length_error'
what(): cannot create std::vector larger than max_size()
Aborted (core dumped)
```
A core dump, not a handled error. The process died mid-session instead of
logging a bad frame and carrying on.
### 13.2 Root cause
Two independent faults lined up:
1. **An overflowable bounds check.** Both the protobuf reader and the binary-node
decoder guarded a length-delimited field with `pos + len > size`. `pos` is a
`size_t` and `len` was read straight from the wire (up to 2^32-1, or a
4-byte length sign-extended into a negative `int`). `pos + len` then wrapped
around past `SIZE_MAX`, computed a small sum, and the check passed. The code
went on to `vector::assign(begin, begin + len)` with a garbage `len`, and the
standard library threw `std::length_error` from inside `vector`.
2. **An unguarded top-level loop.** `WaClient::dispatch` routed nodes straight
into `handle_pair_success` and the other handlers with no `try/catch`. The
decode path in `recv_node` was wrapped, but the handlers that re-parse
sub-messages (the ADV identity in pairing, for instance) were not, so a
`std::length_error` raised there propagated out of `main` and called
`std::terminate`.
Either fault alone is survivable. Together they turn one malformed frame into a
process abort. This is the classic C++ failure: an arithmetic overflow in a
size computation silently defeats a bounds check, and an uncaught exception is
not a logged error but a dead process.
### 13.3 The fix (commit f16c5c6)
- **Overflow-safe length checks everywhere bytes are sliced.** The comparison is
now `len > size - pos` (never `pos + len > size`). Because `pos <= size` is an
invariant at every call site, the subtraction cannot wrap, so no attacker
value can slip through. `src/whatsapp/wa_proto.cpp` (`ProtoReader::next`) and
`src/whatsapp/wa_binary.cpp` (`Decoder::need`).
- **Lengths are read unsigned and range-checked.** The binary decoder reads
4-byte lengths into a `uint32_t` (never a sign-extendable `int`), rejects any
value larger than the whole frame, and `read_bytes` rejects negatives. A bad
length now raises `WireError`, a `std::exception` we catch, long before any
allocation.
- **Dispatch can no longer abort the process.** `WaClient::dispatch` wraps all
node routing in `try/catch(const std::exception&)`; a bad node is logged
(`dropped node <tag>: <reason>`) and the client keeps running.
### 13.4 The standing rule (project principle)
The goal is to make C++ safe to use here, which means a hostile or buggy peer
can waste our time but can never crash us or corrupt us. Three invariants, in
order of importance:
1. **No size arithmetic that can overflow a bounds check.** Never write
`pos + len <= size`. Write `len <= size - pos` against an established
`pos <= size`. Read lengths into an unsigned type wide enough to hold them,
and range-check before you allocate or slice. Treat every length, count and
index off the wire as adversarial.
2. **No unchecked allocation from an attacker-controlled count.** A length field
is a claim, not a fact. Validate it against the bytes actually present before
`reserve`, `resize`, `assign`, or a range constructor. A caller must never be
able to make us ask for a 4 GB vector.
3. **No exception may escape a top-level loop.** Any loop that reads and
dispatches untrusted input (`recv_node`, `dispatch`, the send and pairing
waits) 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
this module is a bug even if the parse that raised it was "impossible".
These apply to any code that reads untrusted bytes, not just WhatsApp. When a
new wire format is added (CAP, MCP, a new media type), it inherits this rule.
### 13.5 How we stop the regression
Prose does not prevent regressions; a failing test does. `tests/whatsapp_test.cpp`
has a `test_hostile` section (run by `build/whatsapp_test`, step `whatsapp.unit`
in `tests/run_all.py`, and `make test`) that feeds the parsers exactly the shapes
that used to crash and asserts they throw a *handled* exception and specifically
**not** `std::length_error`:
- a binary node whose content length claims 0xFFFFFFFF in an 11-byte buffer;
- a protobuf field whose length varint is ~4 billion with no payload;
- truncated and short frames;
- 2000 random byte strings through both `WaBinary` and `ProtoReader` (surviving
the loop is the proof: a single unchecked allocation would abort the run).
Each case `catch`es `std::length_error` and fails the test if it is thrown, so
if anyone reintroduces a `pos + len` check or an unguarded slice, this test goes
red. If you add a new parser, add its hostile cases here in the same pass; a
parser with no adversarial test is treated as unfinished.
+77
View File
@@ -245,6 +245,82 @@ static void test_signal() {
check(WaSession::deserialize(blob).has_value(), "session serialize/deserialize");
}
// ---- hostile / malformed input (regression guard, incident WA-2026-10-27) ----
// Every parser of bytes off the wire must reject malformed input by throwing a
// std::exception (caught upstream), never by an unchecked huge allocation that
// aborts the process. A length field that overflows the bounds check used to
// reach vector::assign and throw std::length_error, which aborted the client.
// These cases must keep throwing a *handled* exception, never std::length_error
// and never crash. See docs/whatsapp.md section 13.
static void test_hostile() {
std::printf("[hostile input]\n");
bool ok = true;
// 1. binary node whose content length claims 0xFFFFFFFF in a tiny buffer.
// flag(0) | LIST_8 size 2 | tag BINARY_8 "x" | content BINARY_32 0xFFFFFFFF
Bytes huge_len = {0x00, 0xf8, 0x02, 0xfc, 0x01, 'x', 0xfe, 0xff, 0xff, 0xff, 0xff};
try {
WaBinary::decode(WaBinary::unpack(huge_len));
ok = false; // must not succeed
} catch (const std::length_error&) {
ok = false; // the exact regression: an unchecked allocation
} catch (const std::exception&) {
// correct: a handled WireError / bounds error
}
check(ok, "binary decode rejects a 4GB length field without length_error");
// 2. protobuf field 1, wire 2, length varint ~4 billion, no payload.
Bytes huge_pb = {0x0a, 0xff, 0xff, 0xff, 0xff, 0x0f};
ok = true;
try {
ProtoReader r(huge_pb);
ProtoReader::Field f;
while (r.next(f)) {
}
ok = false;
} catch (const std::length_error&) {
ok = false;
} catch (const std::exception&) {
}
check(ok, "proto reader rejects an oversized length without length_error");
// 3. negative/short-buffer node tags and truncations throw cleanly.
ok = true;
for (const Bytes& bad : std::vector<Bytes>{{}, {0x00}, {0x00, 0xf8}, {0x00, 0xfe, 0x7f, 0xff, 0xff, 0xff},
{0x00, 0xf8, 0x04, 0xfc, 0xff}}) {
try {
WaBinary::decode(WaBinary::unpack(bad));
} catch (const std::length_error&) {
ok = false;
} catch (const std::exception&) {
}
}
check(ok, "truncated/short frames throw cleanly, never length_error");
// 4. fuzz: random byte strings must each return or throw a std::exception,
// never a length_error and never a crash (surviving the loop is the proof).
ok = true;
for (int i = 0; i < 2000; ++i) {
Bytes r = WaCrypto::random_bytes(1 + (i % 64));
try {
WaBinary::decode(WaBinary::unpack(r));
} catch (const std::length_error&) {
ok = false;
} catch (const std::exception&) {
}
try {
ProtoReader pr(r);
ProtoReader::Field f;
while (pr.next(f)) {
}
} catch (const std::length_error&) {
ok = false;
} catch (const std::exception&) {
}
}
check(ok, "2000 random inputs: no length_error, no crash");
}
int main(int argc, char** argv) {
DIR = argc > 1 ? argv[1] : "tests/whatsapp_data";
test_crypto();
@@ -252,6 +328,7 @@ int main(int argc, char** argv) {
test_client_payload();
test_noise();
test_signal();
test_hostile();
std::printf("%s\n", g_fail == 0 ? "whatsapp: ALL OK" : "whatsapp: FAILURES");
return g_fail == 0 ? 0 : 1;
}