From 4794cdb12004fc2cb17369297c0b087f546ef3f9 Mon Sep 17 00:00:00 2001 From: TheMalkavien Date: Fri, 27 Sep 2024 00:44:11 +0200 Subject: [PATCH] Fix (some ?) memory alignment issues on the crypto part - resulting in crashes or strange behavior (#4867) * Replace multiple potentially non aligned pointer dereference (#4855) First step to fix some Crypto crashes or strange behaviors * Makes the two Crypto byte buffers aligned (#4855) Fix #4855, and probably multiple Crypto problems depending on hardware --------- Co-authored-by: Ben Meadors Co-authored-by: GUVWAF <78759985+GUVWAF@users.noreply.github.com> --- src/mesh/CryptoEngine.cpp | 18 +++++++++--------- src/mesh/Router.cpp | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/mesh/CryptoEngine.cpp b/src/mesh/CryptoEngine.cpp index f705bae030..f9d7b4ad29 100644 --- a/src/mesh/CryptoEngine.cpp +++ b/src/mesh/CryptoEngine.cpp @@ -70,8 +70,8 @@ bool CryptoEngine::encryptCurve25519(uint32_t toNode, uint32_t fromNode, uint64_ long extraNonceTmp = random(); auth = bytesOut + numBytes; extraNonce = (uint32_t *)(auth + 8); - *extraNonce = extraNonceTmp; - LOG_INFO("Random nonce value: %d\n", *extraNonce); + memcpy(extraNonce, &extraNonceTmp, 4); // do not use dereference on potential non aligned pointers : *extraNonce = extraNonceTmp; + LOG_INFO("Random nonce value: %d\n", extraNonceTmp); meshtastic_NodeInfoLite *node = nodeDB->getMeshNode(toNode); if (node->num < 1 || node->user.public_key.size == 0) { LOG_DEBUG("Node %d or their public_key not found\n", toNode); @@ -80,14 +80,14 @@ bool CryptoEngine::encryptCurve25519(uint32_t toNode, uint32_t fromNode, uint64_ if (!crypto->setDHKey(toNode)) { return false; } - initNonce(fromNode, packetNum, *extraNonce); + initNonce(fromNode, packetNum, extraNonceTmp); // Calculate the shared secret with the destination node and encrypt printBytes("Attempting encrypt using nonce: ", nonce, 13); printBytes("Attempting encrypt using 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 - *extraNonce = extraNonceTmp; + memcpy(extraNonce, &extraNonceTmp, 4); // do not use dereference on potential non aligned pointers : *extraNonce = extraNonceTmp; return true; } @@ -100,12 +100,12 @@ bool CryptoEngine::encryptCurve25519(uint32_t toNode, uint32_t fromNode, uint64_ bool CryptoEngine::decryptCurve25519(uint32_t fromNode, uint64_t packetNum, size_t numBytes, uint8_t *bytes, uint8_t *bytesOut) { uint8_t *auth; // set to last 8 bytes of text? - uint32_t *extraNonce; + uint32_t extraNonce; // pointer was not really used auth = bytes + numBytes - 12; - extraNonce = (uint32_t *)(auth + 8); - LOG_INFO("Random nonce value: %d\n", *extraNonce); + memcpy(&extraNonce, auth +8, 4); // do not use dereference on potential non aligned pointers : (uint32_t *)(auth + 8); + LOG_INFO("Random nonce value: %d\n", extraNonce); meshtastic_NodeInfoLite *node = nodeDB->getMeshNode(fromNode); - + if (node == nullptr || node->num < 1 || node->user.public_key.size == 0) { LOG_DEBUG("Node or its public key not found in database\n"); return false; @@ -115,7 +115,7 @@ bool CryptoEngine::decryptCurve25519(uint32_t fromNode, uint64_t packetNum, size if (!crypto->setDHKey(fromNode)) { return false; } - initNonce(fromNode, packetNum, *extraNonce); + initNonce(fromNode, packetNum, extraNonce); printBytes("Attempting decrypt using nonce: ", nonce, 13); printBytes("Attempting decrypt using shared_key starting with: ", shared_key, 8); return aes_ccm_ad(shared_key, 32, nonce, 8, bytes, numBytes - 12, nullptr, 0, auth, bytesOut); diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index a993ee7c0f..f06f541658 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -36,8 +36,8 @@ static MemoryDynamic staticPool; Allocator &packetPool = staticPool; -static uint8_t bytes[MAX_LORA_PAYLOAD_LEN + 1]; -static uint8_t ScratchEncrypted[MAX_LORA_PAYLOAD_LEN + 1]; +static uint8_t bytes[MAX_LORA_PAYLOAD_LEN + 1] __attribute__ ((__aligned__)); +static uint8_t ScratchEncrypted[MAX_LORA_PAYLOAD_LEN + 1] __attribute__ ((__aligned__)); /** * Constructor