diff --git a/taicpp/docs/whatsapp.md b/taicpp/docs/whatsapp.md index 3fcbcfd..37dd69f 100644 --- a/taicpp/docs/whatsapp.md +++ b/taicpp/docs/whatsapp.md @@ -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 : `) 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. diff --git a/taicpp/tests/whatsapp_test.cpp b/taicpp/tests/whatsapp_test.cpp index fbc2306..2086196 100644 --- a/taicpp/tests/whatsapp_test.cpp +++ b/taicpp/tests/whatsapp_test.cpp @@ -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{{}, {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; }