diff --git a/zephcore/app/CompanionMesh.cpp b/zephcore/app/CompanionMesh.cpp index 2f37e7c..379370b 100644 --- a/zephcore/app/CompanionMesh.cpp +++ b/zephcore/app/CompanionMesh.cpp @@ -3031,22 +3031,42 @@ bool CompanionMesh::handleProtocolFrame(const uint8_t *data, size_t len) return true; case CMD_SEND_RAW_DATA: - /* Raw data packet: [cmd][path_len][path...][payload...] */ + /* Raw data packet: [cmd][path_len][path...][payload...] + * + * path_len is the encoded 2-bit (hash_size - 1) + 6-bit hop count, not + * a byte count, so it has to be decoded rather than added to an offset. + * Treating it as a length rejected every path with a hash size above 1: + * hash_size 2 sets bit 6 (path_len 0x42 for two hops read as 66 bytes, + * failing the length check) and hash_size 3 sets bit 7, which used to + * read back as a negative int8_t. */ if (len >= 6) { /* min: cmd + path_len + 4 byte payload */ - int i = 1; - int8_t path_len = (int8_t)data[i++]; - if (path_len >= 0 && i + path_len + 4 <= (int)len) { - const uint8_t *path = &data[i]; - i += path_len; - mesh::Packet *pkt = createRawData(&data[i], len - i); - if (pkt) { - sendDirect(pkt, path, path_len); - sendPacketOk(); + size_t i = 1; + uint8_t path_len = data[i++]; + if (mesh::Packet::isValidPathLen(path_len)) { + uint8_t path[MAX_PATH_SIZE]; + /* Untrusted phone frame: bound the source at the bytes that + * actually remain, so a crafted path_len cannot over-read. */ + size_t path_bytes = mesh::Packet::writePath(path, &data[i], len - i, path_len); + if (path_bytes == 0 && (path_len & 63) != 0) { + /* Zero bytes written with a non-zero hop count (low 6 + * bits): the decoded path runs past the end of the frame. */ + sendPacketError(ERR_ILLEGAL_ARG); + return true; + } + i += path_bytes; + if (i + 4 > len) { /* min payload 4 bytes */ + sendPacketError(ERR_ILLEGAL_ARG); } else { - sendPacketError(ERR_TABLE_FULL); + mesh::Packet *pkt = createRawData(&data[i], len - i); + if (pkt) { + sendDirect(pkt, path, path_len); + sendPacketOk(); + } else { + sendPacketError(ERR_TABLE_FULL); + } } } else { - /* Flood mode not supported (path_len < 0) */ + /* Flood mode not supported */ sendPacketError(ERR_UNSUPPORTED); } } else { diff --git a/zephcore/helpers/BaseChatMesh.cpp b/zephcore/helpers/BaseChatMesh.cpp index 8b4fb26..62bb95b 100644 --- a/zephcore/helpers/BaseChatMesh.cpp +++ b/zephcore/helpers/BaseChatMesh.cpp @@ -405,10 +405,37 @@ void BaseChatMesh::handleReturnPathRetry(const ContactInfo &contact, const uint8 if (rpath) sendDirect(rpath, contact.out_path, contact.out_path_len, 3000); } +/* An unconfigured channel slot has an all-zero secret. */ +static bool isChannelSlotEmpty(const ChannelDetails &ch) +{ + static const uint8_t zeroes32[32] = { 0 }; + return memcmp(ch.channel.secret, zeroes32, sizeof(ch.channel.secret)) == 0; +} + int BaseChatMesh::searchChannelsByHash(const uint8_t *hash, mesh::GroupChannel dest[], int max_matches) { int n = 0; for (int i = 0; i < MAX_GROUP_CHANNELS && n < max_matches; i++) { + /* Skip unconfigured slots. The zero-key MAC validates against an + * all-zero secret, so null-key group traffic (a sender with an unset + * PSK) would be decrypted and delivered as if it belonged to that + * slot — every node with a free slot is a null-key sink. + * + * The test is the secret, not name[0] as upstream uses: CMD_SET_CHANNEL + * copies whatever 32-byte name the app supplies, so a configured + * channel may carry an empty name and a cleared slot may keep a stale + * one. The zero secret is the exact condition that makes a slot + * dangerous. + * + * A memset slot's hash is 0x00, which does not collide with the + * null-key hash — but saveChannels() persists all MAX_GROUP_CHANNELS + * slots and onChannelLoaded() runs setChannel() on each, recomputing + * an empty slot's hash from its zero secret. After the first save + * plus reboot every free slot carries the live null-key hash. + * + * Skipping them also keeps empty slots from consuming dest[] entries + * and crowding out a real channel whose hash byte happens to match. */ + if (isChannelSlotEmpty(channels[i])) continue; if (channels[i].channel.hash[0] == hash[0]) { dest[n++] = channels[i].channel; }