From c7f1abe258a5faae0482392ac7f1943dcbb81943 Mon Sep 17 00:00:00 2001 From: Holger Adams Date: Fri, 21 Aug 2026 20:24:03 +0200 Subject: [PATCH 1/2] Dispatcher: always verify len before pulling out bytes from a frame Every received frame is first turned into a 'Packet' struct by Dispatcher::tryParsePacket(). The frame is a length-prefixed byte string: [header][transport_codes(optional)][path_length][path][payload] The parser has to read the 'header' *first*, and only then does it know how many more bytes it is required to read. The bug is that it never checks that those bytes actually arrived. --- src/Dispatcher.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Dispatcher.cpp b/src/Dispatcher.cpp index c0610b7f8a..e6e7fd344c 100644 --- a/src/Dispatcher.cpp +++ b/src/Dispatcher.cpp @@ -147,6 +147,7 @@ void Dispatcher::loop() { } bool Dispatcher::tryParsePacket(Packet* pkt, const uint8_t* raw, int len) { + if (len < 1) return false; // bad encoding int i = 0; pkt->header = raw[i++]; @@ -156,12 +157,14 @@ bool Dispatcher::tryParsePacket(Packet* pkt, const uint8_t* raw, int len) { } if (pkt->hasTransportCodes()) { + if (i + 4 > len) return false; // bad encoding memcpy(&pkt->transport_codes[0], &raw[i], 2); i += 2; memcpy(&pkt->transport_codes[1], &raw[i], 2); i += 2; } else { pkt->transport_codes[0] = pkt->transport_codes[1] = 0; } + if (i + 1 > len) return false; // bad encoding pkt->path_len = raw[i++]; uint8_t path_mode = pkt->path_len >> 6; // upper 2 bits (legacy firmware: 00) if (path_mode == 3) { // Reserved for future From 05da523ebd32980a1c28b11f2928d351796b9737 Mon Sep 17 00:00:00 2001 From: Holger Adams Date: Fri, 21 Aug 2026 20:24:48 +0200 Subject: [PATCH 2/2] Packet: always verify len before pulling out bytes from a frame Packet::readFrom() is a second, independent parser for the same wire format: [header][transport_codes(optional)][path_length][path][payload] It is used by the ESP-NOW bridge (src/helpers/bridges/ESPNowBridge.cpp) to turn a received blob back into a 'Packet'. It duplicates the parsing logic of Dispatcher::tryParsePacket() and has the same length-check problems, plus one more: the `path` read is not bounds-checked either. --- src/Packet.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Packet.cpp b/src/Packet.cpp index aad3e2f48e..9b54a5bfc8 100644 --- a/src/Packet.cpp +++ b/src/Packet.cpp @@ -63,18 +63,25 @@ uint8_t Packet::writeTo(uint8_t dest[]) const { } bool Packet::readFrom(const uint8_t src[], uint8_t len) { + if (len < 1) return false; // bad encoding + uint8_t i = 0; + header = src[i++]; if (hasTransportCodes()) { + if (i + 4 > len) return false; // bad encoding memcpy(&transport_codes[0], &src[i], 2); i += 2; memcpy(&transport_codes[1], &src[i], 2); i += 2; } else { transport_codes[0] = transport_codes[1] = 0; } + + if (i + 1 > len) return false; // bad encoding path_len = src[i++]; if (!isValidPathLen(path_len)) return false; // bad encoding uint8_t bl = getPathByteLen(); + if (i + bl > len) return false; // bad encoding memcpy(path, &src[i], bl); i += bl; if (i >= len) return false; // bad encoding