Fixes from static analysis

src/boot_arm32.c

@@ -86,7 +86,7 @@ void RAMFUNCTION do_boot(const uint32_t *app_offset)
 #ifdef RAM_CODE
 
 #define AIRCR *(volatile uint32_t *)(0xE000ED0C)
-#define AIRCR_VKEY (0r05FA << 16)
+#define AIRCR_VKEY (0x05FA << 16)
 #define AIRCR_SYSRESETREQ (1 << 2)
 
 void RAMFUNCTION arch_reboot(void)

src/pkcs11_store.c

@@ -436,9 +436,7 @@ int wolfPKCS11_Store_Open(int type, CK_ULONG id1, CK_ULONG id2, int read,
 void wolfPKCS11_Store_Close(void* store)
 {
     struct store_handle *handle = store;
-    /* This removes all flags (including STORE_FLAGS_OPEN) */
-    handle->flags = 0;
-    handle->hdr = NULL;
+    memset(handle, 0, sizeof(*handle));
 }
 
 int wolfPKCS11_Store_Read(void* store, unsigned char* buffer, int len)

src/psa_store.c

@@ -442,9 +442,7 @@ int wolfPSA_Store_OpenSz(int type, unsigned long id1, unsigned long id2, int rea
 void wolfPSA_Store_Close(void* store)
 {
     struct store_handle *handle = store;
-    /* This removes all flags (including STORE_FLAGS_OPEN) */
-    handle->flags = 0;
-    handle->hdr = NULL;
+    memset(handle, 0, sizeof(*handle));
 }
 
 int wolfPSA_Store_Read(void* store, unsigned char* buffer, int len)

src/update_disk.c

@@ -221,6 +221,11 @@ static void disk_crypto_clear(void)
     ForceZero(disk_encrypt_nonce, sizeof(disk_encrypt_nonce));
 }
 
+static void disk_decrypted_header_clear(uint8_t *hdr)
+{
+    ForceZero(hdr, IMAGE_HEADER_SIZE);
+}
+
 #endif /* DISK_ENCRYPT */
 
 extern int wolfBoot_get_dts_size(void *dts_addr);
@@ -267,12 +272,14 @@ 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_decrypted_header_clear(dec_hdr);
         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_decrypted_header_clear(dec_hdr);
         disk_crypto_clear();
         wolfBoot_printf("Error getting encryption key\r\n");
         wolfBoot_panic();
@@ -283,13 +290,15 @@ void RAMFUNCTION wolfBoot_start(void)
     ret = disk_init(BOOT_DISK);
     if (ret != 0) {
 #ifdef DISK_ENCRYPT
+        disk_decrypted_header_clear(dec_hdr);
         disk_crypto_clear();
 #endif
         wolfBoot_panic();
     }
 
     if (disk_open(BOOT_DISK) < 0) {
 #ifdef DISK_ENCRYPT
+        disk_decrypted_header_clear(dec_hdr);
         disk_crypto_clear();
 #endif
         wolfBoot_printf("Error opening disk %d\r\n", BOOT_DISK);
@@ -328,6 +337,7 @@ void RAMFUNCTION wolfBoot_start(void)
 
     if ((pB_ver == 0) && (pA_ver == 0)) {
 #ifdef DISK_ENCRYPT
+        disk_decrypted_header_clear(dec_hdr);
         disk_crypto_clear();
 #endif
         wolfBoot_printf("No valid OS image found in either partition %d or %d\r\n",
@@ -433,6 +443,7 @@ void RAMFUNCTION wolfBoot_start(void)
         wolfBoot_printf("Decrypting image...");
         BENCHMARK_START();
         if ((IMAGE_HEADER_SIZE % ENCRYPT_BLOCK_SIZE) != 0) {
+            disk_decrypted_header_clear(dec_hdr);
             disk_crypto_clear();
             wolfBoot_printf("Encrypted disk images require aligned header size\r\n");
             wolfBoot_panic();
@@ -482,6 +493,7 @@ void RAMFUNCTION wolfBoot_start(void)
 
     if (failures) {
 #ifdef DISK_ENCRYPT
+        disk_decrypted_header_clear(dec_hdr);
         disk_crypto_clear();
 #endif
         wolfBoot_printf("Unable to find a valid partition!\r\n");
@@ -542,6 +554,7 @@ void RAMFUNCTION wolfBoot_start(void)
     wolfBoot_hook_boot(&os_image);
 #endif
 #ifdef DISK_ENCRYPT
+    disk_decrypted_header_clear(dec_hdr);
     disk_crypto_clear();
 #endif
     do_boot((uint32_t*)load_address

src/update_flash.c

@@ -1241,7 +1241,6 @@ int wolfBoot_unlock_disk(void)
             ret = wolfBoot_get_random(secret, secretSz);
             if (ret == 0) {
                 wolfBoot_printf("Creating new secret (%d bytes)\n", secretSz);
-                wolfBoot_print_hexstr(secret, secretSz, 0);
 
                 /* seal new secret */
                 ret = wolfBoot_seal(pubkey_hint, policy, policySz, nvIndex,
@@ -1265,15 +1264,13 @@ int wolfBoot_unlock_disk(void)
                 }
 
                 wolfBoot_printf("Secret Check %d bytes\n", secretCheckSz);
-                wolfBoot_print_hexstr(secretCheck, secretCheckSz, 0);
                 TPM2_ForceZero(secretCheck, sizeof(secretCheck));
             }
         }
     }
 
     if (ret == 0) {
         wolfBoot_printf("Secret %d bytes\n", secretSz);
-        wolfBoot_print_hexstr(secret, secretSz, 0);
 
         /* TODO: Unlock disk */
 

src/x86/ahci.c

@@ -281,7 +281,6 @@ static int sata_create_and_seal_unlock_secret(const uint8_t *pubkey_hint,
     ret = sata_get_random_base64(secret, secret_size);
     if (ret == 0) {
         wolfBoot_printf("Creating new secret (%d bytes)\r\n", *secret_size);
-        wolfBoot_printf("%s\r\n", secret);
 
         /* seal new secret */
         ret = wolfBoot_seal(pubkey_hint, policy, policy_size,
@@ -305,14 +304,11 @@ static int sata_create_and_seal_unlock_secret(const uint8_t *pubkey_hint,
         }
 
         wolfBoot_printf("Secret Check %d bytes\n", secret_check_sz);
-        wolfBoot_printf("%s\r\n", secret_check);
         TPM2_ForceZero(secret_check, sizeof(secret_check));
     }
 
-    if (ret == 0) {
+    if (ret == 0)
         wolfBoot_printf("Secret %d bytes\n", *secret_size);
-        wolfBoot_printf("%s\r\n", secret);
-    }
 
     return ret;
 }
@@ -414,9 +410,6 @@ int sata_unlock_disk(int drv, int freeze)
     r = sata_get_unlock_secret(secret, &secret_size);
     if (r != 0)
         return r;
-#ifdef TARGET_x86_fsp_qemu
-    wolfBoot_printf("DISK LOCK SECRET: %s\r\n", secret);
-#endif
     ata_st = ata_security_get_state(drv);
     wolfBoot_printf("ATA: Security state SEC%d\r\n", ata_st);
 #if defined(TARGET_x86_fsp_qemu)

tools/elf-parser/elf-parser.c

@@ -67,7 +67,9 @@ int main(int argc, char *argv[])
             ret = -1;
         }
     }
-    fclose(f);
+    if (f != NULL) {
+        fclose(f);
+    }
 
     if (ret == 0) {
         ret = elf_load_image_mmu(image, (uint32_t)imageSz, &entry, NULL);

tools/fdt-parser/fdt-parser.c

@@ -192,10 +192,18 @@ static int fdt_test(void* fdt)
     off = fdt_node_offset_by_compatible(fdt, -1, "fsl,qman-portal");
     while (off != -FDT_ERR_NOTFOUND) {
         const int *ci = fdt_getprop(fdt, off, "cell-index", NULL);
+        uint32_t portal_idx;
         uint32_t liodns[2];
         if (!ci)
             break;
-        i = fdt32_to_cpu(*ci);
+        portal_idx = fdt32_to_cpu(*ci);
+        if (portal_idx >= QMAN_NUM_PORTALS) {
+            printf("FDT: Invalid qman-portal cell-index %u at %d\n",
+                portal_idx, off);
+            ret = -FDT_ERR_BADSTRUCTURE;
+            goto exit;
+        }
+        i = (int)portal_idx;
 
         liodns[0] = qp_info[i].dliodn;
         liodns[1] = qp_info[i].fliodn;

tools/tpm/policy_sign.c

@@ -287,7 +287,7 @@ int policy_sign(int argc, char *argv[])
                 pcrDigestSz = -1;
             else
                 pcrDigestSz = hexToByte(hashHexStr, pcrDigest, hashHexStrlen);
-            if (pcrDigestSz <= 0) {
+            if ((int)pcrDigestSz <= 0) {
                 fprintf(stderr, "Invalid PCR hash length\n");
                 usage();
                 return -1;
@@ -300,7 +300,7 @@ int policy_sign(int argc, char *argv[])
                 digestSz = -1;
             else
                 digestSz = hexToByte(hashHexStr, digest, hashHexStrlen);
-            if (digestSz <= 0) {
+            if ((int)digestSz <= 0) {
                 fprintf(stderr, "Invalid Policy Digest hash length\n");
                 usage();
                 return -1;

tools/unit-tests/Makefile

@@ -49,7 +49,7 @@ TESTS:=unit-parser unit-extflash unit-string unit-spi-flash unit-aes128 \
        unit-update-flash-enc unit-update-ram unit-pkcs11_store unit-psa_store unit-disk \
        unit-update-disk unit-multiboot unit-boot-x86-fsp unit-qspi-flash unit-tpm-rsa-exp \
        unit-image-nopart unit-image-sha384 unit-image-sha3-384 unit-store-sbrk \
-       unit-tpm-blob
+       unit-tpm-blob unit-policy-sign
 
 all: $(TESTS)
 
@@ -132,6 +132,12 @@ unit-tpm-blob: ../../include/target.h unit-tpm-blob.c
 		-DWOLFBOOT_HASH_SHA256 \
 		-ffunction-sections -fdata-sections $(LDFLAGS) -Wl,--gc-sections
 
+unit-policy-sign: ../../include/target.h unit-policy-sign.c
+	gcc -o $@ $^ $(CFLAGS) -I../tpm -I$(WOLFBOOT_LIB_WOLFTPM) -DWOLFBOOT_TPM \
+		-DWOLFTPM_USER_SETTINGS -DWOLFBOOT_SIGN_ECC256 -DWOLFBOOT_HASH_SHA256 \
+		-DHAVE_ECC -DHAVE_ECC_KEY_IMPORT \
+		-ffunction-sections -fdata-sections $(LDFLAGS) -Wl,--gc-sections
+
 unit-store-sbrk: unit-store-sbrk.c ../../src/store_sbrk.c
 	gcc -o $@ $^ $(CFLAGS) $(LDFLAGS)
 

tools/unit-tests/unit-pkcs11_store.c

@@ -318,6 +318,39 @@ START_TEST(test_cross_sector_write_preserves_length)
 }
 END_TEST
 
+START_TEST(test_close_clears_handle_state)
+{
+    const int type = DYNAMIC_TYPE_RSA;
+    const CK_ULONG id_tok = 17;
+    const CK_ULONG id_obj = 21;
+    void *store = NULL;
+    struct store_handle *handle;
+    int ret;
+
+    ret = mmap_file("/tmp/wolfboot-unit-keyvault.bin", vault_base,
+            keyvault_size, NULL);
+    ck_assert_int_eq(ret, 0);
+    memset(vault_base, 0xEE, keyvault_size);
+
+    ret = wolfPKCS11_Store_Open(type, id_tok, id_obj, 0, &store);
+    ck_assert_int_eq(ret, 0);
+    ck_assert_ptr_nonnull(store);
+
+    handle = store;
+    ck_assert_ptr_nonnull(handle->buffer);
+    ck_assert_ptr_nonnull(handle->hdr);
+    ck_assert_uint_ne(handle->in_buffer_offset, 0);
+
+    wolfPKCS11_Store_Close(store);
+
+    ck_assert_uint_eq(handle->flags, 0);
+    ck_assert_uint_eq(handle->pos, 0);
+    ck_assert_ptr_null(handle->buffer);
+    ck_assert_ptr_null(handle->hdr);
+    ck_assert_uint_eq(handle->in_buffer_offset, 0);
+}
+END_TEST
+
 START_TEST(test_delete_object_ignores_metadata_prefix)
 {
     const int32_t type = DYNAMIC_TYPE_RSA;
@@ -356,12 +389,15 @@ Suite *wolfboot_suite(void)
 
     TCase* tcase_store_and_load_objs = tcase_create("store_and_load_objs");
     TCase* tcase_cross_sector_write = tcase_create("cross_sector_write");
+    TCase* tcase_close = tcase_create("close_state");
     TCase* tcase_delete_object = tcase_create("delete_object");
     tcase_add_test(tcase_store_and_load_objs, test_store_and_load_objs);
     tcase_add_test(tcase_cross_sector_write, test_cross_sector_write_preserves_length);
+    tcase_add_test(tcase_close, test_close_clears_handle_state);
     tcase_add_test(tcase_delete_object, test_delete_object_ignores_metadata_prefix);
     suite_add_tcase(s, tcase_store_and_load_objs);
     suite_add_tcase(s, tcase_cross_sector_write);
+    suite_add_tcase(s, tcase_close);
     suite_add_tcase(s, tcase_delete_object);
     return s;
 }