From ef00afb14eae8534a6215de9001f9356f148f1b4 Mon Sep 17 00:00:00 2001
From: Peter Woolery <peter@research.bike>
Date: Fri, 1 May 2026 15:36:06 -0700
Subject: [PATCH] fix(firmware/lib): validate HMAC secret length and hex format
 before signing

hmac_sign() previously trusted whatever secret_hex came out of NVS:
- Lengths >128 chars overflowed the fixed 64-byte stack buffer in
  hex_to_bytes (out_len was unbounded).
- Non-hex characters were silently decoded to 0 via strtol with no
  end-pointer check, producing signatures under a corrupted key.
- Empty secrets fell through to mbedtls_md_hmac_starts with len=0.

flash_device.py now rejects malformed --hmac-secret at provision time,
but hmac_sign should also refuse to sign under a malformed key regardless
of how it ended up in NVS (legacy provisioning, partial flash, etc.).

Add length, hex-charset, and even-length validation; make hex_to_bytes
return bool and have hmac_sign return empty HString on any failure
(callers already treat empty as failure via post_json_once).

Found via adversarial review (run 2026-05-01-202910, both reviewers).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 firmware/lib/hmac/hmac.cpp | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/firmware/lib/hmac/hmac.cpp b/firmware/lib/hmac/hmac.cpp
index a3903c7..f8cc73e 100644
--- a/firmware/lib/hmac/hmac.cpp
+++ b/firmware/lib/hmac/hmac.cpp
@@ -14,12 +14,21 @@ static HString bytes_to_hex(const uint8_t* bytes, size_t len) {
     return out;
 }
 
-static void hex_to_bytes(const HString& hex, uint8_t* out, size_t out_len) {
-    if (hex.length() % 2 != 0) return; // malformed — odd-length hex
-    for (size_t i = 0; i < out_len && (i * 2 + 1) < hex.length(); i++) {
-        char byte_str[3] = {hex[i*2], hex[i*2+1], 0};
+static bool is_hex_char(char c) {
+    return (c >= '0' && c <= '9') ||
+           (c >= 'a' && c <= 'f') ||
+           (c >= 'A' && c <= 'F');
+}
+
+static bool hex_to_bytes(const HString& hex, uint8_t* out, size_t out_len) {
+    if (hex.length() != out_len * 2) return false;
+    for (size_t i = 0; i < out_len; i++) {
+        char a = hex[i*2], b = hex[i*2+1];
+        if (!is_hex_char(a) || !is_hex_char(b)) return false;
+        char byte_str[3] = {a, b, 0};
         out[i] = (uint8_t)strtol(byte_str, nullptr, 16);
     }
+    return true;
 }
 
 static bool sha256(const uint8_t* data, size_t len, uint8_t out[32]) {
@@ -52,10 +61,20 @@ HString hmac_sign(const HString& secret_hex,
     snprintf(ts_buf, sizeof(ts_buf), "%u", (unsigned)timestamp);
     HString message = method + "\n" + path + "\n" + ts_buf + "\n" + body_hash_hex;
 
-    // 3. Decode secret from hex
+    // 3. Decode secret from hex. Reject empty / odd-length / oversized /
+    // non-hex inputs — flash_device.py validates at provision time, but
+    // hmac_sign refuses to sign under a malformed key regardless of how it
+    // ended up in NVS (legacy provisioning, NVS corruption, etc.).
+    if (secret_hex.length() == 0 ||
+        secret_hex.length() > 128 ||
+        secret_hex.length() % 2 != 0) {
+        return HString{};
+    }
     size_t secret_len = secret_hex.length() / 2;
     uint8_t secret[64] = {};
-    hex_to_bytes(secret_hex, secret, secret_len);
+    if (!hex_to_bytes(secret_hex, secret, secret_len)) {
+        return HString{};
+    }
 
     // 4. HMAC-SHA256(secret, message)
     uint8_t hmac_result[32];