Multiple fixes: buffer bound checks

.github/workflows/test-library.yml

@@ -33,11 +33,6 @@ jobs:
         asym: [ed25519, ecc256, ecc384, ecc521, rsa2048, rsa3072, rsa4096, ed448]
         hash: [sha256, sha384, sha3]
 
-        # See https://github.com/wolfSSL/wolfBoot/issues/614 regarding exclusions:
-        exclude:
-          - math: "SPMATH=1 WOLFBOOT_SMALL_STACK=1"
-          - math: "SPMATHALL=1 WOLFBOOT_SMALL_STACK=1"
-
     steps:
       - uses: actions/checkout@v4
         with:
@@ -201,4 +196,3 @@ jobs:
           fi
 
           echo "Got expected non-zero exit and 'Failure' message."
-

Makefile

@@ -612,7 +612,7 @@ include/target.h: $(TARGET_H_TEMPLATE) FORCE
 	sed -e "s/@WOLFBOOT_DTS_UPDATE_ADDRESS@/$(WOLFBOOT_DTS_UPDATE_ADDRESS)/g" | \
 	sed -e "s/@WOLFBOOT_LOAD_ADDRESS@/$(WOLFBOOT_LOAD_ADDRESS)/g" | \
 	sed -e "s/@WOLFBOOT_LOAD_DTS_ADDRESS@/$(WOLFBOOT_LOAD_DTS_ADDRESS)/g" | \
-	sed -e "s/@WOLFBOOT_RAMBOOT_MAX_SIZE@/$(WOLFBOOT_RAMBOOT_MAX_SIZE)/g" | \
+	sed -e "s|@WOLFBOOT_RAMBOOT_MAX_SIZE_DEFINE@|$(if $(strip $(WOLFBOOT_RAMBOOT_MAX_SIZE)),#define WOLFBOOT_RAMBOOT_MAX_SIZE             $(WOLFBOOT_RAMBOOT_MAX_SIZE),/* WOLFBOOT_RAMBOOT_MAX_SIZE undefined */)|g" | \
 	sed -e "s/@WOLFBOOT_PARTITION_SELF_HEADER_ADDRESS@/$(WOLFBOOT_PARTITION_SELF_HEADER_ADDRESS)/g" \
 		> $@
 

docs/TPM.md

