From f16c5c6ede21a360a31be2493c0fcb3b1df53400 Mon Sep 17 00:00:00 2001 From: retoor Date: Tue, 27 Oct 2026 18:32:49 +0100 Subject: [PATCH] whatsapp: fix crash on malformed node (length_error abort) A protobuf/binary length field could be read as a huge value, and the `pos + len > size` bounds check overflowed size_t and slipped past, so vector::assign threw std::length_error. Node dispatch (pairing, etc.) was not wrapped in a catch, so this aborted the whole process (core dump) during --whatsapp-link after the QR refreshed. - ProtoReader::next and the binary Decoder now use overflow-safe length checks (len > remaining) and the decoder reads 4-byte lengths unsigned and rejects anything larger than the frame; read_bytes rejects negatives. - WaClient::dispatch wraps all node routing in try/catch: a bad node is logged and skipped instead of crashing the client. Verified: build/whatsapp_test 16/16, make lint clean, and a live --whatsapp-link run survives the QR-refresh window with no abort. Co-Authored-By: Claude Opus 4.8 --- taicpp/src/whatsapp/wa_binary.cpp | 15 ++++++++++----- taicpp/src/whatsapp/wa_client.cpp | 9 +++++++++ taicpp/src/whatsapp/wa_client.h | 3 ++- taicpp/src/whatsapp/wa_proto.cpp | 4 +++- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/taicpp/src/whatsapp/wa_binary.cpp b/taicpp/src/whatsapp/wa_binary.cpp index 0f4c618..ed8a3bd 100644 --- a/taicpp/src/whatsapp/wa_binary.cpp +++ b/taicpp/src/whatsapp/wa_binary.cpp @@ -55,24 +55,29 @@ class Decoder { size_t pos_{0}; void need(size_t n) { - if (pos_ + n > buf_.size()) throw WireError("end of stream"); + // overflow-safe: pos_ <= buf_.size() always holds here. + if (n > buf_.size() - pos_) throw WireError("end of stream"); } uint8_t read_byte() { need(1); return buf_[pos_++]; } Bytes read_bytes(int n) { - need(n); + if (n < 0) throw WireError("negative length"); + need(static_cast(n)); Bytes b(buf_.begin() + pos_, buf_.begin() + pos_ + n); pos_ += n; return b; } int read_int(int n) { - need(n); - int v = 0; + need(static_cast(n)); + // accumulate unsigned so a 4-byte length with the high bit set does not become + // a negative int that later underflows a size; reject anything a node cannot hold. + uint32_t v = 0; for (int i = 0; i < n; ++i) v = (v << 8) | buf_[pos_ + i]; pos_ += n; - return v; + if (v > static_cast(buf_.size())) throw WireError("length exceeds frame"); + return static_cast(v); } int read_int20() { need(3); diff --git a/taicpp/src/whatsapp/wa_client.cpp b/taicpp/src/whatsapp/wa_client.cpp index 4828bd8..925be8a 100644 --- a/taicpp/src/whatsapp/wa_client.cpp +++ b/taicpp/src/whatsapp/wa_client.cpp @@ -119,6 +119,15 @@ void WaClient::send_ack(const WaNode& node) { } void WaClient::dispatch(const WaNode& node) { + // A malformed or unexpected node must never abort the process: log and move on. + try { + dispatch_inner(node); + } catch (const std::exception& e) { + log(std::string("dropped node <") + node.tag + ">: " + e.what()); + } +} + +void WaClient::dispatch_inner(const WaNode& node) { if (node.tag == "iq") { if (node.child("pair-device")) { handle_pair_device(node); diff --git a/taicpp/src/whatsapp/wa_client.h b/taicpp/src/whatsapp/wa_client.h index 39b387c..b338283 100644 --- a/taicpp/src/whatsapp/wa_client.h +++ b/taicpp/src/whatsapp/wa_client.h @@ -82,7 +82,8 @@ class WaClient { // send a binary node (framed + noise transport encrypted) void send_node(const WaNode& node); std::optional recv_node(int timeout_seconds); - void dispatch(const WaNode& node); + void dispatch(const WaNode& node); // exception-safe wrapper + void dispatch_inner(const WaNode& node); // the actual routing void handle_pair_device(const WaNode& iq); void handle_pair_success(const WaNode& iq); void send_ack(const WaNode& node); diff --git a/taicpp/src/whatsapp/wa_proto.cpp b/taicpp/src/whatsapp/wa_proto.cpp index 20d513e..ee1c86b 100644 --- a/taicpp/src/whatsapp/wa_proto.cpp +++ b/taicpp/src/whatsapp/wa_proto.cpp @@ -76,8 +76,10 @@ bool ProtoReader::next(Field& out) { len |= static_cast(b & 0x7f) << s; if (!(b & 0x80)) break; s += 7; + if (s > 63) throw std::runtime_error("wa proto: len varint too long"); } - if (pos_ + len > buf_.size()) throw std::runtime_error("wa proto: truncated bytes"); + // overflow-safe bounds check: pos_ <= buf_.size() here, so this cannot wrap. + if (len > buf_.size() - pos_) throw std::runtime_error("wa proto: truncated bytes"); out.bytes.assign(buf_.begin() + pos_, buf_.begin() + pos_ + len); pos_ += len; return true;