Fix stack buffer overflow in aes_ccm_encr for partial blocks (#11347)

* Fix out-of-bounds write in aes_ccm_encr for partial blocks

aes_ccm_encr() writes a full 16-byte AES block to the output before XOR-ing with
the input, so a trailing partial block writes up to 15 bytes past the length the
caller asked for. Every caller in the tree passes a buffer with enough slack, so
nothing misbehaves today, but the decrypt path clears it by only a few bytes.

Encrypt into a temporary block and XOR out of it, matching what
aes_ccm_encr_auth() and aes_ccm_decr_auth() already do in this same file. The
ciphertext is unchanged.

* Add regression test for the CCM partial-block write and drop stale workarounds

The guard bytes past the caller's buffer catch the overflow without relying on a
sanitizer, so the test is meaningful in the native environment too.

encryptCurve25519() no longer needs to write extraNonce before aes_ccm_ae(): the
call stays inside numBytes now, so the copy after it is the only one required.
The comment warning about the 15-byte overshoot no longer describes the code.

src/mesh/CryptoEngine.cpp

@@ -230,8 +230,6 @@ bool CryptoEngine::encryptCurve25519(uint32_t toNode, uint32_t fromNode, meshtas
     if (!HardwareRNG::fill((uint8_t *)&extraNonceTmp, sizeof(extraNonceTmp)))
         CryptRNG.rand((uint8_t *)&extraNonceTmp, sizeof(extraNonceTmp));
     auth = bytesOut + numBytes;
-    memcpy((uint8_t *)(auth + 8), &extraNonceTmp,
-           sizeof(uint32_t)); // do not use dereference on potential non aligned pointers : *extraNonce = extraNonceTmp;
     LOG_DEBUG("Random nonce value: %d", extraNonceTmp);
     if (remotePublic.size == 0) {
         LOG_DEBUG("Node %d or their public_key not found", toNode);
@@ -246,8 +244,7 @@ bool CryptoEngine::encryptCurve25519(uint32_t toNode, uint32_t fromNode, meshtas
     // Calculate the shared secret with the destination node and encrypt
     printBytes("Attempt encrypt with nonce: ", nonce, 13);
     printBytes("Attempt encrypt with shared_key starting with: ", shared_key, 8);
-    aes_ccm_ae(shared_key, 32, nonce, 8, bytes, numBytes, nullptr, 0, bytesOut,
-               auth); // this can write up to 15 bytes longer than numbytes past bytesOut
+    aes_ccm_ae(shared_key, 32, nonce, 8, bytes, numBytes, nullptr, 0, bytesOut, auth);
     memcpy((uint8_t *)(auth + 8), &extraNonceTmp,
            sizeof(uint32_t)); // do not use dereference on potential non aligned pointers : *extraNonce = extraNonceTmp;
     return true;

src/mesh/aes-ccm.cpp

@@ -111,11 +111,12 @@ static void aes_ccm_encr(size_t L, const uint8_t *in, size_t len, uint8_t *out,
         in += AES_BLOCK_SIZE;
     }
     if (last) {
+        uint8_t tmp[AES_BLOCK_SIZE];
         WPA_PUT_BE16(&a[AES_BLOCK_SIZE - 2], i);
-        crypto->aesEncrypt(a, out);
+        crypto->aesEncrypt(a, tmp);
         /* XOR zero-padded last block */
         for (i = 0; i < last; i++)
-            *out++ ^= *in++;
+            out[i] = tmp[i] ^ in[i];
     }
 }
 static void aes_ccm_encr_auth(size_t M, const uint8_t *x, uint8_t *a, uint8_t *auth)

test/test_crypto/test_main.cpp

@@ -2,6 +2,7 @@
 #include "CryptoEngine.h"
 
 #include "TestUtil.h"
+#include "aes-ccm.h"
 #include <XEdDSA.h>
 #include <unity.h>
 
@@ -310,6 +311,42 @@ void test_AES_CTR(void)
     TEST_ASSERT_EQUAL_MEMORY(expected, plain, 16);
 }
 
+void test_AES_CCM_partial_block_bounds(void)
+{
+    // aes_ccm_encr() used to write a whole 16-byte AES block at the output before XOR-ing,
+    // so a trailing partial block scribbled up to 15 bytes past what the caller allocated.
+    const uint8_t guard = 0xA5;
+    const size_t guardLen = 16;
+    const size_t lengths[] = {5, 20}; // pure partial block, and one full block plus a partial one
+    uint8_t key[32];
+    uint8_t nonce[13];
+    uint8_t auth[8];
+
+    HexToBytes(key, "603DEB1015CA71BE2B73AEF0857D77811F352C073B6108D72D9810A30914DFF4");
+    HexToBytes(nonce, "000102030405060708090A0B0C");
+
+    for (size_t n = 0; n < sizeof(lengths) / sizeof(lengths[0]); n++) {
+        const size_t len = lengths[n];
+        uint8_t plain[32];
+        uint8_t crypt[32 + guardLen];
+        uint8_t decrypted[32 + guardLen];
+
+        for (size_t i = 0; i < len; i++)
+            plain[i] = (uint8_t)i;
+        memset(crypt + len, guard, guardLen);
+        memset(decrypted + len, guard, guardLen);
+
+        TEST_ASSERT_EQUAL(0, aes_ccm_ae(key, sizeof(key), nonce, sizeof(auth), plain, len, nullptr, 0, crypt, auth));
+        for (size_t i = 0; i < guardLen; i++)
+            TEST_ASSERT_EQUAL_UINT8(guard, crypt[len + i]);
+
+        TEST_ASSERT_TRUE(aes_ccm_ad(key, sizeof(key), nonce, sizeof(auth), crypt, len, nullptr, 0, auth, decrypted));
+        for (size_t i = 0; i < guardLen; i++)
+            TEST_ASSERT_EQUAL_UINT8(guard, decrypted[len + i]);
+        TEST_ASSERT_EQUAL_MEMORY(plain, decrypted, len);
+    }
+}
+
 void setup()
 {
     // NOTE!!! Wait for >2 secs
@@ -323,6 +360,7 @@ void setup()
     RUN_TEST(test_ECB_AES256);
     RUN_TEST(test_DH25519);
     RUN_TEST(test_AES_CTR);
+    RUN_TEST(test_AES_CCM_partial_block_bounds);
     RUN_TEST(test_PKC);
     RUN_TEST(test_XEdDSA);
     RUN_TEST(test_XEdDSA_cross_key_reject);