fix(hash): Apply copilot suggestions

Author: lucasssvaz

cores/esp32/HashBuilder.h

@@ -20,6 +20,25 @@
 
 #include "HEXBuilder.h"
 
+/* Try to prevent most compilers from optimizing out clearing of memory that
+ * becomes unaccessible after this function is called. This is mostly the case
+ * for clearing local stack variables at the end of a function. This is not
+ * exactly perfect, i.e., someone could come up with a compiler that figures out
+ * the pointer is pointing to memset and then end up optimizing the call out, so
+ * try go a bit further by storing the first octet (now zero) to make this even
+ * a bit more difficult to optimize out. Once memset_s() is available, that
+ * could be used here instead. */
+static void * (* const volatile memset_func)(void *, int, size_t) = memset;
+static uint8_t forced_memzero_val;
+
+static inline void forced_memzero(void *ptr, size_t len)
+{
+  memset_func(ptr, 0, len);
+  if (len) {
+    forced_memzero_val = ((uint8_t *) ptr)[0];
+  }
+}
+
 // Base class for hash builders
 
 class HashBuilder : public HEXBuilder {

libraries/Hash/src/PBKDF2_HMACBuilder.cpp

@@ -47,6 +47,7 @@ PBKDF2_HMACBuilder::~PBKDF2_HMACBuilder() {
 
 void PBKDF2_HMACBuilder::clearData() {
   if (derivedKey != nullptr) {
+    forced_memzero(derivedKey, derivedKeyLen);
     delete[] derivedKey;
     derivedKey = nullptr;
   }
@@ -126,6 +127,7 @@ void PBKDF2_HMACBuilder::calculate() {
 
   // Allocate output buffer
   if (derivedKey != nullptr) {
+    forced_memzero(derivedKey, derivedKeyLen);
     delete[] derivedKey;
   }
   derivedKey = new uint8_t[derivedKeyLen];
@@ -148,9 +150,8 @@ void PBKDF2_HMACBuilder::getChars(char *output) {
     log_e("Error: PBKDF2-HMAC not calculated or no output buffer provided.");
     return;
   }
-  for (size_t i = 0; i < derivedKeyLen; i++) {
-    output[i] = (char)derivedKey[i];
-  }
+
+  bytes2hex(output, derivedKeyLen * 2 + 1, derivedKey, derivedKeyLen);
 }
 
 String PBKDF2_HMACBuilder::toString() {
@@ -159,19 +160,15 @@ String PBKDF2_HMACBuilder::toString() {
     return "";
   }
 
-  String result = "";
-  for (size_t i = 0; i < derivedKeyLen; i++) {
-    if (derivedKey[i] < 0x10) {
-      result += "0";
-    }
-    result += String(derivedKey[i], HEX);
-  }
-  return result;
+  char out[(derivedKeyLen * 2) + 1];
+  getChars(out);
+  return String(out);
 }
 
 // PBKDF2 specific methods
 void PBKDF2_HMACBuilder::setPassword(const uint8_t* password, size_t len) {
   if (this->password != nullptr) {
+    forced_memzero(this->password, len);
     delete[] this->password;
   }
   this->password = new uint8_t[len];
@@ -190,6 +187,7 @@ void PBKDF2_HMACBuilder::setPassword(String password) {
 
 void PBKDF2_HMACBuilder::setSalt(const uint8_t* salt, size_t len) {
   if (this->salt != nullptr) {
+    forced_memzero(this->salt, len);
     delete[] this->salt;
   }
   this->salt = new uint8_t[len];

libraries/Hash/src/SHA1Builder.cpp

@@ -195,6 +195,8 @@ void SHA1Builder::process(const uint8_t *data) {
 // Public methods
 
 void SHA1Builder::begin(void) {
+  finalized = false;
+
   total[0] = 0;
   total[1] = 0;
 
@@ -212,7 +214,7 @@ void SHA1Builder::add(const uint8_t *data, size_t len) {
   size_t fill;
   uint32_t left;
 
-  if (len == 0) {
+  if (finalized || len == 0) {
     return;
   }
 
@@ -289,6 +291,10 @@ void SHA1Builder::calculate(void) {
   uint32_t high, low;
   uint8_t msglen[8];
 
+  if (finalized) {
+    return;
+  }
+
   high = (total[0] >> 29) | (total[1] << 3);
   low = (total[0] << 3);
 
@@ -306,13 +312,20 @@ void SHA1Builder::calculate(void) {
   PUT_UINT32_BE(state[2], hash, 8);
   PUT_UINT32_BE(state[3], hash, 12);
   PUT_UINT32_BE(state[4], hash, 16);
+
+  finalized = true;
 }
 
 void SHA1Builder::getBytes(uint8_t *output) {
   memcpy(output, hash, SHA1_HASH_SIZE);
 }
 
 void SHA1Builder::getChars(char *output) {
+  if (!finalized || output == nullptr) {
+    log_e("Error: SHA1 not calculated or no output buffer provided.");
+    return;
+  }
+
   bytes2hex(output, SHA1_HASH_SIZE * 2 + 1, hash, SHA1_HASH_SIZE);
 }
 

libraries/Hash/src/SHA1Builder.h

@@ -28,12 +28,14 @@ class SHA1Builder : public HashBuilder {
   uint32_t state[5];            /* intermediate digest state  */
   unsigned char buffer[64];     /* data block being processed */
   uint8_t hash[SHA1_HASH_SIZE]; /* SHA-1 result               */
+  bool finalized;               /* Whether hash has been finalized */
 
   void process(const uint8_t *data);
 
 public:
   using HashBuilder::add;
 
+  SHA1Builder() : finalized(false) {}
   void begin() override;
   void add(const uint8_t *data, size_t len) override;
   bool addStream(Stream &stream, const size_t maxLen) override;

libraries/Hash/src/SHA2Builder.cpp

@@ -423,6 +423,11 @@ void SHA2Builder::getBytes(uint8_t *output) {
 
 // Get the hash as hex string
 void SHA2Builder::getChars(char *output) {
+  if (!finalized || output == nullptr) {
+      log_e("Error: SHA2 not calculated or no output buffer provided.");
+      return;
+  }
+
     bytes2hex(output, hash_size * 2 + 1, hash, hash_size);
 }
 

libraries/Hash/src/SHA3Builder.cpp

@@ -253,6 +253,11 @@ void SHA3Builder::getBytes(uint8_t *output) {
 
 // Get the hash as hex string
 void SHA3Builder::getChars(char *output) {
+  if (!finalized || output == nullptr) {
+    log_e("Error: SHA3 not calculated or no output buffer provided.");
+    return;
+  }
+
   bytes2hex(output, hash_size * 2 + 1, hash, hash_size);
 }