From d9e67222f59391a4352bd406ed21bed69ae0dc22 Mon Sep 17 00:00:00 2001 From: Wessel Nieboer Date: Wed, 25 Feb 2026 09:11:23 +0100 Subject: [PATCH 1/3] prefs is 5 char length :nerd: --- src/helpers/CommonCLI.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/helpers/CommonCLI.cpp b/src/helpers/CommonCLI.cpp index e20bbb1c0..fd6312734 100644 --- a/src/helpers/CommonCLI.cpp +++ b/src/helpers/CommonCLI.cpp @@ -749,7 +749,7 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch _prefs->advert_loc_policy = ADVERT_LOC_SHARE; savePrefs(); strcpy(reply, "ok"); - } else if (memcmp(command+11, "prefs", 4) == 0) { + } else if (memcmp(command+11, "prefs", 5) == 0) { _prefs->advert_loc_policy = ADVERT_LOC_PREFS; savePrefs(); strcpy(reply, "ok"); From 9fdb6922d8ff2c39aacc16064f721f93b4d2a193 Mon Sep 17 00:00:00 2001 From: Wessel Nieboer Date: Wed, 11 Feb 2026 03:30:41 +0100 Subject: [PATCH 2/3] 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 a1c045722e10c973681e2aa9f04f55d3a3ec0925 Mon Sep 17 00:00:00 2001 From: Wessel Nieboer Date: Mon, 16 Feb 2026 09:49:19 +0100 Subject: [PATCH 3/3] 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