From 090e12fdb4709e583afd102ec63403de2244535c Mon Sep 17 00:00:00 2001 From: dahuib Date: Fri, 13 Nov 2026 20:45:06 +0100 Subject: [PATCH] Validate packet and response payload bounds on receive Radio frames can arrive with short or zero-length payloads in pooled Packet buffers that retain stale bytes, and AES-decrypted payloads are only block-aligned rather than guaranteed to hold variable-length inner fields: - Require a full 9-byte TRACE header before subtracting the header size (preventing uint8_t len underflow) and ensure a complete hop hash remains before matching self_id. - Require payload_len > 0 before reading pkt->payload[0] on direct CONTROL frames so pooled buffer remnants do not trigger empty control callbacks. - Reject PAYLOAD_TYPE_PATH frames whose encoded path plus extra_type byte exceed the decrypted payload length before uint8_t extra_len underflows and passes stack memory into onPeerPathRecv(). - Bound MyMesh::onContactResponse() against short piggybacked PATH responses (preserving 8-12 byte legacy login responses by defaulting missing FIRMWARE_VER_LEVEL to 0) and clamp len to MAX_FRAME_SIZE - 4 so max-size responses neither overrun out_frame[] nor exceed writeFrame()'s frame limit. --- examples/companion_radio/MyMesh.cpp | 10 +++++++--- src/Mesh.cpp | 14 ++++++++++---- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/examples/companion_radio/MyMesh.cpp b/examples/companion_radio/MyMesh.cpp index 1a1bcbe767a..9c3c2d9957f 100644 --- a/examples/companion_radio/MyMesh.cpp +++ b/examples/companion_radio/MyMesh.cpp @@ -688,6 +688,10 @@ uint8_t MyMesh::onContactRequest(const ContactInfo &contact, uint32_t sender_tim } void MyMesh::onContactResponse(const ContactInfo &contact, const uint8_t *data, uint8_t len) { + if (len < 4) return; + if (len > MAX_FRAME_SIZE - 4) { // largest frame below writes 8-byte header + (len - 4) bytes + len = MAX_FRAME_SIZE - 4; + } uint32_t tag; memcpy(&tag, data, 4); @@ -696,12 +700,12 @@ void MyMesh::onContactResponse(const ContactInfo &contact, const uint8_t *data, pending_login = 0; int i = 0; - if (memcmp(&data[4], "OK", 2) == 0) { // legacy Repeater login OK response + if (len >= 6 && memcmp(&data[4], "OK", 2) == 0) { // legacy Repeater login OK response out_frame[i++] = PUSH_CODE_LOGIN_SUCCESS; out_frame[i++] = 0; // legacy: is_admin = false memcpy(&out_frame[i], contact.id.pub_key, 6); i += 6; // pub_key_prefix - } else if (data[4] == RESP_SERVER_LOGIN_OK) { // new login response + } else if (len >= 8 && data[4] == RESP_SERVER_LOGIN_OK) { // new login response uint16_t keep_alive_secs = ((uint16_t)data[5]) * 16; if (keep_alive_secs > 0) { startConnection(contact, keep_alive_secs); @@ -713,7 +717,7 @@ void MyMesh::onContactResponse(const ContactInfo &contact, const uint8_t *data, memcpy(&out_frame[i], &tag, 4); i += 4; // NEW: include server timestamp out_frame[i++] = data[7]; // NEW (v7): ACL permissions - out_frame[i++] = data[12]; // FIRMWARE_VER_LEVEL + out_frame[i++] = (len >= 13) ? data[12] : 0; // FIRMWARE_VER_LEVEL } else { out_frame[i++] = PUSH_CODE_LOGIN_FAIL; out_frame[i++] = 0; // reserved diff --git a/src/Mesh.cpp b/src/Mesh.cpp index 9dc42f0b04b..a1f29ed4cf7 100644 --- a/src/Mesh.cpp +++ b/src/Mesh.cpp @@ -40,7 +40,7 @@ int Mesh::searchChannelsByHash(const uint8_t* hash, GroupChannel channels[], int DispatcherAction Mesh::onRecvPacket(Packet* pkt) { if (pkt->isRouteDirect() && pkt->getPayloadType() == PAYLOAD_TYPE_TRACE) { - if (pkt->path_len < MAX_PATH_SIZE) { + if (pkt->path_len < MAX_PATH_SIZE && pkt->payload_len >= 9) { uint8_t i = 0; uint32_t trace_tag; memcpy(&trace_tag, &pkt->payload[i], 4); i += 4; @@ -48,6 +48,7 @@ DispatcherAction Mesh::onRecvPacket(Packet* pkt) { memcpy(&auth_code, &pkt->payload[i], 4); i += 4; uint8_t flags = pkt->payload[i++]; uint8_t path_sz = flags & 0x03; // NEW v1.11+: lower 2 bits is path hash size + uint8_t hash_sz = (uint8_t)(1 << path_sz); uint8_t len = pkt->payload_len - i; // path_len*entry_size can exceed 255 (path_len up to 63, entry_size up to 8); @@ -55,7 +56,7 @@ DispatcherAction Mesh::onRecvPacket(Packet* pkt) { uint16_t offset = (uint16_t)pkt->path_len << path_sz; if (offset >= len) { // TRACE has reached end of given path onTraceRecv(pkt, trace_tag, auth_code, flags, pkt->path, &pkt->payload[i], len); - } else if (self_id.isHashMatch(&pkt->payload[i + offset], 1 << path_sz) && allowPacketForward(pkt) && !_tables->wasSeen(pkt)) { + } else if (offset + hash_sz <= len && self_id.isHashMatch(&pkt->payload[i + offset], hash_sz) && allowPacketForward(pkt) && !_tables->wasSeen(pkt)) { _tables->markSeen(pkt); // append SNR (Not hash!) pkt->path[pkt->path_len++] = (int8_t) (pkt->getSNR()*4); @@ -67,7 +68,7 @@ DispatcherAction Mesh::onRecvPacket(Packet* pkt) { return ACTION_RELEASE; } - if (pkt->isRouteDirect() && pkt->getPayloadType() == PAYLOAD_TYPE_CONTROL && (pkt->payload[0] & 0x80) != 0) { + if (pkt->isRouteDirect() && pkt->getPayloadType() == PAYLOAD_TYPE_CONTROL && pkt->payload_len > 0 && (pkt->payload[0] & 0x80) != 0) { if (pkt->getPathHashCount() == 0) { onControlDataRecv(pkt); } @@ -166,7 +167,12 @@ DispatcherAction Mesh::onRecvPacket(Packet* pkt) { } uint8_t hash_size = (path_len >> 6) + 1; uint8_t hash_count = path_len & 63; - uint8_t* path = &data[k]; k += hash_size*hash_count; + uint8_t path_byte_len = hash_size * hash_count; + if (k + path_byte_len + 1 > len) { + MESH_DEBUG_PRINTLN("%s PAYLOAD_TYPE_PATH, incomplete payload: len=%d", getLogDateTime(), len); + break; // reject bad encoding + } + uint8_t* path = &data[k]; k += path_byte_len; uint8_t extra_type = data[k++] & 0x0F; // upper 4 bits reserved for future use uint8_t* extra = &data[k]; uint8_t extra_len = len - k; // remainder of packet (may be padded with zeroes!)