From 1756eaeaa91ab4bcbc304097e193ead06b4e2f97 Mon Sep 17 00:00:00 2001 From: Andrew Jackura Date: Fri, 1 May 2026 23:47:27 -0700 Subject: [PATCH] Fix buffer over-read in HMAC generation for 16-byte group channel keys --- src/Mesh.cpp | 14 +++++++------- src/Utils.cpp | 12 ++++++------ src/Utils.h | 4 ++-- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/Mesh.cpp b/src/Mesh.cpp index 57fee14036..47055e26cf 100644 --- a/src/Mesh.cpp +++ b/src/Mesh.cpp @@ -148,7 +148,7 @@ DispatcherAction Mesh::onRecvPacket(Packet* pkt) { // decrypt, checking MAC is valid uint8_t data[MAX_PACKET_PAYLOAD]; - int len = Utils::MACThenDecrypt(secret, data, macAndData, pkt->payload_len - i); + int len = Utils::MACThenDecrypt(secret, PUB_KEY_SIZE, data, macAndData, pkt->payload_len - i); if (len > 0) { // success! if (pkt->getPayloadType() == PAYLOAD_TYPE_PATH) { int k = 0; @@ -200,7 +200,7 @@ DispatcherAction Mesh::onRecvPacket(Packet* pkt) { // decrypt, checking MAC is valid uint8_t data[MAX_PACKET_PAYLOAD]; - int len = Utils::MACThenDecrypt(secret, data, macAndData, pkt->payload_len - i); + int len = Utils::MACThenDecrypt(secret, PUB_KEY_SIZE, data, macAndData, pkt->payload_len - i); if (len > 0) { // success! onAnonDataRecv(pkt, secret, sender, data, len); pkt->markDoNotRetransmit(); @@ -226,7 +226,7 @@ DispatcherAction Mesh::onRecvPacket(Packet* pkt) { for (int j = 0; j < num; j++) { // decrypt, checking MAC is valid uint8_t data[MAX_PACKET_PAYLOAD]; - int len = Utils::MACThenDecrypt(channels[j].secret, data, macAndData, pkt->payload_len - i); + int len = Utils::MACThenDecrypt(channels[j].secret, 16, data, macAndData, pkt->payload_len - i); if (len > 0) { // success! onGroupDataRecv(pkt, pkt->getPayloadType(), channels[j], data, len); break; @@ -463,7 +463,7 @@ Packet* Mesh::createPathReturn(const uint8_t* dest_hash, const uint8_t* secret, getRNG()->random(&data[data_len], 4); data_len += 4; } - len += Utils::encryptThenMAC(secret, &packet->payload[len], data, data_len); + len += Utils::encryptThenMAC(secret, PUB_KEY_SIZE, &packet->payload[len], data, data_len); } packet->payload_len = len; @@ -488,7 +488,7 @@ Packet* Mesh::createDatagram(uint8_t type, const Identity& dest, const uint8_t* int len = 0; len += dest.copyHashTo(&packet->payload[len]); // dest hash len += self_id.copyHashTo(&packet->payload[len]); // src hash - len += Utils::encryptThenMAC(secret, &packet->payload[len], data, data_len); + len += Utils::encryptThenMAC(secret, PUB_KEY_SIZE, &packet->payload[len], data, data_len); packet->payload_len = len; @@ -516,7 +516,7 @@ Packet* Mesh::createAnonDatagram(uint8_t type, const LocalIdentity& sender, cons } else { // FUTURE: } - len += Utils::encryptThenMAC(secret, &packet->payload[len], data, data_len); + len += Utils::encryptThenMAC(secret, PUB_KEY_SIZE, &packet->payload[len], data, data_len); packet->payload_len = len; @@ -536,7 +536,7 @@ Packet* Mesh::createGroupDatagram(uint8_t type, const GroupChannel& channel, con int len = 0; memcpy(&packet->payload[len], channel.hash, PATH_HASH_SIZE); len += PATH_HASH_SIZE; - len += Utils::encryptThenMAC(channel.secret, &packet->payload[len], data, data_len); + len += Utils::encryptThenMAC(channel.secret, 16, &packet->payload[len], data, data_len); packet->payload_len = len; diff --git a/src/Utils.cpp b/src/Utils.cpp index 186c8720a2..c7992efe9d 100644 --- a/src/Utils.cpp +++ b/src/Utils.cpp @@ -60,26 +60,26 @@ int Utils::encrypt(const uint8_t* shared_secret, uint8_t* dest, const uint8_t* s return dp - dest; // will always be multiple of 16 } -int Utils::encryptThenMAC(const uint8_t* shared_secret, uint8_t* dest, const uint8_t* src, int src_len) { +int Utils::encryptThenMAC(const uint8_t* shared_secret, int secret_len, uint8_t* dest, const uint8_t* src, int src_len) { int enc_len = encrypt(shared_secret, dest + CIPHER_MAC_SIZE, src, src_len); SHA256 sha; - sha.resetHMAC(shared_secret, PUB_KEY_SIZE); + sha.resetHMAC(shared_secret, secret_len); sha.update(dest + CIPHER_MAC_SIZE, enc_len); - sha.finalizeHMAC(shared_secret, PUB_KEY_SIZE, dest, CIPHER_MAC_SIZE); + sha.finalizeHMAC(shared_secret, secret_len, dest, CIPHER_MAC_SIZE); return CIPHER_MAC_SIZE + enc_len; } -int Utils::MACThenDecrypt(const uint8_t* shared_secret, uint8_t* dest, const uint8_t* src, int src_len) { +int Utils::MACThenDecrypt(const uint8_t* shared_secret, int secret_len, uint8_t* dest, const uint8_t* src, int src_len) { if (src_len <= CIPHER_MAC_SIZE) return 0; // invalid src bytes uint8_t hmac[CIPHER_MAC_SIZE]; { SHA256 sha; - sha.resetHMAC(shared_secret, PUB_KEY_SIZE); + sha.resetHMAC(shared_secret, secret_len); sha.update(src + CIPHER_MAC_SIZE, src_len - CIPHER_MAC_SIZE); - sha.finalizeHMAC(shared_secret, PUB_KEY_SIZE, hmac, CIPHER_MAC_SIZE); + sha.finalizeHMAC(shared_secret, secret_len, hmac, CIPHER_MAC_SIZE); } if (memcmp(hmac, src, CIPHER_MAC_SIZE) == 0) { return decrypt(shared_secret, dest, src + CIPHER_MAC_SIZE, src_len - CIPHER_MAC_SIZE); diff --git a/src/Utils.h b/src/Utils.h index 5736b8747a..8194e0301e 100644 --- a/src/Utils.h +++ b/src/Utils.h @@ -46,13 +46,13 @@ class Utils { * \brief encrypts bytes in src, then calculates MAC on ciphertext, inserting into leading bytes of 'dest'. * \returns total length of bytes in 'dest' (MAC + ciphertext) */ - static int encryptThenMAC(const uint8_t* shared_secret, uint8_t* dest, const uint8_t* src, int src_len); + static int encryptThenMAC(const uint8_t* shared_secret, int secret_len, uint8_t* dest, const uint8_t* src, int src_len); /** * \brief checks the MAC (in leading bytes of 'src'), then if valid, decrypts remaining bytes in src. * \returns zero if MAC is invalid, otherwise the length of decrypted bytes in 'dest' */ - static int MACThenDecrypt(const uint8_t* shared_secret, uint8_t* dest, const uint8_t* src, int src_len); + static int MACThenDecrypt(const uint8_t* shared_secret, int secret_len, uint8_t* dest, const uint8_t* src, int src_len); /** * \brief converts 'src' bytes with given length to Hex representation, and null terminates.