@@ -47,6 +47,10 @@ int wolfBoot_unseal_auth(const uint8_t* pubkey_hint, const uint8_t* policy, uint
     int index, uint8_t* secret, int* secret_sz, const byte* auth, int authSz);
 ```
 
+For `wolfBoot_unseal_auth()`, `*secret_sz` is an in/out parameter: set it to the
+capacity of `secret` before the call, and on success it is updated to the
+number of bytes unsealed.
+
 By default this index will be based on an NV Index at `(0x01400300 + index)`.
 The default NV base can be overridden with `WOLFBOOT_TPM_SEAL_NV_BASE`.
 

hal/stm32h7.c

@@ -160,7 +160,7 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len)
 
 void RAMFUNCTION hal_flash_unlock(void)
 {
-    flash_wait_complete(1);
+    flash_wait_complete(0);
     if ((FLASH_CR1 & FLASH_CR_LOCK) != 0) {
         FLASH_KEYR1 = FLASH_KEY1;
         DMB();
@@ -170,7 +170,7 @@ void RAMFUNCTION hal_flash_unlock(void)
             ;
     }
 
-    flash_wait_complete(2);
+    flash_wait_complete(1);
     if ((FLASH_CR2 & FLASH_CR_LOCK) != 0) {
         FLASH_KEYR2 = FLASH_KEY1;
         DMB();
@@ -183,11 +183,11 @@ void RAMFUNCTION hal_flash_unlock(void)
 
 void RAMFUNCTION hal_flash_lock(void)
 {
-    flash_wait_complete(1);
+    flash_wait_complete(0);
     if ((FLASH_CR1 & FLASH_CR_LOCK) == 0)
         FLASH_CR1 |= FLASH_CR_LOCK;
 
-    flash_wait_complete(2);
+    flash_wait_complete(1);
     if ((FLASH_CR2 & FLASH_CR_LOCK) == 0)
         FLASH_CR2 |= FLASH_CR_LOCK;
 }
@@ -619,4 +619,3 @@ int hal_flash_otp_read(uint32_t flashAddress, void* data, uint32_t length)
 }
 
 #endif /* FLASH_OTP_KEYSTORE */
-

include/target.h.in

@@ -118,7 +118,7 @@
 #endif
 
 /* Optional RAM-boot image size cap for targets without partitions */
-#define WOLFBOOT_RAMBOOT_MAX_SIZE             @WOLFBOOT_RAMBOOT_MAX_SIZE@
+@WOLFBOOT_RAMBOOT_MAX_SIZE_DEFINE@
 #define WOLFBOOT_LOAD_DTS_ADDRESS             @WOLFBOOT_LOAD_DTS_ADDRESS@
 
 

src/image.c

@@ -1143,6 +1143,7 @@ static void key_sha384(uint8_t key_slot, uint8_t *hash)
     int pubkey_sz = keystore_get_size(key_slot);
     wc_Sha384 sha384_ctx;
 
+    memset(hash, 0, SHA384_DIGEST_SIZE);
     if (!pubkey || (pubkey_sz < 0))
         return;
 
@@ -1237,6 +1238,7 @@ static void key_sha3_384(uint8_t key_slot, uint8_t *hash)
     int pubkey_sz = keystore_get_size(key_slot);
     wc_Sha3 sha3_ctx;
 
+    memset(hash, 0, WC_SHA3_384_DIGEST_SIZE);
     if (!pubkey || (pubkey_sz < 0))
         return;
     wc_InitSha3_384(&sha3_ctx, NULL, INVALID_DEVID);
@@ -1317,6 +1319,14 @@ int wolfBoot_open_image_address(struct wolfBoot_image *img, uint8_t *image)
     }
     img->trailer = img->hdr + WOLFBOOT_PARTITION_SIZE;
 #else
+#ifdef WOLFBOOT_RAMBOOT_MAX_SIZE
+    if (img->fw_size > WOLFBOOT_RAMBOOT_MAX_SIZE) {
+        wolfBoot_printf("Image size %d > max %d\n",
+            (unsigned int)img->fw_size,
+            (unsigned int)WOLFBOOT_RAMBOOT_MAX_SIZE);
+        return -1;
+    }
+#endif
     if (img->hdr == NULL) {
         img->hdr = image;
     }

src/libwolfboot.c

@@ -374,6 +374,8 @@ static int RAMFUNCTION partition_magic_write(uint8_t part, uintptr_t addr)
     XMEMCPY(NVM_CACHE, (void*)addr_read, NVM_CACHE_SIZE);
     XMEMCPY(NVM_CACHE + off, &wolfboot_magic_trail, sizeof(uint32_t));
     ret = hal_flash_write(addr_write, NVM_CACHE, WOLFBOOT_SECTOR_SIZE);
+    if (ret != 0)
+        return ret;
     nvm_cached_sector = !nvm_cached_sector;
     ret = hal_flash_erase(addr_read, WOLFBOOT_SECTOR_SIZE);
     return ret;

src/string.c

@@ -98,8 +98,9 @@ char *strcat(char *dest, const char *src)
 {
     size_t i = 0;
     size_t j = strlen(dest);
+    size_t src_len = strlen(src);
 
-    for (i = 0; i < strlen(src); i++) {
+    for (i = 0; i < src_len; i++) {
         dest[j++] = src[i];
     }
     dest[j] = '\0';
@@ -186,6 +187,10 @@ char *strncpy(char *dst, const char *src, size_t n)
             break;
     }
 
+    while (++i < n) {
+        dst[i] = '\0';
+    }
+
     return dst;
 }
 

src/tpm.c

@@ -44,6 +44,19 @@ WOLFTPM2_KEY     wolftpm_srk;
 #endif
 
 #if defined(WOLFBOOT_TPM_SEAL) || defined(WOLFBOOT_TPM_KEYSTORE)
+static int wolfBoot_constant_compare(const uint8_t* a, const uint8_t* b,
+    uint32_t len)
+{
+    uint32_t i;
+    uint8_t diff = 0;
+
+    for (i = 0; i < len; i++) {
+        diff |= a[i] ^ b[i];
+    }
+
+    return diff;
+}
+
 void wolfBoot_print_hexstr(const unsigned char* bin, unsigned long sz,
     unsigned long maxLine)
 {
@@ -605,6 +618,8 @@ int wolfBoot_store_blob(TPMI_RH_NV_AUTH authHandle, uint32_t nvIndex,
     if (authSz > 0) {
         if (auth == NULL)
             return BAD_FUNC_ARG;
+        if (authSz > sizeof(nv.handle.auth.buffer))
+            return BAD_FUNC_ARG;
         nv.handle.auth.size = authSz;
         memcpy(nv.handle.auth.buffer, auth, authSz);
     }
@@ -685,6 +700,8 @@ int wolfBoot_read_blob(uint32_t nvIndex, WOLFTPM2_KEYBLOB* blob,
     if (authSz > 0) {
         if (auth == NULL)
             return BAD_FUNC_ARG;
+        if (authSz > sizeof(nv.handle.auth.buffer))
+            return BAD_FUNC_ARG;
         nv.handle.auth.size = authSz;
         memcpy(nv.handle.auth.buffer, auth, authSz);
     }
@@ -696,9 +713,14 @@ int wolfBoot_read_blob(uint32_t nvIndex, WOLFTPM2_KEYBLOB* blob,
         (uint8_t*)&blob->pub.size, &readSz, pos);
     if (rc == 0) {
         pos += readSz;
-        readSz = blob->pub.size;
-        rc = wolfTPM2_NVReadAuth(&wolftpm_dev, &nv, nv.handle.hndl,
-            pubAreaBuffer, &readSz, pos);
+        if (blob->pub.size > sizeof(pubAreaBuffer)) {
+            rc = BUFFER_E;
+        }
+        else {
+            readSz = blob->pub.size;
+            rc = wolfTPM2_NVReadAuth(&wolftpm_dev, &nv, nv.handle.hndl,
+                pubAreaBuffer, &readSz, pos);
+        }
     }
     if (rc == 0) {
         pos += readSz;
@@ -712,9 +734,14 @@ int wolfBoot_read_blob(uint32_t nvIndex, WOLFTPM2_KEYBLOB* blob,
     }
     if (rc == 0) {
         pos += sizeof(blob->priv.size);
-        readSz = blob->priv.size;
-        rc = wolfTPM2_NVReadAuth(&wolftpm_dev, &nv, nv.handle.hndl,
-            blob->priv.buffer, &readSz, pos);
+        if (blob->priv.size > sizeof(blob->priv.buffer)) {
+            rc = BUFFER_E;
+        }
+        else {
+            readSz = blob->priv.size;
+            rc = wolfTPM2_NVReadAuth(&wolftpm_dev, &nv, nv.handle.hndl,
+                blob->priv.buffer, &readSz, pos);
+        }
     }
     if (rc == 0) {
         pos += blob->priv.size;
@@ -744,6 +771,8 @@ int wolfBoot_delete_blob(TPMI_RH_NV_AUTH authHandle, uint32_t nvIndex,
     if (authSz > 0) {
         if (auth == NULL)
             return BAD_FUNC_ARG;
+        if (authSz > sizeof(nv.handle.auth.buffer))
+            return BAD_FUNC_ARG;
         nv.handle.auth.size = authSz;
         memcpy(nv.handle.auth.buffer, auth, authSz);
     }
@@ -925,6 +954,7 @@ int wolfBoot_unseal_blob(const uint8_t* pubkey_hint,
     const uint8_t* auth, int authSz)
 {
     int rc, i;
+    int secret_capacity;
     WOLFTPM2_SESSION policy_session;
     uint32_t key_type;
     TPM_ALG_ID pcrAlg = WOLFBOOT_TPM_PCR_ALG;
@@ -949,8 +979,13 @@ int wolfBoot_unseal_blob(const uint8_t* pubkey_hint,
         return -1;
     }
 
+    secret_capacity = *secret_sz;
     *secret_sz = 0; /* init */
 
+    if (secret_capacity < 0) {
+        return BAD_FUNC_ARG;
+    }
+
     /* extract pcrMask and populate PCR selection array */
     memcpy(&pcrMask, policy, sizeof(pcrMask));
     memset(pcrArray, 0, sizeof(pcrArray));
@@ -1069,9 +1104,16 @@ int wolfBoot_unseal_blob(const uint8_t* pubkey_hint,
         rc = TPM2_Unseal(&unsealIn, &unsealOut);
     }
     if (rc == 0) {
-        *secret_sz = unsealOut.outData.size;
-        memcpy(secret, unsealOut.outData.buffer, *secret_sz);
+        if (unsealOut.outData.size > WOLFBOOT_MAX_SEAL_SZ ||
+                (int)unsealOut.outData.size > secret_capacity) {
+            rc = BUFFER_E;
+        }
+        else {
+            *secret_sz = unsealOut.outData.size;
+            memcpy(secret, unsealOut.outData.buffer, *secret_sz);
+        }
     }
+    TPM2_ForceZero(&unsealOut, sizeof(unsealOut));
 
     wolfTPM2_UnloadHandle(&wolftpm_dev, &seal_blob->handle);
     wolfTPM2_UnloadHandle(&wolftpm_dev, &policy_session.handle);
@@ -1486,7 +1528,8 @@ int wolfBoot_check_rot(int key_slot, uint8_t* pubkey_hint)
         if (rc == 0) {
             /* verify the hint (hash) matches */
             if (digestSz == WOLFBOOT_SHA_DIGEST_SIZE &&
-                memcmp(digest, pubkey_hint, WOLFBOOT_SHA_DIGEST_SIZE) == 0) {
+                wolfBoot_constant_compare(digest, pubkey_hint,
+                    WOLFBOOT_SHA_DIGEST_SIZE) == 0) {
                 wolfBoot_printf("TPM Root of Trust valid (id %d)\n", key_slot);
             }
             else {

src/update_disk.c

@@ -134,6 +134,9 @@ static uint32_t get_decrypted_blob_version(uint8_t *hdr)
             continue;
         }
 
+        if (p + 4 + tlv_len > max_p)
+            break;
+
         if (tlv_type == HDR_VERSION && tlv_len == 4) {
             uint32_t ver = *((uint32_t*)(p + 4));
             return ver;
@@ -208,6 +211,12 @@ static int decrypt_header(const uint8_t *src, uint8_t *dst)
     return 0;
 }
 
+static void disk_crypto_clear(void)
+{
+    ForceZero(disk_encrypt_key, sizeof(disk_encrypt_key));
+    ForceZero(disk_encrypt_nonce, sizeof(disk_encrypt_nonce));
+}
+
 #endif /* DISK_ENCRYPT */
 
 extern int wolfBoot_get_dts_size(void *dts_addr);
@@ -254,11 +263,13 @@ void RAMFUNCTION wolfBoot_start(void)
 #ifdef DISK_ENCRYPT
     /* Initialize encryption - this sets up the cipher with key from storage */
     if (wolfBoot_initialize_encryption() != 0) {
+        disk_crypto_clear();
         wolfBoot_printf("Error initializing encryption\r\n");
         wolfBoot_panic();
     }
     /* Retrieve encryption key and nonce for disk decryption */
     if (wolfBoot_get_encrypt_key(disk_encrypt_key, disk_encrypt_nonce) != 0) {
+        disk_crypto_clear();
         wolfBoot_printf("Error getting encryption key\r\n");
         wolfBoot_panic();
     }
@@ -267,10 +278,16 @@ void RAMFUNCTION wolfBoot_start(void)
 
     ret = disk_init(BOOT_DISK);
     if (ret != 0) {
+#ifdef DISK_ENCRYPT
+        disk_crypto_clear();
+#endif
         wolfBoot_panic();
     }
 
     if (disk_open(BOOT_DISK) < 0) {
+#ifdef DISK_ENCRYPT
+        disk_crypto_clear();
+#endif
         wolfBoot_printf("Error opening disk %d\r\n", BOOT_DISK);
         wolfBoot_panic();
     }
@@ -306,6 +323,9 @@ void RAMFUNCTION wolfBoot_start(void)
     }
 
     if ((pB_ver == 0) && (pA_ver == 0)) {
+#ifdef DISK_ENCRYPT
+        disk_crypto_clear();
+#endif
         wolfBoot_printf("No valid OS image found in either partition %d or %d\r\n",
             BOOT_PART_A, BOOT_PART_B);
         wolfBoot_panic();
@@ -409,6 +429,7 @@ void RAMFUNCTION wolfBoot_start(void)
         wolfBoot_printf("Decrypting image...");
         BENCHMARK_START();
         if ((IMAGE_HEADER_SIZE % ENCRYPT_BLOCK_SIZE) != 0) {
+            disk_crypto_clear();
             wolfBoot_printf("Encrypted disk images require aligned header size\r\n");
             wolfBoot_panic();
         }
@@ -456,6 +477,9 @@ void RAMFUNCTION wolfBoot_start(void)
     } while (failures < MAX_FAILURES);
 
     if (failures) {
+#ifdef DISK_ENCRYPT
+        disk_crypto_clear();
+#endif
         wolfBoot_printf("Unable to find a valid partition!\r\n");
         wolfBoot_panic();
     }
@@ -512,6 +536,9 @@ void RAMFUNCTION wolfBoot_start(void)
 
 #ifdef WOLFBOOT_HOOK_BOOT
     wolfBoot_hook_boot(&os_image);
+#endif
+#ifdef DISK_ENCRYPT
+    disk_crypto_clear();
 #endif
     do_boot((uint32_t*)load_address
     #ifdef MMU

src/update_flash.c

@@ -1206,7 +1206,7 @@ int wolfBoot_unlock_disk(void)
     int ret;
     struct wolfBoot_image img;
     uint8_t secret[WOLFBOOT_MAX_SEAL_SZ];
-    int     secretSz;
+    int     secretSz = sizeof(secret);
     uint8_t* policy = NULL, *pubkey_hint = NULL;
     uint16_t policySz = 0;
     int      nvIndex = 0; /* where the sealed blob is stored in NV */
@@ -1249,7 +1249,7 @@ int wolfBoot_unlock_disk(void)
             }
             if (ret == 0) {
                 uint8_t secretCheck[WOLFBOOT_MAX_SEAL_SZ];
-                int     secretCheckSz = 0;
+                int     secretCheckSz = sizeof(secretCheck);
 
                 /* unseal again to make sure it works */
                 memset(secretCheck, 0, sizeof(secretCheck));