Memory and Secret Handling Fixes

RPi-Pico/src/tlsClient_main.c

@@ -158,11 +158,12 @@ void tlsClient_test(void *arg)
             goto exit;
         }
 
-        ret = wolfSSL_read(ssl, buffer, BUFF_SIZE);
+        ret = wolfSSL_read(ssl, buffer, BUFF_SIZE - 1);
         if (ret < 0) {
             printf("Failed to read data. err=%d\n", ret);
             goto exit;
         }
+        buffer[ret] = '\0';
         printf("Message: %s\n", buffer);
 
         wolfSSL_free(ssl);

can-bus/common.c

@@ -103,7 +103,8 @@ int can_connect(const char *address, uint16_t filter)
     /* Set the filter */
     setsockopt(sock, SOL_CAN_RAW, CAN_RAW_FILTER, &rfilter, sizeof(rfilter));
 
-    strcpy(ifr.ifr_name, address);
+    strncpy(ifr.ifr_name, address, IFNAMSIZ - 1);
+    ifr.ifr_name[IFNAMSIZ - 1] = '\0';
     ioctl(sock, SIOCGIFINDEX, &ifr);
 
     memset(&addr, 0, sizeof(addr));
@@ -196,11 +197,11 @@ int setup_ssl(enum service_type type, WOLFSSL_CTX **new_ctx,
         return -1;
     }
 
-    wolfSSL_CTX_set_verify(ctx, WOLFSSL_VERIFY_NONE, NULL);
-
     if (type == SERVICE_TYPE_CLIENT) {
-        ret = wolfSSL_CTX_load_verify_locations(ctx, "client.pem", NULL);
+        wolfSSL_CTX_set_verify(ctx, WOLFSSL_VERIFY_PEER, NULL);
+        ret = wolfSSL_CTX_load_verify_locations(ctx, "ca.crt", NULL);
     } else {
+        wolfSSL_CTX_set_verify(ctx, WOLFSSL_VERIFY_NONE, NULL);
         ret = wolfSSL_CTX_use_certificate_file(ctx, "server.pem",
                 WOLFSSL_FILETYPE_PEM);
     }

ccb_vaultic/ccb_vaultic.c

@@ -87,6 +87,7 @@
 
 /* wolfcrypt includes */
 #include "wolfssl/wolfcrypt/types.h"       /* types and X-defines */
+#include "wolfssl/wolfcrypt/memory.h"      /* For wc_ForceZero */
 
 #ifndef CCBVAULTIC_NO_SHA
 #include "wolfssl/wolfcrypt/hash.h"  /* For HASH_FLAGS and types */
@@ -156,6 +157,31 @@ static void hexdump(const unsigned char* p, size_t len)
 }
 #endif
 
