From 56b5f4e9c68ae668662f93044b0c2a62d2ccf10b Mon Sep 17 00:00:00 2001 From: Marcel Verdult Date: Thu, 22 Oct 2026 09:33:07 +0000 Subject: [PATCH] fix(chat): refuse a negative text length for a group message CMD_SEND_CHANNEL_TXT_MSG reads txt_type, channel_idx and a 4 byte timestamp, so the frame needs 7 bytes before the text starts, and the branch had no length guard. For a shorter frame "len - i" is computed in size_t, wraps, and reaches sendGroupMessage() as a negative int. The clamp there tests "text_len + prefix_len > MAX_TEXT_LEN", which a negative value passes, and the following memcpy() converts the length back to a size_t: a 3 byte frame asks for 4294967292 bytes into a 197 byte stack buffer. Two guards, because there are two mistakes. The branch now states its minimum like most other branches of handleCmdFrame do, so a frame below 7 bytes is answered through the existing path instead of being parsed. And sendGroupMessage() refuses a negative length the way sendGroupData() in the same file already does, which protects every other caller. A frame of exactly 7 bytes keeps working and sends an empty message, as before. Evidence. On a T-Echo running dev 428a95c6, built with FIRMWARE_BUILD_DATE "f1-dev" and confirmed by "ver => v1.18.0 (Build: f1-dev)": CH_TXT len=7 -> RESP_CODE_OK GET_STATS core -> uptime=78s CH_TXT len=3 -> NO REPLY DEVICE_QUERY -> NO REPLY (three times over 8 s, node never came back) --- examples/companion_radio/MyMesh.cpp | 4 +++- src/helpers/BaseChatMesh.cpp | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/examples/companion_radio/MyMesh.cpp b/examples/companion_radio/MyMesh.cpp index 99a9aa30d6e..d74050be7e0 100644 --- a/examples/companion_radio/MyMesh.cpp +++ b/examples/companion_radio/MyMesh.cpp @@ -1175,7 +1175,9 @@ void MyMesh::handleCmdFrame(size_t len) { ? ERR_CODE_NOT_FOUND : ERR_CODE_UNSUPPORTED_CMD); // unknown recipient, or unsupported TXT_TYPE_* } - } else if (cmd_frame[0] == CMD_SEND_CHANNEL_TXT_MSG) { // send GroupChannel text msg + } else if (cmd_frame[0] == CMD_SEND_CHANNEL_TXT_MSG && len >= 7) { // send GroupChannel text msg + // 7 = code, txt_type, channel_idx and the 4 byte timestamp; the text may be empty. + // Without this the text length below is computed as len - 7 and turns negative. int i = 1; uint8_t txt_type = cmd_frame[i++]; // should be TXT_TYPE_PLAIN uint8_t channel_idx = cmd_frame[i++]; diff --git a/src/helpers/BaseChatMesh.cpp b/src/helpers/BaseChatMesh.cpp index 28592415ca9..2637bc327ab 100644 --- a/src/helpers/BaseChatMesh.cpp +++ b/src/helpers/BaseChatMesh.cpp @@ -516,6 +516,11 @@ int BaseChatMesh::sendCommandData(const ContactInfo& recipient, uint32_t timest } bool BaseChatMesh::sendGroupMessage(uint32_t timestamp, mesh::GroupChannel& channel, const char* sender_name, const char* text, int text_len) { + if (text_len < 0) { // memcpy() below would read this as a size_t, so clamping cannot catch it + MESH_DEBUG_PRINTLN("sendGroupMessage: invalid negative text_len=%d", text_len); + return false; + } + uint8_t temp[5+MAX_TEXT_LEN+32]; memcpy(temp, ×tamp, 4); // mostly an extra blob to help make packet_hash unique temp[4] = 0; // TXT_TYPE_PLAIN