From 761f018c603cb30a66d558cfa2991e8274338c79 Mon Sep 17 00:00:00 2001 From: Wessel Nieboer Date: Wed, 11 Feb 2026 03:30:41 +0100 Subject: [PATCH 1/2] use constant-time comparison for MAC verification The HMAC check in MACThenDecrypt used standard memcmp(), which short-circuits on the first mismatched byte. This makes the comparison time dependent on how many bytes of the MAC are correct, leaking information through a timing side-channel. With a 2-byte MAC (65,536 possible values), an attacker on a local interface (serial, BLE, or WiFi) can measure response latency to distinguish "first byte wrong" from "first byte correct, second wrong". This reduces a brute-force from 65,536 attempts down to roughly 384 (256 + 128 on average), making MAC forgery practical. An attacker could use this to forge packets that pass MAC verification without knowing the shared secret, allowing them to inject arbitrary messages that appear to come from a trusted peer. Replace memcmp with a constant-time XOR-accumulate loop so the comparison always takes the same time regardless of which bytes match. --- src/Utils.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Utils.cpp b/src/Utils.cpp index 186c8720a..a07de2e8d 100644 --- a/src/Utils.cpp +++ b/src/Utils.cpp @@ -81,7 +81,10 @@ int Utils::MACThenDecrypt(const uint8_t* shared_secret, uint8_t* dest, const uin sha.update(src + CIPHER_MAC_SIZE, src_len - CIPHER_MAC_SIZE); sha.finalizeHMAC(shared_secret, PUB_KEY_SIZE, hmac, CIPHER_MAC_SIZE); } - if (memcmp(hmac, src, CIPHER_MAC_SIZE) == 0) { + // constant-time comparison to prevent timing side-channel attacks + uint8_t diff = 0; + for (int i = 0; i < CIPHER_MAC_SIZE; i++) diff |= hmac[i] ^ src[i]; + if (diff == 0) { return decrypt(shared_secret, dest, src + CIPHER_MAC_SIZE, src_len - CIPHER_MAC_SIZE); } return 0; // invalid HMAC From 83bbda06e8f3f4953d599fcb87c8b0110e1968bf Mon Sep 17 00:00:00 2001 From: Wessel Nieboer Date: Mon, 16 Feb 2026 09:49:19 +0100 Subject: [PATCH 2/2] Use secure_compare --- src/Utils.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Utils.cpp b/src/Utils.cpp index a07de2e8d..31af2db14 100644 --- a/src/Utils.cpp +++ b/src/Utils.cpp @@ -1,5 +1,6 @@ #include "Utils.h" #include +#include #include #ifdef ARDUINO @@ -81,10 +82,7 @@ int Utils::MACThenDecrypt(const uint8_t* shared_secret, uint8_t* dest, const uin sha.update(src + CIPHER_MAC_SIZE, src_len - CIPHER_MAC_SIZE); sha.finalizeHMAC(shared_secret, PUB_KEY_SIZE, hmac, CIPHER_MAC_SIZE); } - // constant-time comparison to prevent timing side-channel attacks - uint8_t diff = 0; - for (int i = 0; i < CIPHER_MAC_SIZE; i++) diff |= hmac[i] ^ src[i]; - if (diff == 0) { + if (secure_compare(hmac, src, CIPHER_MAC_SIZE)) { return decrypt(shared_secret, dest, src + CIPHER_MAC_SIZE, src_len - CIPHER_MAC_SIZE); } return 0; // invalid HMAC