+#ifdef WOLF_CRYPTO_CB
+/* Constant-time buffer compare for secret material.
+ *
+ * wolfSSL's own constant-time compare (ConstantCompare() in
+ * wolfcrypt/src/misc.c) is WOLFSSL_LOCAL and not exported, so it can't be
+ * called from application code. Every byte is compared regardless of where
+ * a mismatch occurs, so the runtime doesn't leak position information via
+ * early-exit timing.
+ *
+ * Returns 0 if the buffers are equal, non-zero otherwise. */
+static int const_time_memcmp(const void* a, const void* b, size_t len)
+{
+    const byte* pa = (const byte*)a;
+    const byte* pb = (const byte*)b;
+    byte diff = 0;
+    size_t i;
+
+    for (i = 0; i < len; i++) {
+        diff |= (byte)(pa[i] ^ pb[i]);
+    }
+
+    return (int)diff;
+}
+#endif /* WOLF_CRYPTO_CB */
+
 /* Helper to translate vlt return codes to wolfSSL code */
 static int translateError(int vlt_rc)
 {
@@ -233,8 +259,10 @@ void ccbVaultIc_Cleanup(ccbVaultIc_Context *c)
     /* Free allocated buffers */
     if (c->m != NULL)
         XFREE(c->m, NULL, NULL);
-    if (c->aescbc_key != NULL)
+    if (c->aescbc_key != NULL) {
+        wc_ForceZero(c->aescbc_key, c->aescbc_keylen);
         XFREE(c->aescbc_key, NULL, NULL);
+    }
 
     clearContext(c);
 
@@ -975,17 +1003,20 @@ static int HandleCipherCallback(int devId, wc_CryptoInfo* info,
             vlt_mode = VLT_DECRYPT_MODE;
         }
 
-        /* Check if key is not the same as last time */
+        /* Check if key is not the same as last time. Uses a constant-time
+         * compare since this is secret key material, even though this
+         * check only gates a cache refresh (not authentication). */
         if ((c->aescbc_key    == NULL) ||
             (c->aescbc_keylen != aes->keylen) ||
-            (XMEMCMP(c->aescbc_key, aes->devKey, aes->keylen))) {
+            (const_time_memcmp(c->aescbc_key, aes->devKey, aes->keylen))) {
 #if defined(CCBVAULTIC_DEBUG_ALL)
             XPRINTF("   New AES Key: ckey:%p clen:%lu akey:%p alen:%u\n",
                     c->aescbc_key,c->aescbc_keylen, aes->devKey, aes->keylen);
             hexdump((void*)aes->devKey, aes->keylen);
 #endif
             /* Free the current key buffer if necessary */
             if (c->aescbc_key != NULL) {
+                wc_ForceZero(c->aescbc_key, c->aescbc_keylen);
                 XFREE(c->aescbc_key, NULL, NULL);
                 c->aescbc_key    = NULL;
                 c->aescbc_keylen = 0;

certfields/all-fields/main.c

@@ -64,7 +64,7 @@ int main(int argc, char** argv)
 
     RsaKey pubKeyRsa;
     ecc_key pubKeyEcc;
-    WOLFSSL_X509* cert;
+    WOLFSSL_X509* cert = NULL;
     WOLFSSL_EVP_PKEY* pubKeyTmp;
     WOLFSSL_X509_NAME* name;
 

certgen/csr_example.c

@@ -218,6 +218,7 @@ static int init_pk_key(Cert* req, void** keyPtr, WC_RNG* rng, byte* der,
             ret = wc_ecc_init((ecc_key*)*keyPtr);
             if (ret != 0) {
                 XFREE(*keyPtr, NULL, DYNAMIC_TYPE_ECC);
+                *keyPtr = NULL;
                 break;
             }
             ret = wc_ecc_make_key_ex(rng, 32, (ecc_key*)*keyPtr, ECC_SECP256R1);
@@ -249,6 +250,7 @@ static int init_pk_key(Cert* req, void** keyPtr, WC_RNG* rng, byte* der,
             ret = wc_InitRsaKey((RsaKey*)*keyPtr, NULL);
             if (ret != 0) {
                 XFREE(*keyPtr, NULL, DYNAMIC_TYPE_RSA);
+                *keyPtr = NULL;
                 break;
             }
             ret = wc_MakeRsaKey((RsaKey*)*keyPtr, 2048, WC_RSA_EXPONENT, rng);
@@ -280,6 +282,7 @@ static int init_pk_key(Cert* req, void** keyPtr, WC_RNG* rng, byte* der,
             ret = wc_ed25519_init((ed25519_key*)*keyPtr);
             if (ret != 0) {
                 XFREE(*keyPtr, NULL, DYNAMIC_TYPE_ED25519);
+                *keyPtr = NULL;
                 break;
             }
             ret = wc_ed25519_make_key(rng, ED25519_KEY_SIZE,

crypto/3des/3des-file-encrypt.c

@@ -27,6 +27,7 @@
 #include <wolfssl/wolfcrypt/sha256.h>
 #include <wolfssl/wolfcrypt/random.h>
 #include <wolfssl/wolfcrypt/pwdbased.h>
+#include <wolfssl/wolfcrypt/memory.h>
 
 #define DES3_BLOCK_SIZE 24               /* size of encryption blocks */
 #define SALT_SIZE 8
@@ -82,12 +83,24 @@ int Des3Encrypt(Des3* des3, byte* key, int size, FILE* inFile, FILE* outFile)
         padCounter++;
     }
 
-    input = malloc(length);
-    output = malloc(length);
+    input = XMALLOC(length, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+    output = XMALLOC(length, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+    if (input == NULL || output == NULL) {
+        printf("Failed to allocate memory\n");
+        wc_ForceZero(key, size);
+        XFREE(input, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+        XFREE(output, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+        XFREE(key, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+        return -1012;
+    }
 
     ret = wc_InitRng(&rng);
     if (ret != 0) {
         printf("Failed to initialize random number generator\n");
+        wc_ForceZero(key, size);
+        XFREE(input, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+        XFREE(output, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+        XFREE(key, NULL, DYNAMIC_TYPE_TMP_BUFFER);
         return -1030;
     }
 
@@ -127,12 +140,12 @@ int Des3Encrypt(Des3* des3, byte* key, int size, FILE* inFile, FILE* outFile)
     fwrite(output, 1, length, outFile);
 
     /* closes the opened files and frees the memory*/
-    memset(input, 0, length);
-    memset(output, 0, length);
-    memset(key, 0, size);
-    free(input);
-    free(output);
-    free(key);
+    wc_ForceZero(input, length);
+    wc_ForceZero(output, length);
+    wc_ForceZero(key, size);
+    XFREE(input, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+    XFREE(output, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+    XFREE(key, NULL, DYNAMIC_TYPE_TMP_BUFFER);
     fclose(inFile);
     fclose(outFile);
     wc_FreeRng(&rng);
@@ -161,62 +174,126 @@ int Des3Decrypt(Des3* des3, byte* key, int size, FILE* inFile, FILE* outFile)
     fseek(inFile, 0, SEEK_SET);
     aSize = length;
 
-    input = malloc(aSize);
-    output = malloc(aSize);
+    if (length < SALT_SIZE + DES3_BLOCK_SIZE) {
+        printf("Input file is too small.\n");
+        wc_ForceZero(key, size);
+        XFREE(key, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+        fclose(inFile);
+        fclose(outFile);
+        return -1011;
+    }
+
+    input = XMALLOC(aSize, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+    output = XMALLOC(aSize, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+    if (input == NULL || output == NULL) {
+        printf("Failed to allocate memory\n");
+        wc_ForceZero(key, size);
+        XFREE(input, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+        XFREE(output, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+        XFREE(key, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+        fclose(inFile);
+        fclose(outFile);
+        return -1012;
+    }
 
-    wc_InitRng(&rng);
+    ret = wc_InitRng(&rng);
+    if (ret != 0) {
+        printf("Failed to initialize random number generator\n");
+        wc_ForceZero(key, size);
+        XFREE(input, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+        XFREE(output, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+        XFREE(key, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+        fclose(inFile);
+        fclose(outFile);
+        return -1030;
+    }
 
     /* reads from inFile and writes whatever is there to the input array */
     ret = fread(input, 1, length, inFile);
     if (ret == 0) {
         printf("Input file does not exist.\n");
-        return -1010;
-    }
-    for (i = 0; i < SALT_SIZE; i++) {
-        /* finds salt from input message */
-        salt[i] = input[i];
+        ret = -1010;
     }
-    for (i = SALT_SIZE; i < DES3_BLOCK_SIZE + SALT_SIZE; i++) {
-        /* finds iv from input message */
-        iv[i - SALT_SIZE] = input[i];
+
+    if (ret > 0) {
+        for (i = 0; i < SALT_SIZE; i++) {
+            /* finds salt from input message */
+            salt[i] = input[i];
+        }
+        for (i = SALT_SIZE; i < DES3_BLOCK_SIZE + SALT_SIZE; i++) {
+            /* finds iv from input message */
+            iv[i - SALT_SIZE] = input[i];
+        }
+
+        /* replicates old key if keys match */
+        ret = wc_PBKDF2(key, key, strlen((const char*)key), salt, SALT_SIZE,
+            4096, size, WC_SHA256);
+        if (ret != 0)
+            ret = -1050;
     }
 
-    /* replicates old key if keys match */
-    ret = wc_PBKDF2(key, key, strlen((const char*)key), salt, SALT_SIZE, 4096,
-        size, WC_SHA256);
-    if (ret != 0)
-        return -1050;
+    if (ret == 0) {
+        /* sets key */
+        ret = wc_Des3_SetKey(des3, key, iv, DES_DECRYPTION);
+        if (ret != 0)
+            ret = -1002;
+    }
 
-    /* sets key */
-    ret = wc_Des3_SetKey(des3, key, iv, DES_DECRYPTION);
-    if (ret != 0)
-        return -1002;
+    if (ret == 0) {
+        /* change length to remove salt/iv block from being decrypted */
+        length -= (DES3_BLOCK_SIZE + SALT_SIZE);
+        for (i = 0; i < length; i++) {
+            /* shifts message: ignores salt/iv on message*/
+            input[i] = input[i + (DES3_BLOCK_SIZE + SALT_SIZE)];
+        }
+        /* decrypts the message to output based on input length + padding */
+        ret = wc_Des3_CbcDecrypt(des3, output, input, length);
+        if (ret != 0)
+            ret = -1006;
+    }
 
-    /* change length to remove salt/iv block from being decrypted */
-    length -= (DES3_BLOCK_SIZE + SALT_SIZE);
-    for (i = 0; i < length; i++) {
-        /* shifts message: ignores salt/iv on message*/
-        input[i] = input[i + (DES3_BLOCK_SIZE + SALT_SIZE)];
+    if (ret != 0) {
+        wc_ForceZero(input, aSize);
+        wc_ForceZero(output, aSize);
+        wc_ForceZero(key, size);
+        XFREE(input, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+        XFREE(output, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+        XFREE(key, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+        fclose(inFile);
+        fclose(outFile);
+        wc_FreeRng(&rng);
+        return ret;
     }
-    /* decrypts the message to output based on input length + padding */
-    ret = wc_Des3_CbcDecrypt(des3, output, input, length);
-    if (ret != 0)
-        return -1006;
 
     if (salt[0] != 0) {
         /* reduces length based on number of padded elements  */
+        if (length < 1 || output[length-1] < 1 ||
+                output[length-1] > length ||
+                output[length-1] > DES3_BLOCK_SIZE) {
+            printf("Error: invalid padding value\n");
+            wc_ForceZero(input, aSize);
+            wc_ForceZero(output, aSize);
+            wc_ForceZero(key, size);
+            XFREE(input, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+            XFREE(output, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+            XFREE(key, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+            fclose(inFile);
+            fclose(outFile);
+            wc_FreeRng(&rng);
+            return -1013;
+        }
         length -= output[length-1];
     }
     /* writes output to the outFile based on shortened length */
     fwrite(output, 1, length, outFile);
 
     /* closes the opened files and frees the memory*/
-    memset(input, 0, aSize);
-    memset(output, 0, aSize);
-    memset(key, 0, size);
-    free(input);
-    free(output);
-    free(key);
+    wc_ForceZero(input, aSize);
+    wc_ForceZero(output, aSize);
+    wc_ForceZero(key, size);
+    XFREE(input, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+    XFREE(output, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+    XFREE(key, NULL, DYNAMIC_TYPE_TMP_BUFFER);
     fclose(inFile);
     fclose(outFile);
     wc_FreeRng(&rng);
@@ -340,7 +417,7 @@ int main(int argc, char** argv)
     }
 
     else if (ret == 0 && choice != 'n') {
-        key = malloc(size);    /* sets size memory of key */
+        key = XMALLOC(size, NULL, DYNAMIC_TYPE_TMP_BUFFER);    /* sets size memory of key */
         ret = NoEcho((char*)key, size);
         if (choice == 'e')
             Des3Encrypt(&des3, key, size, inFile, outFile);