From 55e13b526c2da31717d7dd9d4a4576b6dad3efd8 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 13:07:33 +0200 Subject: [PATCH 01/37] F-4782: bound hdr->pos in bitmap_put to prevent OOB write from corrupted vault A power fault during cache_commit(0) can leave a node header in the PSA keyvault with valid magic/tok/obj/type but pos left as erased flash (WOLFPSA_INVALID_ID). find_object_buffer() detects the data-sector mismatch and calls delete_object(), which reaches bitmap_put(0xFFFFFFFF, 0). bitmap_put computed octet = pos/8 and indexed cached_sector[4 + octet] with no bounds check, writing ~512 MB past the static sector buffer. Reject pos >= KEYVAULT_MAX_ITEMS so a corrupted header can no longer turn into an out-of-bounds write. This is the psa_store.c sibling of the pkcs11_store.c fix in F-4965. Adds a unit test that seeds a node with pos=WOLFPSA_INVALID_ID and confirms delete_object() no longer faults. --- src/psa_store.c | 6 +++++ tools/unit-tests/unit-psa_store.c | 41 +++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/src/psa_store.c b/src/psa_store.c index 0b8fb6d8c2..3737c8fad0 100644 --- a/src/psa_store.c +++ b/src/psa_store.c @@ -126,6 +126,12 @@ static void bitmap_put(uint32_t pos, int val) uint32_t bit = pos % 8; uint8_t *bitmap = cached_sector + sizeof(uint32_t); + /* Reject out-of-range positions (e.g. a power-fault-corrupted hdr->pos + * left as erased flash) to avoid an out-of-bounds write past the + * bitmap, which lives within cached_sector. */ + if (pos >= KEYVAULT_MAX_ITEMS) + return; + if (val != 0) { bitmap[octet] |= (1 << bit); } else { diff --git a/tools/unit-tests/unit-psa_store.c b/tools/unit-tests/unit-psa_store.c index 432a5bff19..ac2fbfeadc 100644 --- a/tools/unit-tests/unit-psa_store.c +++ b/tools/unit-tests/unit-psa_store.c @@ -154,6 +154,44 @@ START_TEST(test_delete_object_ignores_metadata_prefix) } END_TEST +START_TEST(test_delete_object_corrupted_pos_no_oob) +{ + enum { type = WOLFPSA_STORE_KEY }; + const uint32_t tok_id = 0x0A0B0C0DU; + const uint32_t obj_id = 0x10203040U; + struct obj_hdr *hdr; + int ret; + + ret = mmap_file("/tmp/wolfboot-unit-psa-keyvault.bin", vault_base, + keyvault_size, NULL); + ck_assert_int_eq(ret, 0); + memset(vault_base, 0xFF, keyvault_size); + + /* Valid header magic and zeroed bitmap so check_vault() accepts the + * sector without restoring/reinitializing it. */ + ((uint32_t *)vault_base)[0] = VAULT_HEADER_MAGIC; + memset(vault_base + sizeof(uint32_t), 0x00, BITMAP_SIZE); + + /* Simulate a power-fault-corrupted node: valid tok/obj/type but the + * 'pos' field was never written and is left as erased flash + * (WOLFPSA_INVALID_ID). delete_object() must not turn this into an + * out-of-bounds bitmap_put(0xFFFFFFFF, 0). */ + hdr = NODES_TABLE; + hdr->token_id = tok_id; + hdr->object_id = obj_id; + hdr->type = type; + hdr->pos = WOLFPSA_INVALID_ID; + hdr->size = 2 * sizeof(uint32_t); + + delete_object(type, tok_id, obj_id); + + /* If we get here without a crash, the OOB write was avoided. The node + * should also have been invalidated. */ + ck_assert_uint_eq(NODES_TABLE->token_id, WOLFPSA_INVALID_ID); + ck_assert_uint_eq(NODES_TABLE->object_id, WOLFPSA_INVALID_ID); +} +END_TEST + START_TEST(test_find_object_search_stops_at_header_sector) { enum { type = WOLFPSA_STORE_KEY }; @@ -190,15 +228,18 @@ Suite *wolfboot_suite(void) TCase *tcase_write = tcase_create("cross_sector_write"); TCase *tcase_close = tcase_create("close_state"); TCase *tcase_delete = tcase_create("delete_object"); + TCase *tcase_delete_corrupted = tcase_create("delete_corrupted_pos"); TCase *tcase_find_bounds = tcase_create("find_bounds"); tcase_add_test(tcase_write, test_cross_sector_write_preserves_length); tcase_add_test(tcase_close, test_close_clears_handle_state); tcase_add_test(tcase_delete, test_delete_object_ignores_metadata_prefix); + tcase_add_test(tcase_delete_corrupted, test_delete_object_corrupted_pos_no_oob); tcase_add_test(tcase_find_bounds, test_find_object_search_stops_at_header_sector); suite_add_tcase(s, tcase_write); suite_add_tcase(s, tcase_close); suite_add_tcase(s, tcase_delete); + suite_add_tcase(s, tcase_delete_corrupted); suite_add_tcase(s, tcase_find_bounds); return s; } From b3cfb9ef815e4b5c6df8ad1fc415bdc99b48f5cf Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 13:12:24 +0200 Subject: [PATCH 02/37] F-4717: reject PCI BARs whose region overruns the pool to prevent cursor wrap pci_enum_next_aligned32 only validates the aligned start address against the pool limit, not the end of the allocated region. A malformed PCI device advertising a 2GB non-prefetchable 32-bit MMIO BAR (bar_align 0x80000000, length 0x80000000) gets an aligned start of 0x80000000 that passes the start-only limit check, but the cursor update *base = bar_value + length wraps to 0 in uint32_t. The wrapped cursor lands subsequent BARs at address 0 (IVT/BIOS region) and makes pci_program_bridge program a spurious 2GB MMIO window via (info->mem - 1) >> 16 = 0xFFFF. Add an explicit check in pci_program_bar that the whole region [bar_value, bar_value + length) fits below the pool limit without overflowing 32 bits, mirroring the guards already present for the zero-align and oversized-IO cases. Add a unit test that fails before the fix (the oversized BAR is accepted and the cursor wraps to 0). --- src/pci.c | 12 ++++++++ tools/unit-tests/unit-pci.c | 55 +++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/src/pci.c b/src/pci.c index 6482b7a2c2..86f47a595a 100644 --- a/src/pci.c +++ b/src/pci.c @@ -510,6 +510,18 @@ static int pci_program_bar(uint8_t bus, uint8_t dev, uint8_t fun, goto restore_bar; } + /* pci_enum_next_aligned32 only validates the aligned start address against + * the pool limit, not the end of the region. A malformed BAR advertising a + * huge length (e.g. a 2GB MMIO BAR with bar_align == 0x80000000) can pass + * that check at a start that leaves [bar_value, bar_value + length) running + * past the pool, wrapping the 32-bit cursor below to 0. Reject it here so + * the whole region fits under the limit without overflowing. */ + if ((uint64_t)bar_value + (uint64_t)length > (uint64_t)limit) { + PCI_DEBUG_PRINTF("PCI device region does not fit in the pool... skipping\r\n"); + ret = -1; + goto restore_bar; + } + pci_config_write32(bus, dev, fun, bar_off, bar_value); if (*is_64bit) pci_config_write32(bus, dev, fun, bar_off + 4, 0x0); diff --git a/tools/unit-tests/unit-pci.c b/tools/unit-tests/unit-pci.c index 1ff4021289..9d4cc222ec 100644 --- a/tools/unit-tests/unit-pci.c +++ b/tools/unit-tests/unit-pci.c @@ -1000,6 +1000,57 @@ START_TEST(test_program_bar_no_space) } END_TEST +/* test_program_bar_2gb_no_wrap: a device advertising a 2GB non-prefetchable + * 32-bit MMIO BAR (bar_align == 0x80000000, length == 0x80000000) must be + * rejected. The aligned start 0x80000000 passes the start-only limit check in + * pci_enum_next_aligned32, but [start, start+length) overruns the 128MB pool + * and the cursor update *base = start + length wraps to 0 in uint32_t. That + * wrap collides every subsequent BAR onto the low IVT/BIOS region and + * mis-programs the bridge MMIO window to a spurious 2GB range. The oversized + * BAR must be skipped and the allocator left untouched. */ +START_TEST(test_program_bar_2gb_no_wrap) +{ + struct test_pci_topology t; + struct pci_enum_info info; + uint8_t is_64bit = 0; + int dev_node; + int ret; + uint32_t bar0_val; + + test_pci_init(&t); + dev_node = test_pci_add_dev(&t, 0, 0, 0x1234, 0x5678, TEST_PCI_ROOT_BUS); + /* 2GB 32-bit non-prefetchable MMIO BAR */ + test_pci_dev_set_bar(&t, dev_node, 0, 0x80000000, TEST_PCI_BAR_MMIO); + test_pci_commit(&t); + + /* Pre-fill BAR0 so we can confirm it is restored, not programmed. */ + { + uint32_t orig = 0xBEEF0000; + memcpy(&t.nodes[dev_node].cfg[PCI_BAR0_OFFSET], &orig, 4); + } + + memset(&info, 0, sizeof(info)); + info.mem = 0x80000000; + info.mem_limit = 0x88000000; + info.mem_pf = 0x90000000; + info.mem_pf_limit = 0xFFFFFFFF; + info.io = 0x2000; + + ret = pci_program_bar(0, 0, 0, 0, &info, &is_64bit); + /* the oversized BAR must not be accepted */ + ck_assert_int_ne(ret, 0); + + /* the allocator cursor must NOT have wrapped to 0 */ + ck_assert_uint_eq(info.mem, 0x80000000); + + /* BAR0 restored, never programmed onto the MMIO window */ + bar0_val = pci_config_read32(0, 0, 0, PCI_BAR0_OFFSET); + ck_assert_uint_eq(bar0_val, 0xBEEF0000); + + test_pci_cleanup(&t); +} +END_TEST + /* test_program_bars_iteration: full BAR iteration with mixed types */ START_TEST(test_program_bars_iteration) { @@ -1786,6 +1837,10 @@ Suite *wolfboot_suite(void) tcase_add_test(tc_bar_nospace, test_program_bar_no_space); suite_add_tcase(s, tc_bar_nospace); + TCase *tc_bar_2gb = tcase_create("program-bar-2gb-no-wrap"); + tcase_add_test(tc_bar_2gb, test_program_bar_2gb_no_wrap); + suite_add_tcase(s, tc_bar_2gb); + TCase *tc_bars_iter = tcase_create("program-bars-iteration"); tcase_add_test(tc_bars_iter, test_program_bars_iteration); suite_add_tcase(s, tc_bars_iter); From 306a30433cacbd7a8fb3e2266fa49290778ec16b Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 13:15:09 +0200 Subject: [PATCH 03/37] F-4716: reject SDHCI block addresses that overflow 32-bit SD command argument disk_read and disk_write divided the 64-bit byte offset by the block size into a uint32_t block_addr, silently truncating addresses at or above 2 TB (LBA >= 2^32). A crafted GPT partition whose first LBA is >= 2^32 passes the 64-bit bounds check in disk_part_read but wraps the SD command argument, redirecting reads/writes to an unintended in-range sector (e.g. sector 0). The SD/SDHC command argument register (SDHCI_SRS02) is inherently 32-bit, so such addresses are not hardware-addressable; reject them instead of wrapping. --- src/sdhci.c | 16 ++++++++-- tools/unit-tests/unit-sdhci-disk-unaligned.c | 33 ++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/sdhci.c b/src/sdhci.c index f54161d027..f474184d0a 100644 --- a/src/sdhci.c +++ b/src/sdhci.c @@ -1586,7 +1586,13 @@ int disk_read(int drv, uint64_t start, uint32_t count, uint8_t *buf) #endif while (count > 0) { - block_addr = (start / SDHCI_BLOCK_SIZE); + uint64_t block_addr64 = (start / SDHCI_BLOCK_SIZE); + /* SD/SDHC command argument is 32-bit: reject addresses that would + * otherwise be silently truncated and wrap to an in-range sector. */ + if (block_addr64 > 0xFFFFFFFFUL) { + return -1; + } + block_addr = (uint32_t)block_addr64; read_sz = count; if (read_sz > SDHCI_BLOCK_SIZE) { read_sz = SDHCI_BLOCK_SIZE; @@ -1641,7 +1647,13 @@ int disk_write(int drv, uint64_t start, uint32_t count, const uint8_t *buf) #endif while (count > 0) { - block_addr = (start / SDHCI_BLOCK_SIZE); + uint64_t block_addr64 = (start / SDHCI_BLOCK_SIZE); + /* SD/SDHC command argument is 32-bit: reject addresses that would + * otherwise be silently truncated and wrap to an in-range sector. */ + if (block_addr64 > 0xFFFFFFFFUL) { + return -1; + } + block_addr = (uint32_t)block_addr64; write_sz = count; if (write_sz > SDHCI_BLOCK_SIZE) { write_sz = SDHCI_BLOCK_SIZE; diff --git a/tools/unit-tests/unit-sdhci-disk-unaligned.c b/tools/unit-tests/unit-sdhci-disk-unaligned.c index bd306c61b1..232fa62bb0 100644 --- a/tools/unit-tests/unit-sdhci-disk-unaligned.c +++ b/tools/unit-tests/unit-sdhci-disk-unaligned.c @@ -168,6 +168,37 @@ START_TEST(test_disk_write_unaligned_spans_blocks) } END_TEST +/* A byte offset whose block address (offset / 512) does not fit in the 32-bit + * SD command argument must be rejected, not silently truncated/wrapped to an + * in-range sector. Here offset / 512 == 0x100000000, which truncates to 0. */ +START_TEST(test_disk_read_rejects_block_addr_overflow) +{ + uint8_t out[SDHCI_BLOCK_SIZE]; + uint64_t start = (uint64_t)0x100000000ULL * SDHCI_BLOCK_SIZE; + + reset_mock_state(); + + /* Must fail rather than wrap and read sector 0 (mock_disk[0..]). */ + ck_assert_int_ne(disk_read(0, start, sizeof(out), out), 0); +} +END_TEST + +START_TEST(test_disk_write_rejects_block_addr_overflow) +{ + uint8_t in[SDHCI_BLOCK_SIZE]; + uint8_t before[sizeof(mock_disk)]; + uint64_t start = (uint64_t)0x100000000ULL * SDHCI_BLOCK_SIZE; + + reset_mock_state(); + memcpy(before, mock_disk, sizeof(before)); + memset(in, 0x5A, sizeof(in)); + + /* Must fail rather than wrap and overwrite sector 0. */ + ck_assert_int_ne(disk_write(0, start, sizeof(in), in), 0); + ck_assert_mem_eq(mock_disk, before, sizeof(before)); +} +END_TEST + Suite *sdhci_disk_suite(void) { Suite *s = suite_create("sdhci-disk"); @@ -175,6 +206,8 @@ Suite *sdhci_disk_suite(void) tcase_add_test(tc, test_disk_read_unaligned_spans_blocks); tcase_add_test(tc, test_disk_write_unaligned_spans_blocks); + tcase_add_test(tc, test_disk_read_rejects_block_addr_overflow); + tcase_add_test(tc, test_disk_write_rejects_block_addr_overflow); suite_add_tcase(s, tc); return s; From e0f271bfd513a4727479cddb66486783f6d50a50 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 13:19:48 +0200 Subject: [PATCH 04/37] F-4715: bound GPT partition-entry array size before CRC scan in disk_open disk_open() computed bytes_left = n_part * array_sz from the GPT header and scanned the whole declared partition-entry array (one disk_read per 512-byte chunk) to compute its CRC32 *before* comparing against ptable.part_crc. Both n_part and array_sz are taken verbatim from the GPT header, whose only gate is a header CRC32 the attacker can freely recompute. A crafted header with e.g. n_part=0xFFFFFFFF forces ~10^9 disk reads before the mismatch is detected: a pre-auth denial of service that can trip a watchdog and block boot. Reject the header when n_part * array_sz exceeds GPT_MAX_PART_ENTRIES (128, the UEFI default) * GPT_PART_ENTRY_SIZE before entering the scan loop. The bound is generous enough for any standard table (128 * 128 = 16 KiB) and for the existing oversized-array test cases, but caps the scan at 64 sectors. Add a regression test that crafts a header with a valid header CRC and an 8 MB declared array and asserts disk_open performs no partition-array reads. --- include/gpt.h | 4 ++++ src/disk.c | 13 +++++++++++++ tools/unit-tests/unit-disk.c | 29 +++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/include/gpt.h b/include/gpt.h index 5e74ce0709..a0936492c1 100644 --- a/include/gpt.h +++ b/include/gpt.h @@ -35,6 +35,10 @@ #define GPT_MBR_BOOTSIG_OFFSET 0x01FE #define GPT_MBR_BOOTSIG_VALUE 0xAA55 #define GPT_PART_ENTRY_SIZE 256 +/* UEFI reserves 128 partition entries by default; bound the partition-entry + * array (n_part * array_sz) used by the CRC scan to this many entries so a + * crafted header cannot force an unbounded number of disk reads. */ +#define GPT_MAX_PART_ENTRIES 128 /** * @brief MBR partition table entry structure. diff --git a/src/disk.c b/src/disk.c index 782c002f77..052c51a8c6 100644 --- a/src/disk.c +++ b/src/disk.c @@ -169,6 +169,19 @@ int disk_open(int drv) array_addr = ptable.start_array * GPT_SECTOR_SIZE; bytes_left = (uint64_t)ptable.n_part * ptable.array_sz; + /* Reject an implausibly large partition-entry array before scanning + * it to compute the CRC. n_part and array_sz come straight from the + * (attacker-craftable) GPT header whose only gate is a recomputable + * CRC32, so without this bound a header with e.g. n_part=0xFFFFFFFF + * forces ~10^9 disk reads before the part_crc mismatch is caught: a + * pre-auth denial of service. */ + if (bytes_left > + (uint64_t)GPT_MAX_PART_ENTRIES * GPT_PART_ENTRY_SIZE) { + wolfBoot_printf("GPT partition entry array too large\r\n"); + Drives[drv].is_open = 0; + return -1; + } + gpt_crc32_init(&part_crc); while (bytes_left > 0) { chunk = GPT_SECTOR_SIZE; diff --git a/tools/unit-tests/unit-disk.c b/tools/unit-tests/unit-disk.c index 40dd08741f..e0ee504a1f 100644 --- a/tools/unit-tests/unit-disk.c +++ b/tools/unit-tests/unit-disk.c @@ -35,10 +35,14 @@ static uint8_t fake_disk[FAKE_DISK_SIZE]; /* Set to a byte offset to make disk_read fail at that address. -1 = no fail */ static int64_t mock_disk_read_fail_at = -1; +/* Number of disk_read calls since last reset (used to detect scan blow-ups) */ +static unsigned int disk_read_count = 0; + /* Mock disk I/O — copies to/from fake_disk buffer */ int disk_read(int drv, uint64_t start, uint32_t count, uint8_t *buf) { (void)drv; + disk_read_count++; if (mock_disk_read_fail_at >= 0 && (int64_t)start == mock_disk_read_fail_at) return -1; if (start + count > FAKE_DISK_SIZE) @@ -625,6 +629,30 @@ START_TEST(test_disk_open_gpt_large_array_sz) } END_TEST +START_TEST(test_disk_open_gpt_rejects_huge_part_array) +{ + /* A crafted GPT header with a valid (recomputed) header CRC but an + * enormous n_part * array_sz must be rejected before the partition-entry + * CRC scan loop runs, otherwise it forces a pre-auth DoS via one disk + * read per 512-byte chunk of the (here 8 MB) declared array. */ + struct guid_ptable *gpt_hdr; + + build_gpt_disk(); + + gpt_hdr = (struct guid_ptable *)(fake_disk + GPT_SECTOR_SIZE); + gpt_hdr->n_part = 0x10000; /* 65536 entries ... */ + gpt_hdr->array_sz = 128; /* ... * 128 bytes = 8 MB */ + finalize_gpt_header_crc(gpt_hdr); /* attacker can always fix header CRC */ + + disk_read_count = 0; + ck_assert_int_eq(disk_open(0), -1); + /* Only the MBR sector and the GPT header sector may be read; the + * partition-array scan must not run at all. */ + ck_assert_uint_le(disk_read_count, 2); + ck_assert_int_eq(Drives[0].is_open, 0); +} +END_TEST + START_TEST(test_disk_open_gpt_empty_entry_mid_table) { /* GPT header says 3 partitions but entry[1] has zeroed type GUID. @@ -961,6 +989,7 @@ Suite *wolfboot_suite(void) tcase_add_test(tc_cov, test_disk_open_mbr_bad_bootsig); tcase_add_test(tc_cov, test_disk_open_gpt_excess_partitions); tcase_add_test(tc_cov, test_disk_open_gpt_large_array_sz); + tcase_add_test(tc_cov, test_disk_open_gpt_rejects_huge_part_array); tcase_add_test(tc_cov, test_disk_open_gpt_empty_entry_mid_table); tcase_add_test(tc_cov, test_disk_open_mbr_zero_lba_entry); tcase_add_test(tc_cov, test_open_part_invalid_drive); From 93a8fe8d5a6713097f2b8bb08b69b6f0f5bafa1f Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 13:21:56 +0200 Subject: [PATCH 05/37] F-4714: compute GPT header offset in 64 bits to prevent uint32_t overflow The protective-MBR lba_first field (attacker-controlled uint32_t, no range check) was multiplied by GPT_SECTOR_SIZE (int 512) in disk_open. C's usual arithmetic conversions made the product a 32-bit unsigned, which wraps for gpt_lba >= 0x800000 (512 * 0x800001 -> 0x200), silently redirecting the GPT header read back to LBA 1 instead of the out-of-range LBA named. Cast the constant to uint64_t so the byte offset is computed in 64 bits. Add a unit test that points lba_first at an overflowing LBA and confirms disk_open now rejects it. --- src/disk.c | 3 ++- tools/unit-tests/unit-disk.c | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/disk.c b/src/disk.c index 052c51a8c6..6b9cb5c22b 100644 --- a/src/disk.c +++ b/src/disk.c @@ -149,7 +149,8 @@ int disk_open(int drv) wolfBoot_printf("Found GPT PTE at sector %u\r\n", gpt_lba); /* Read GPT header */ - r = disk_read(drv, GPT_SECTOR_SIZE * gpt_lba, GPT_SECTOR_SIZE, sector); + r = disk_read(drv, (uint64_t)GPT_SECTOR_SIZE * gpt_lba, GPT_SECTOR_SIZE, + sector); if (r < 0) { wolfBoot_printf("Disk read failed\r\n"); Drives[drv].is_open = 0; diff --git a/tools/unit-tests/unit-disk.c b/tools/unit-tests/unit-disk.c index e0ee504a1f..8584069efa 100644 --- a/tools/unit-tests/unit-disk.c +++ b/tools/unit-tests/unit-disk.c @@ -653,6 +653,31 @@ START_TEST(test_disk_open_gpt_rejects_huge_part_array) } END_TEST +START_TEST(test_disk_open_gpt_lba_no_overflow) +{ + /* The protective-MBR lba_first is an attacker-controlled uint32_t. The + * GPT-header byte offset must be computed in 64 bits. If it is computed + * as the 32-bit product GPT_SECTOR_SIZE * gpt_lba, it wraps for + * gpt_lba >= 0x800000: 512 * 0x800001 wraps to 0x200, silently + * redirecting the read back to LBA 1 (the real GPT header) instead of + * the out-of-range LBA the field actually names. */ + struct gpt_mbr_part_entry *mbr_entry; + + build_gpt_disk(); + + /* Point the protective entry at an LBA whose 512* product overflows a + * 32-bit unsigned back to 0x200 (LBA 1). */ + mbr_entry = (struct gpt_mbr_part_entry *)(fake_disk + GPT_MBR_ENTRY_START); + mbr_entry->lba_first = 0x800001; + + /* With correct 64-bit arithmetic the header read targets byte + * 0x100000200, far past the fake disk, so disk_open must fail rather + * than wrap to LBA 1 and accept the table. */ + ck_assert_int_eq(disk_open(0), -1); + ck_assert_int_eq(Drives[0].is_open, 0); +} +END_TEST + START_TEST(test_disk_open_gpt_empty_entry_mid_table) { /* GPT header says 3 partitions but entry[1] has zeroed type GUID. @@ -990,6 +1015,7 @@ Suite *wolfboot_suite(void) tcase_add_test(tc_cov, test_disk_open_gpt_excess_partitions); tcase_add_test(tc_cov, test_disk_open_gpt_large_array_sz); tcase_add_test(tc_cov, test_disk_open_gpt_rejects_huge_part_array); + tcase_add_test(tc_cov, test_disk_open_gpt_lba_no_overflow); tcase_add_test(tc_cov, test_disk_open_gpt_empty_entry_mid_table); tcase_add_test(tc_cov, test_disk_open_mbr_zero_lba_entry); tcase_add_test(tc_cov, test_open_part_invalid_drive); From 0f01a9d831a58b3b159badf0000cdfabab4bcac7 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 13:29:16 +0200 Subject: [PATCH 06/37] F-4692: guard squashelf segment-offset alignment against integer overflow The alignment round-up current_offset = (current_offset + p_align - 1) & ~(p_align - 1) used p_align straight from a (possibly crafted) program header without bounds. For ELF64 a p_align near UINT64_MAX wraps the sum to a tiny value (e.g. 0x78 -> 0); for ELF32 a value that rounds the offset past 2^32 is silently truncated when stored into the uint32_t offset field. Either way segment data is written at a wrong (often zero) file offset, clobbering the output ELF header while squashelf still reports success. Reject such inputs: bound the round-up against UINT64_MAX before applying it (both classes) and reject an ELF32 offset that exceeds UINT32_MAX. Normal page-aligned segments are unaffected. Adds test-align-overflow.py covering the ELF64 wrap, the ELF32 truncation, and the normal-segment regression. --- .gitignore | 1 + tools/squashelf/Makefile | 1 + tools/squashelf/squashelf.c | 33 +++++ tools/squashelf/test-align-overflow.py | 160 +++++++++++++++++++++++++ 4 files changed, 195 insertions(+) create mode 100644 tools/squashelf/test-align-overflow.py diff --git a/.gitignore b/.gitignore index 539fdcd240..c215a1e7db 100644 --- a/.gitignore +++ b/.gitignore @@ -207,6 +207,7 @@ tools/squashelf/** !tools/squashelf/Makefile !tools/squashelf/README.md !tools/squashelf/test-range-overflow.py +!tools/squashelf/test-align-overflow.py # Generated configuration files .config diff --git a/tools/squashelf/Makefile b/tools/squashelf/Makefile index cb6ec5b505..f7842c6f85 100644 --- a/tools/squashelf/Makefile +++ b/tools/squashelf/Makefile @@ -27,6 +27,7 @@ debug: all test: $(TARGET) python3 test-range-overflow.py ./$(TARGET) + python3 test-align-overflow.py ./$(TARGET) $(TARGET): $(TARGET).o @echo "Building squashelf tool" diff --git a/tools/squashelf/squashelf.c b/tools/squashelf/squashelf.c index cffb2e12c3..c8f676d881 100644 --- a/tools/squashelf/squashelf.c +++ b/tools/squashelf/squashelf.c @@ -752,10 +752,31 @@ int main(int argCount, char** argValues) /* Align the segment according to its alignment requirement if * needed */ if (p_align > 1) { + /* Guard against overflow in the round-up (CWE-190). p_align + * comes straight from a possibly crafted program header; a + * large value would wrap current_offset to a small value and + * place this segment's data over the ELF header. */ + if (current_offset > UINT64_MAX - (p_align - 1)) { + fprintf(stderr, + "Segment %zu alignment 0x%lx overflows file " + "offset 0x%lx\n", + i, (unsigned long)p_align, + (unsigned long)current_offset); + goto cleanup; + } current_offset = (current_offset + p_align - 1) & ~(p_align - 1); } + /* The 32-bit program header offset is a uint32_t; a larger value + * would be silently truncated and corrupt the output layout. */ + if (current_offset > UINT32_MAX) { + fprintf(stderr, + "Segment %zu offset 0x%lx exceeds 32-bit ELF range\n", + i, (unsigned long)current_offset); + goto cleanup; + } + /* Update the segment's offset */ ph32_array[i].offset = current_offset; p_filesz = ph32_array[i].file_size; @@ -770,6 +791,18 @@ int main(int argCount, char** argValues) /* Align the segment according to its alignment requirement if * needed */ if (p_align > 1) { + /* Guard against overflow in the round-up (CWE-190). p_align + * comes straight from a possibly crafted program header; a + * value near UINT64_MAX would wrap current_offset to a small + * value and place this segment's data over the ELF header. */ + if (current_offset > UINT64_MAX - (p_align - 1)) { + fprintf(stderr, + "Segment %zu alignment 0x%lx overflows file " + "offset 0x%lx\n", + i, (unsigned long)p_align, + (unsigned long)current_offset); + goto cleanup; + } current_offset = (current_offset + p_align - 1) & ~(p_align - 1); } diff --git a/tools/squashelf/test-align-overflow.py b/tools/squashelf/test-align-overflow.py new file mode 100644 index 0000000000..d7386c40ee --- /dev/null +++ b/tools/squashelf/test-align-overflow.py @@ -0,0 +1,160 @@ +#!/usr/bin/env python3 +# test-align-overflow.py +# +# Regression test for the integer overflow in squashelf's segment-offset +# alignment round-up (current_offset = (current_offset + p_align - 1) & +# ~(p_align - 1)). p_align comes straight from a possibly crafted program +# header. A value near UINT64_MAX (ELF64) wraps the sum to a tiny value, and a +# value that rounds the offset past 2^32 (ELF32) is silently truncated to a +# uint32_t. Either way the segment data lands at a wrong (often zero) file +# offset, clobbering the ELF header while squashelf still reports success. +# After the fix squashelf must reject such inputs with a non-zero exit instead +# of writing a corrupt output ELF. +# +# Copyright (C) 2026 wolfSSL Inc. +# +# This file is part of wolfBoot. +# +# wolfBoot is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# wolfBoot is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +import os +import struct +import subprocess +import sys +import tempfile + +EHSIZE64 = 64 +PHSIZE64 = 56 +EHSIZE32 = 52 +PHSIZE32 = 32 +UINT64_MAX = 0xFFFFFFFFFFFFFFFF +UINT32_MAX = 0xFFFFFFFF + + +def make_elf64(path, align, filesz=0x10): + ph_off = EHSIZE64 + seg_off = ph_off + PHSIZE64 + ident = b"\x7fELF" + bytes([2, 1, 1, 0]) + b"\x00" * 8 # ELF64, little-endian + ehdr = ident + struct.pack( + " 1 else "./squashelf" + rc = 0 + with tempfile.TemporaryDirectory() as d: + # 1) ELF64 alignment near UINT64_MAX wraps the round-up to a tiny + # offset. squashelf MUST reject it (non-zero exit), not write a + # corrupt ELF over its own header and report success. + bad64 = os.path.join(d, "align64.elf") + out64 = os.path.join(d, "align64.out") + make_elf64(bad64, align=UINT64_MAX) + if run(squashelf, bad64, out64) == 0: + print("FAIL: ELF64 overflowing alignment was accepted") + rc = 1 + else: + print("PASS: ELF64 overflowing alignment rejected") + + # 2) ELF32 alignment that rounds the offset past 2^32; the uint32_t + # offset field would silently truncate (to 0 here). Must be rejected. + bad32 = os.path.join(d, "align32.elf") + out32 = os.path.join(d, "align32.out") + make_elf32(bad32, align=UINT32_MAX) + if run(squashelf, bad32, out32) == 0: + print("FAIL: ELF32 truncating alignment was accepted") + rc = 1 + else: + print("PASS: ELF32 truncating alignment rejected") + + # 3) Regression guard: normal page-aligned segments must still succeed. + ok64 = os.path.join(d, "ok64.elf") + ok64o = os.path.join(d, "ok64.out") + make_elf64(ok64, align=0x1000) + if run(squashelf, ok64, ok64o) != 0: + print("FAIL: normal ELF64 segment was rejected") + rc = 1 + else: + print("PASS: normal ELF64 segment kept") + + ok32 = os.path.join(d, "ok32.elf") + ok32o = os.path.join(d, "ok32.out") + make_elf32(ok32, align=0x1000) + if run(squashelf, ok32, ok32o) != 0: + print("FAIL: normal ELF32 segment was rejected") + rc = 1 + else: + print("PASS: normal ELF32 segment kept") + + sys.exit(rc) + + +if __name__ == "__main__": + main() From 6a60ea3d53ce0f353c6a6ba6fcf4d997018b7350 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 13:32:27 +0200 Subject: [PATCH 07/37] F-4651: bound multiboot2 header_length to prevent OOB tag walk mb2_build_boot_info_header only rejected header_length below the size of struct mb2_header; an oversized value (e.g. 0xFFFFFFFF) made tags_len wrap to ~4GB so mb2_find_tag_by_type's end pointer was inflated far past the image, defeating its size guards and allowing an out-of-bounds read on 64-bit builds. Per the Multiboot2 spec the header must lie within the first 32 KiB of the image, so reject header_length > MB2_HEADER_MAX_OFF. --- src/multiboot.c | 7 +++++- tools/unit-tests/unit-multiboot.c | 37 +++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/multiboot.c b/src/multiboot.c index b618c76cb7..9d3677be70 100644 --- a/src/multiboot.c +++ b/src/multiboot.c @@ -265,7 +265,12 @@ int mb2_build_boot_info_header(uint8_t *mb2_boot_info, idx = (uint8_t*)hdr + sizeof(*hdr); hdr->reserved = 0; header_length = ((struct mb2_header *)mb2_header)->header_length; - if (header_length < sizeof(struct mb2_header)) + /* Per the Multiboot2 spec the whole header must lie within the first + * MB2_HEADER_MAX_OFF bytes of the image. Bounding header_length here keeps + * the tag walker inside the header window and prevents an oversized value + * (e.g. 0xFFFFFFFF) from inflating its end pointer into an OOB read. */ + if (header_length < sizeof(struct mb2_header) || + header_length > MB2_HEADER_MAX_OFF) return -1; info_req_tag = (struct mb2_tag_info_req *)mb2_find_tag_by_type( diff --git a/tools/unit-tests/unit-multiboot.c b/tools/unit-tests/unit-multiboot.c index f63d7e1180..40b365218b 100644 --- a/tools/unit-tests/unit-multiboot.c +++ b/tools/unit-tests/unit-multiboot.c @@ -424,6 +424,42 @@ START_TEST(test_build_info_header_length_underflow) } END_TEST +/* check that an attacker-controlled header_length larger than the 32 KiB + * Multiboot2 header window cannot inflate the tag walker's bounds. With + * header_length = 0xFFFFFFFF the subtraction header_length - sizeof(struct + * mb2_header) yields ~4GB, so end = tags + ~4GB in mb2_find_tag_by_type and the + * size guard becomes ineffective, letting the walker read far past the image. + * + * We place a fake info_req tag right after the header. Without the upper-bound + * fix mb2_find_tag_by_type finds it and the function returns 0 (success). With + * the fix the oversized header_length is rejected and it returns -1. */ +START_TEST(test_build_info_header_length_overflow) +{ + uint8_t header[48] __attribute__((aligned(8))); + uint8_t boot_info[256]; + struct stage2_parameter p = make_stage2(); + struct mb2_header *h = (struct mb2_header *)header; + struct mb2_tag_info_req *info; + struct mb2_tag *term; + memset(header, 0, sizeof(header)); + h->magic = MB2_MAGIC; + h->header_length = 0xFFFFFFFFu; /* far larger than the 32 KiB window */ + + info = (struct mb2_tag_info_req *)(header + sizeof(struct mb2_header)); + info->type = 1; /* MB2_TAG_TYPE_INFO_REQ */ + info->flags = 0; + info->size = 12; + info->mbi_tag_types[0] = 4; /* MB2_REQ_TAG_BASIC_MEM_INFO */ + + term = (struct mb2_tag *)(header + 32); + term->type = 0; term->flags = 0; term->size = 8; + + ck_assert_int_eq( + mb2_build_boot_info_header(boot_info, header, &p, + sizeof(boot_info)), -1); +} +END_TEST + START_TEST(test_dump_header_length_underflow) { uint8_t header[48] __attribute__((aligned(8))); @@ -767,6 +803,7 @@ Suite *wolfboot_suite(void) tcase_add_test(tc_build, test_build_info_unsupported_tag); tcase_add_test(tc_build, test_build_info_malformed_size); tcase_add_test(tc_build, test_build_info_header_length_underflow); + tcase_add_test(tc_build, test_build_info_header_length_overflow); tcase_add_test(tc_build, test_dump_header_length_underflow); suite_add_tcase(s, tc_build); From 71545e96de9955cd46248a0172fbc42f230e6162 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 13:44:10 +0200 Subject: [PATCH 08/37] F-4339: widen uart-flash-server bounds checks to 64-bit to stop uint32_t wrap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit uart_flash_erase/read/write read attacker-controlled address and len as raw 4-byte words from the UART peer and guard the mmap region with `address + len > FIRMWARE_PARTITION_SIZE + SWAP_SIZE`. The addition is done in uint32_t, so a large len (e.g. 0xFFFFFFFF) wraps the sum below 0x21000 and the guard passes, after which the loop walks far past the 0x21000-byte mapping — SIGSEGV on a 64-bit host, or wrapped writes over the firmware header on 32-bit. Compute address + len in uint64_t before the comparison so the sum cannot wrap. Add a pty-driven regression test (test_overflow) and a `make test` target that sends an ERASE with a wrapping address+len and asserts the server survives. --- tools/uart-flash-server/Makefile | 7 +- tools/uart-flash-server/test_overflow.c | 112 ++++++++++++++++++++++++ tools/uart-flash-server/ufserver.c | 6 +- 3 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 tools/uart-flash-server/test_overflow.c diff --git a/tools/uart-flash-server/Makefile b/tools/uart-flash-server/Makefile index fe0b5b1f10..03ccb85534 100644 --- a/tools/uart-flash-server/Makefile +++ b/tools/uart-flash-server/Makefile @@ -16,6 +16,11 @@ $(EXE): $(EXE).o libwolfboot.o libwolfboot.o: ../../src/libwolfboot.c $(Q)$(CC) $(CFLAGS) -c -o $(@) $(^) +test_overflow: test_overflow.c + $(Q)$(CC) -Wall -g -o $@ $^ -lutil + +test: $(EXE) test_overflow + $(Q)./test_overflow ./$(EXE) clean: - $(Q)rm -f *.o $(EXE) + $(Q)rm -f *.o $(EXE) test_overflow diff --git a/tools/uart-flash-server/test_overflow.c b/tools/uart-flash-server/test_overflow.c new file mode 100644 index 0000000000..c0c9bdf1f0 --- /dev/null +++ b/tools/uart-flash-server/test_overflow.c @@ -0,0 +1,112 @@ +/* Regression test for the uint32_t wraparound bounds-check bypass in + * uart_flash_erase()/read()/write() (finding F-4339). + * + * Drives the real ufserver binary over a pseudo-terminal and sends an ERASE + * command with address=0x1000 and len=0xFFFFFFFF. With the buggy uint32_t + * guard, address+len wraps to 0xFFF (< FIRMWARE_PARTITION_SIZE+SWAP_SIZE), the + * guard is bypassed and the erase loop walks far past the 0x21000-byte mmap, + * crashing the server with SIGSEGV. With the 64-bit guard the command is + * rejected and the server stays alive. + * + * Usage: test_overflow ./ufserver + */ +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define CMD_HDR_WOLF 'W' +#define CMD_HDR_ERASE 0x03 + +int main(int argc, char *argv[]) +{ + int master, slave; + char slavename[256]; + pid_t pid; + char tmpl[] = "/tmp/ufserver_fw_XXXXXX"; + int fd; + uint8_t buf[64]; + int i; + + if (argc != 2) { + fprintf(stderr, "usage: %s ./ufserver\n", argv[0]); + return 2; + } + + /* Create a small (non-WOLF) firmware file; mmap_firmware() pads it to + * FIRMWARE_PARTITION_SIZE + SWAP_SIZE (0x21000). */ + fd = mkstemp(tmpl); + if (fd < 0) { perror("mkstemp"); return 2; } + memset(buf, 0xA5, sizeof(buf)); + if (write(fd, buf, sizeof(buf)) != (ssize_t)sizeof(buf)) { perror("write"); return 2; } + close(fd); + + if (openpty(&master, &slave, slavename, NULL, NULL) != 0) { + perror("openpty"); + return 2; + } + + signal(SIGPIPE, SIG_IGN); + + pid = fork(); + if (pid < 0) { perror("fork"); return 2; } + if (pid == 0) { + /* Child: run the server attached to the slave pty. */ + close(master); + close(slave); + fd = open("/dev/null", O_WRONLY); + if (fd >= 0) { dup2(fd, STDOUT_FILENO); } + execl(argv[1], argv[1], tmpl, slavename, (char *)NULL); + perror("execl"); + _exit(127); + } + + /* Parent: drive the server. */ + close(slave); + usleep(200000); /* let the child open and configure the port */ + + /* STX selecting a flash command, then the ERASE opcode. */ + buf[0] = CMD_HDR_WOLF; + buf[1] = CMD_HDR_ERASE; + /* address = 0x00001000 (little-endian raw bytes) */ + buf[2] = 0x00; buf[3] = 0x10; buf[4] = 0x00; buf[5] = 0x00; + /* len = 0xFFFFFFFF -> address+len wraps to 0xFFF in uint32_t */ + buf[6] = 0xFF; buf[7] = 0xFF; buf[8] = 0xFF; buf[9] = 0xFF; + if (write(master, buf, 10) != 10) { perror("write master"); } + + /* Drain ack bytes the server sends back so it never blocks. */ + fcntl(master, F_SETFL, O_NONBLOCK); + + /* Give the server time to process (and crash, if vulnerable). */ + for (i = 0; i < 20; i++) { + uint8_t drain[64]; + (void)read(master, drain, sizeof(drain)); + usleep(50000); + int status; + pid_t r = waitpid(pid, &status, WNOHANG); + if (r == pid) { + unlink(tmpl); + if (WIFSIGNALED(status)) { + fprintf(stderr, "FAIL: server died from signal %d " + "(OOB access from wrapped bounds check)\n", + WTERMSIG(status)); + return 1; + } + fprintf(stderr, "FAIL: server exited unexpectedly (status %d)\n", + status); + return 1; + } + } + + /* Still alive after processing the malicious command: guard held. */ + kill(pid, SIGKILL); + waitpid(pid, NULL, 0); + unlink(tmpl); + printf("PASS: server rejected wrapped address+len and stayed alive\n"); + return 0; +} diff --git a/tools/uart-flash-server/ufserver.c b/tools/uart-flash-server/ufserver.c index b6156ef517..bd15999268 100644 --- a/tools/uart-flash-server/ufserver.c +++ b/tools/uart-flash-server/ufserver.c @@ -280,7 +280,7 @@ static void uart_flash_erase(uint8_t *base, int ud) return; if (read_word(ud,&len) != 4) return; - if (address + len > (FIRMWARE_PARTITION_SIZE + SWAP_SIZE)) + if ((uint64_t)address + (uint64_t)len > (FIRMWARE_PARTITION_SIZE + SWAP_SIZE)) return; if (address < FIRMWARE_PARTITION_SIZE) { printmsg(msgEraseUpdate); @@ -319,7 +319,7 @@ static void uart_flash_read(uint8_t *base, int ud) #if LOG_FLASH_ADDRESS printf("Read @%x\n", address); #endif - if (address + len > (FIRMWARE_PARTITION_SIZE + SWAP_SIZE)) + if ((uint64_t)address + (uint64_t)len > (FIRMWARE_PARTITION_SIZE + SWAP_SIZE)) return; for (i = 0; i < len; i++) { write(ud, base + address + i, 1); @@ -344,7 +344,7 @@ static void uart_flash_write(uint8_t *base, int ud) #if LOG_FLASH_ADDRESS printf("Write @%x\n", address); #endif - if (address + len > (FIRMWARE_PARTITION_SIZE + SWAP_SIZE)) + if ((uint64_t)address + (uint64_t)len > (FIRMWARE_PARTITION_SIZE + SWAP_SIZE)) return; for (i = 0; i < len; i++) { read(ud, base + address + i, 1); From 34231739a5ee2da6030e0ba2fd4ccd05cd445cc7 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 13:46:22 +0200 Subject: [PATCH 09/37] F-4258: reject GPT partitions whose LBA extent overflows uint64_t A crafted GPT entry with pe->last = UINT64_MAX passes the existing geometry guards (first>last, last==0), but the byte-offset conversion ((pe->last + 1) * GPT_SECTOR_SIZE) - 1 wraps to UINT64_MAX. The bogus part->end then defeats the 'start > p->end' bound in disk_part_read(), neutralising partition isolation. Reject any pe->last >= UINT64_MAX / GPT_SECTOR_SIZE before the multiply; since pe->first <= pe->last this also bounds the part->start computation. --- src/gpt.c | 8 ++++++++ tools/unit-tests/unit-disk.c | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/gpt.c b/src/gpt.c index f5b079d03e..b2c7bf588a 100644 --- a/src/gpt.c +++ b/src/gpt.c @@ -205,6 +205,14 @@ int gpt_parse_partition(const uint8_t *entry_data, uint32_t entry_size, if (pe->last == 0) { return -1; } + /* Reject extents whose byte offset would overflow uint64_t: (pe->last + 1) + * must not wrap and (pe->last + 1) * GPT_SECTOR_SIZE must stay representable. + * Without this, pe->last = UINT64_MAX yields part->end = UINT64_MAX, which + * defeats the bounds check in disk_part_read(). pe->first <= pe->last, so + * bounding pe->last also bounds the part->start multiply. */ + if (pe->last >= (UINT64_MAX / GPT_SECTOR_SIZE)) { + return -1; + } /* Extract partition info (convert LBA to byte offsets) */ part->start = pe->first * GPT_SECTOR_SIZE; diff --git a/tools/unit-tests/unit-disk.c b/tools/unit-tests/unit-disk.c index 8584069efa..086c5f9429 100644 --- a/tools/unit-tests/unit-disk.c +++ b/tools/unit-tests/unit-disk.c @@ -563,6 +563,25 @@ START_TEST(test_gpt_parse_partition_last_zero) } END_TEST +START_TEST(test_gpt_parse_partition_last_overflow) +{ + /* pe->last = UINT64_MAX passes the first>last and last==0 guards, but + * (pe->last + 1) * GPT_SECTOR_SIZE - 1 would wrap to UINT64_MAX, defeating + * the bounds check in disk_part_read. Must be rejected. */ + uint8_t entry[128]; + struct gpt_part_entry *pe = (struct gpt_part_entry *)entry; + struct gpt_part_info info; + + memset(entry, 0, sizeof(entry)); + pe->type[0] = 0x0001020304050607ULL; + pe->type[1] = 0x08090A0B0C0D0E0FULL; + pe->first = 1; + pe->last = 0xFFFFFFFFFFFFFFFFULL; + + ck_assert_int_eq(gpt_parse_partition(entry, 128, &info), -1); +} +END_TEST + /* ============================================================ * Coverage tests: disk.c error paths and boundary conditions * ============================================================ */ @@ -1007,6 +1026,7 @@ Suite *wolfboot_suite(void) tcase_add_test(tc_disk_bugs, test_gpt_partition_end_inclusive); tcase_add_test(tc_disk_bugs, test_disk_open_failure_clears_is_open); tcase_add_test(tc_disk_bugs, test_gpt_parse_partition_last_zero); + tcase_add_test(tc_disk_bugs, test_gpt_parse_partition_last_overflow); suite_add_tcase(s, tc_disk_bugs); TCase *tc_cov = tcase_create("disk-coverage"); From f7e773333e5d2ebe53c1fa9bc16633d54a370e60 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 14:12:14 +0200 Subject: [PATCH 10/37] F-5747: guard fdt_data_size_ and fdt_splice_string_ against off_dt_strings+size_dt_strings uint32_t overflow fdt_data_size_() added the two FDT header uint32_t fields without overflow protection; a crafted FDT with off_dt_strings+size_dt_strings>=2^32 caused the sum to wrap to zero. On 32-bit MMU targets (Cortex-M, RV32, PPC32) the pointer arithmetic in fdt_splice_string_ and fdt_find_add_string_ also wraps, placing 'p' and 'new' at the start of the FDT buffer. All bounds checks in fdt_splice_ then pass (p==end==fdt, oldlen==0, newlen (uint64_t)UINT32_MAX) + return -FDT_ERR_BADOFFSET; + return (int)(off + sz); } static const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int len) @@ -329,7 +333,13 @@ static int fdt_resize_property_(void *fdt, int nodeoffset, const char *name, static int fdt_splice_string_(void *fdt, int newlen) { int err; - void *p = (char*)fdt + fdt_off_dt_strings(fdt) + fdt_size_dt_strings(fdt); + uint32_t off = fdt_off_dt_strings(fdt); + uint32_t sz = fdt_size_dt_strings(fdt); + void *p; + + if (sz > UINT32_MAX - off) + return -FDT_ERR_BADOFFSET; + p = (char*)fdt + off + sz; if ((err = fdt_splice_(fdt, p, 0, newlen))) { return err; @@ -794,8 +804,10 @@ int fdt_del_node(void *fdt, int nodeoffset) /* adjust the actual total size in the FDT header */ int fdt_shrink(void* fdt) { - uint32_t total_size = fdt_data_size_(fdt); - return fdt_set_totalsize(fdt, total_size); + int total_size = fdt_data_size_(fdt); + if (total_size < 0) + return total_size; + return fdt_set_totalsize(fdt, (uint32_t)total_size); } /* FTD Fixup API's */ diff --git a/tools/unit-tests/unit-fdt.c b/tools/unit-tests/unit-fdt.c index 96f9e6e235..7ed9424225 100644 --- a/tools/unit-tests/unit-fdt.c +++ b/tools/unit-tests/unit-fdt.c @@ -136,6 +136,27 @@ START_TEST(test_fit_load_image_rejects_oversized_prop_len) } END_TEST +/* off_dt_strings=4, size_dt_strings=0xFFFFFFFC: sum overflows uint32_t to 0. + * Before the fix, fdt_data_size_() returned 0 and fdt_shrink() silently set + * totalsize=0. After the fix fdt_shrink() must return an error and leave + * totalsize unchanged. */ +START_TEST(test_fdt_shrink_rejects_dt_strings_area_overflow) +{ + static uint8_t buf[256]; + int rc; + + memset(buf, 0, sizeof(buf)); + fdt_set_totalsize(buf, sizeof(buf)); + fdt_set_off_dt_strings(buf, 4); + fdt_set_size_dt_strings(buf, 0xFFFFFFFC); + + rc = fdt_shrink(buf); + + ck_assert_int_lt(rc, 0); + ck_assert_uint_eq(fdt_totalsize(buf), sizeof(buf)); +} +END_TEST + static Suite *fdt_suite(void) { Suite *s = suite_create("fdt"); @@ -144,6 +165,7 @@ static Suite *fdt_suite(void) tcase_add_test(tc, test_fdt_get_string_rejects_out_of_range_offset); tcase_add_test(tc, test_fdt_get_string_returns_string_with_valid_offset); tcase_add_test(tc, test_fit_load_image_rejects_oversized_prop_len); + tcase_add_test(tc, test_fdt_shrink_rejects_dt_strings_area_overflow); suite_add_tcase(s, tc); return s; From 2b0fde4b2c307671c7ae7eaaf35b74c38c5b256b Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 14:14:07 +0200 Subject: [PATCH 11/37] F-5745: fix off-by-one in hal_flash_erase skipping last page when len%PAGE_SIZE!=0 The loop condition `p < end_address` excluded the final page whenever len % FLASH_PAGE_SIZE != 0: end_address landed exactly on that page's start offset and the strict less-than guard skipped it. Change to `p <= end_address` to match the intended inclusive-end semantics. Add a unit test (test_erase_unaligned_len_covers_last_page) to the existing unit-flash-erase-h7 harness that verifies len=PAGE_SIZE+1 erases two pages, not one. --- hal/stm32h7.c | 2 +- tools/unit-tests/unit-flash-erase-h7.c | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/hal/stm32h7.c b/hal/stm32h7.c index 71e9ad53c2..eb47d696b9 100644 --- a/hal/stm32h7.c +++ b/hal/stm32h7.c @@ -208,7 +208,7 @@ int RAMFUNCTION hal_flash_erase(uint32_t address, int len) return -1; end_address = (address - FLASHMEM_ADDRESS_SPACE) + len - 1; for (p = (address - FLASHMEM_ADDRESS_SPACE); - p < end_address; + p <= end_address; p += FLASH_PAGE_SIZE) { if (p < FLASH_BANK2_BASE_REL) { diff --git a/tools/unit-tests/unit-flash-erase-h7.c b/tools/unit-tests/unit-flash-erase-h7.c index 44e9a0b65d..c09577cdaf 100644 --- a/tools/unit-tests/unit-flash-erase-h7.c +++ b/tools/unit-tests/unit-flash-erase-h7.c @@ -137,6 +137,21 @@ START_TEST(test_erase_bank1_two_sectors) } END_TEST +/* Regression for F-5745: when len % FLASH_PAGE_SIZE != 0 the final page was + * skipped because end_address = base + len - 1 lands exactly on the start of + * that last page and the strict `p < end_address` guard excludes it. */ +START_TEST(test_erase_unaligned_len_covers_last_page) +{ + reset_mocks(); + /* len = PAGE_SIZE + 1: two pages must be erased (sectors 0 and 1). */ + hal_flash_erase(0x08000000UL, FLASH_PAGE_SIZE + 1); + + ck_assert_int_eq(erase_log_n, 2); + ck_assert_uint_eq(snb_of(erase_cr1[0]), 0); + ck_assert_uint_eq(snb_of(erase_cr1[1]), 1); +} +END_TEST + Suite *flash_erase_suite(void) { Suite *s = suite_create("flash-erase-h7"); @@ -145,6 +160,7 @@ Suite *flash_erase_suite(void) tcase_add_test(tc, test_erase_bank2_two_sectors); tcase_add_test(tc, test_erase_bank2_single_sector); tcase_add_test(tc, test_erase_bank1_two_sectors); + tcase_add_test(tc, test_erase_unaligned_len_covers_last_page); suite_add_tcase(s, tc); return s; From c165905fe274149617785d7d3fc0d71311d9bebf Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 14:16:23 +0200 Subject: [PATCH 12/37] F-5357: zeroize digest and uuid_be stack buffers in hal_uds_derive_key before every return Add hal_uds_zeroize (volatile byte-loop, same pattern as hal_dice_zeroize in the WOLFBOOT_DICE_HW block) and call it to clear the SHA digest and uuid_be stack arrays on both error and success paths, preventing UDS key material and UUID bytes from remaining on the stack after the function returns. --- hal/mcxn.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/hal/mcxn.c b/hal/mcxn.c index b8d8495adb..72d55a3bd6 100644 --- a/hal/mcxn.c +++ b/hal/mcxn.c @@ -71,6 +71,13 @@ #include #include +static NOINLINEFUNCTION void hal_uds_zeroize(void *ptr, size_t len) +{ + volatile uint8_t *p = (volatile uint8_t *)ptr; + while (len-- > 0U) + *p++ = 0U; +} + /* Derive UDS from device UUID for software DICE testing. * NOT secure — UUID is publicly observable. Only enabled with * WOLFBOOT_UDS_UID_FALLBACK_FORTEST. */ @@ -121,8 +128,11 @@ int hal_uds_derive_key(uint8_t *out, size_t out_len) ret = wc_Sha384Final(&hash, digest); wc_Sha384Free(&hash); } - if (ret != 0) + if (ret != 0) { + hal_uds_zeroize(uuid_be, sizeof(uuid_be)); + hal_uds_zeroize(digest, sizeof(digest)); return -1; + } } #elif defined(WOLFBOOT_HASH_SHA256) { @@ -133,14 +143,19 @@ int hal_uds_derive_key(uint8_t *out, size_t out_len) ret = wc_Sha256Final(&hash, digest); wc_Sha256Free(&hash); } - if (ret != 0) + if (ret != 0) { + hal_uds_zeroize(uuid_be, sizeof(uuid_be)); + hal_uds_zeroize(digest, sizeof(digest)); return -1; + } } #endif if (copy_len > out_len) copy_len = out_len; XMEMCPY(out, digest, copy_len); + hal_uds_zeroize(digest, sizeof(digest)); + hal_uds_zeroize(uuid_be, sizeof(uuid_be)); return 0; #endif /* WOLFBOOT_UDS_UID_FALLBACK_FORTEST */ } From d3bd09f42f151f316fb6d63d5fdbcc4de72cf49f Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 14:20:01 +0200 Subject: [PATCH 13/37] F-5351: test wolfBoot_open_self_address rejects bad magic with hdr_ok cleared --- tools/unit-tests/unit-image.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tools/unit-tests/unit-image.c b/tools/unit-tests/unit-image.c index b2efa29610..5407eed590 100644 --- a/tools/unit-tests/unit-image.c +++ b/tools/unit-tests/unit-image.c @@ -883,6 +883,16 @@ START_TEST(test_open_image) ck_assert_int_eq(ret, -1); ck_assert_uint_eq(img.hdr_ok, 0); + /* Self header must reject bad magic and leave hdr_ok cleared */ + memset(self_hdr, 0xFF, sizeof(self_hdr)); + ((uint32_t *)self_hdr)[0] = ~WOLFBOOT_MAGIC; + + memset(&img, 0, sizeof(img)); + ret = wolfBoot_open_self_address(&img, self_hdr, + (uint8_t *)WOLFBOOT_PARTITION_BOOT_ADDRESS); + ck_assert_int_eq(ret, -1); + ck_assert_uint_eq(img.hdr_ok, 0); + /* Self header must reject sizes beyond the partition payload budget */ memset(self_hdr, 0xFF, sizeof(self_hdr)); ((uint32_t *)self_hdr)[0] = WOLFBOOT_MAGIC; From 712f38988f7877cfd34429803b7d05fbc7e6ff2f Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 14:24:39 +0200 Subject: [PATCH 14/37] F-5348: always evaluate keyslot_CT_hint_matches for every slot The short-circuit `(match_id < 0) &&` caused the constant-time comparison to be skipped for all slots after the first match, leaking which trust-anchor hash matched via timing despite the function's comment promising otherwise. Evaluate the CT comparison unconditionally each iteration and update match_id only when no prior match was found. --- src/image.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/image.c b/src/image.c index 41817b7d07..6aba68e7a6 100644 --- a/src/image.c +++ b/src/image.c @@ -2538,10 +2538,11 @@ int keyslot_id_by_sha(const uint8_t *hint) int match_id = -1; for (id = 0; id < keystore_num_pubkeys(); id++) { + int match; key_hash(id, digest); - if ((match_id < 0) && keyslot_CT_hint_matches(digest, hint)) { + match = keyslot_CT_hint_matches(digest, hint); + if (match && (match_id < 0)) match_id = id; - } } return match_id; } From 286581cf18ae3a08ecbb7fc4a6f08ba4f13cbbd3 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 14:39:25 +0200 Subject: [PATCH 15/37] F-5093: fix OOB memset in x86_paging_setup_ptp and non-looping panic Three related defects: - panic() halted with a single hlt instruction (no loop), so any resumable interrupt (LAPIC timer via iretq) caused it to return, allowing callers to continue executing. Add while(1) and declare __attribute__((noreturn)) in both definition and header. - x86_paging_setup_ptp guarded with == WOLFBOOT_PTP_NUM instead of >=, so if the counter ever exceeded that value (after a panic() return) the guard was permanently bypassed. - The ptp pointer was computed before the bounds check, creating an out-of-bounds pointer for one-past-end indices; move the assignment to after the guard so no invalid pointer is ever formed. Add unit-x86-paging-oob test that sets page_table_page_used to WOLFBOOT_PTP_NUM and verifies that every subsequent call to x86_paging_setup_ptp triggers panic (via longjmp stub) rather than silently proceeding with an out-of-bounds memset. --- include/x86/common.h | 2 +- src/x86/common.c | 6 +- src/x86/paging.c | 6 +- tools/unit-tests/Makefile | 5 ++ tools/unit-tests/unit-x86-paging-oob.c | 85 ++++++++++++++++++++++++++ 5 files changed, 98 insertions(+), 6 deletions(-) create mode 100644 tools/unit-tests/unit-x86-paging-oob.c diff --git a/include/x86/common.h b/include/x86/common.h index 0bd371bcbb..5a40701226 100644 --- a/include/x86/common.h +++ b/include/x86/common.h @@ -61,7 +61,7 @@ void io_write32(uint16_t port, uint32_t value); uint32_t io_read32(uint16_t port); void reset(uint8_t warm); void delay(int msec); -void panic(void); +__attribute__((noreturn)) void panic(void); void cpuid(uint32_t eax_param, uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx); int cpuid_is_1gb_page_supported(void); diff --git a/src/x86/common.c b/src/x86/common.c index 80bdcd2691..7e0db43797 100644 --- a/src/x86/common.c +++ b/src/x86/common.c @@ -300,9 +300,11 @@ uint64_t hal_get_timer_us(void) * This function is used for error handling when the system encounters an unrecoverable issue. * It enters an infinite loop, causing a panic state. */ -void panic() +__attribute__((noreturn)) void panic() { - hlt(); + while(1) { + hlt(); + } } /** diff --git a/src/x86/paging.c b/src/x86/paging.c index 083b1c21e2..1e957e6be1 100644 --- a/src/x86/paging.c +++ b/src/x86/paging.c @@ -156,12 +156,12 @@ static void x86_paging_setup_ptp(uint64_t* e) { uint8_t *ptp; - ptp = &page_table_pages[page_table_page_used * PAGE_TABLE_PAGE_SIZE]; - page_table_page_used++; - if (page_table_page_used == WOLFBOOT_PTP_NUM) { + if (page_table_page_used >= WOLFBOOT_PTP_NUM) { wolfBoot_printf("No more page table page structure\r\n"); panic(); } + ptp = &page_table_pages[page_table_page_used * PAGE_TABLE_PAGE_SIZE]; + page_table_page_used++; x86_paging_setup_entry(e, (uintptr_t)ptp); memset(ptp, 0, PAGE_TABLE_PAGE_SIZE); } diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 6bd15d38e0..1be8de3fce 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -64,6 +64,7 @@ TESTS+=unit-fit-fpga TESTS+=unit-mpusize TESTS+=unit-flash-erase-h7 TESTS+=unit-otp-keystore +TESTS+=unit-x86-paging-oob # linux_loader.c is x86 32-bit only, so its unit tests need a working 32-bit # (multilib) toolchain. Probe whether "gcc -m32" can link, and only add the @@ -443,6 +444,10 @@ unit-disk: unit-disk.c gpt-sfdisk-test.h unit-multiboot: unit-multiboot.c gcc -o $@ unit-multiboot.c $(CFLAGS) $(LDFLAGS) +unit-x86-paging-oob: ../../include/target.h unit-x86-paging-oob.c + gcc -o $@ unit-x86-paging-oob.c $(CFLAGS) \ + -ffunction-sections -fdata-sections $(LDFLAGS) -Wl,--gc-sections + %.o:%.c gcc -c -o $@ $^ $(CFLAGS) diff --git a/tools/unit-tests/unit-x86-paging-oob.c b/tools/unit-tests/unit-x86-paging-oob.c new file mode 100644 index 0000000000..1f78c60cb5 --- /dev/null +++ b/tools/unit-tests/unit-x86-paging-oob.c @@ -0,0 +1,85 @@ +/* unit-x86-paging-oob.c + * + * Regression test for OOB memset in x86_paging_setup_ptp when pool is + * exhausted. The guard must fire for any call where page_table_page_used + * >= WOLFBOOT_PTP_NUM, not only the exact == boundary. With the old + * single-hlt panic() the guard check can be bypassed on a second call, + * writing 4 KB one page past the end of page_table_pages[]. + */ + +#include +#include +#include +#include + +static jmp_buf panic_jmp; +static int panic_count = 0; + +/* Satisfies __attribute__((noreturn)) via longjmp; allows test to continue + * after a guarded panic call without executing the code that follows it. */ +__attribute__((noreturn)) void panic(void) +{ + panic_count++; + longjmp(panic_jmp, 1); +} + +/* paging.c includes which maps wolfBoot_printf to fprintf(stderr, + * ...) on Linux, so no extra stub is needed. The cr3-reading static function + * x86_paging_get_paget_table_root() is compiled but never called here. */ +#include "../../src/x86/paging.c" + +static void reset_pool(void) +{ + page_table_page_used = 0; + panic_count = 0; + memset(page_table_pages, 0, sizeof(page_table_pages)); +} + +/* Verify that x86_paging_setup_ptp triggers panic for ANY call when the pool + * is full (page_table_page_used >= WOLFBOOT_PTP_NUM), not only the first. */ +START_TEST(test_ptp_guard_triggers_on_full_pool) +{ + uint64_t e = 0; + + reset_pool(); + page_table_page_used = WOLFBOOT_PTP_NUM; + + /* An over-limit call must trigger panic (longjmp). */ + if (setjmp(panic_jmp) == 0) { + x86_paging_setup_ptp(&e); + /* Reaching here means the guard was bypassed — the bug is present. */ + ck_abort_msg("panic not triggered for over-limit allocation (== guard bypassed)"); + } + ck_assert_int_eq(panic_count, 1); + + /* page_table_page_used must not have advanced past WOLFBOOT_PTP_NUM; + * with the fix the counter is checked before being incremented. */ + ck_assert_int_le(page_table_page_used, WOLFBOOT_PTP_NUM); + + /* A second over-limit call must also trigger panic (>= guard). */ + if (setjmp(panic_jmp) == 0) { + x86_paging_setup_ptp(&e); + ck_abort_msg("panic not triggered on second over-limit allocation"); + } + ck_assert_int_eq(panic_count, 2); +} +END_TEST + +static Suite *paging_oob_suite(void) +{ + Suite *s = suite_create("x86_paging_oob"); + TCase *tc = tcase_create("setup_ptp_guard"); + tcase_add_test(tc, test_ptp_guard_triggers_on_full_pool); + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + Suite *s = paging_oob_suite(); + SRunner *sr = srunner_create(s); + srunner_run_all(sr, CK_NORMAL); + int failed = srunner_ntests_failed(sr); + srunner_free(sr); + return failed == 0 ? 0 : 1; +} From 87fc9296442f13fd3263fd6220daf7dedb6e0e81 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 14:48:52 +0200 Subject: [PATCH 16/37] F-5092: fix off-by-one in fwtpm_nv_read/write/erase offset guard Replace offset > WCS_FWTPM_NV_SIZE with offset >= WCS_FWTPM_NV_SIZE in all three NV backend helpers so that offset == NV_SIZE returns BAD_FUNC_ARG for any size, including 0. Add unit-fwtpm-nv-oob to prove the boundary. --- src/fwtpm_callable.c | 6 +- tools/unit-tests/Makefile | 5 + tools/unit-tests/unit-fwtpm-nv-oob.c | 139 +++++++++++++++++++++++++++ 3 files changed, 147 insertions(+), 3 deletions(-) create mode 100644 tools/unit-tests/unit-fwtpm-nv-oob.c diff --git a/src/fwtpm_callable.c b/src/fwtpm_callable.c index 23d7b822ad..d778475c2a 100644 --- a/src/fwtpm_callable.c +++ b/src/fwtpm_callable.c @@ -60,7 +60,7 @@ static int fwtpm_nv_read(void *ctx, word32 offset, byte *buf, word32 size) { uint8_t *nv = (uint8_t *)ctx; - if (nv == NULL || buf == NULL || offset > WCS_FWTPM_NV_SIZE || + if (nv == NULL || buf == NULL || offset >= WCS_FWTPM_NV_SIZE || size > (WCS_FWTPM_NV_SIZE - offset)) { return BAD_FUNC_ARG; } @@ -74,7 +74,7 @@ static int fwtpm_nv_write(void *ctx, word32 offset, const byte *buf, { uint8_t *nv = (uint8_t *)ctx; - if (nv == NULL || buf == NULL || offset > WCS_FWTPM_NV_SIZE || + if (nv == NULL || buf == NULL || offset >= WCS_FWTPM_NV_SIZE || size > (WCS_FWTPM_NV_SIZE - offset)) { return BAD_FUNC_ARG; } @@ -87,7 +87,7 @@ static int fwtpm_nv_erase(void *ctx, word32 offset, word32 size) { uint8_t *nv = (uint8_t *)ctx; - if (nv == NULL || offset > WCS_FWTPM_NV_SIZE || + if (nv == NULL || offset >= WCS_FWTPM_NV_SIZE || size > (WCS_FWTPM_NV_SIZE - offset)) { return BAD_FUNC_ARG; } diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 1be8de3fce..0175ace263 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -65,6 +65,7 @@ TESTS+=unit-mpusize TESTS+=unit-flash-erase-h7 TESTS+=unit-otp-keystore TESTS+=unit-x86-paging-oob +TESTS+=unit-fwtpm-nv-oob # linux_loader.c is x86 32-bit only, so its unit tests need a working 32-bit # (multilib) toolchain. Probe whether "gcc -m32" can link, and only add the @@ -212,6 +213,10 @@ unit-fwtpm-stub: ../../include/target.h unit-fwtpm-stub.c -DWOLFTPM_USER_SETTINGS -ffunction-sections -fdata-sections \ $(LDFLAGS) -Wl,--gc-sections +unit-fwtpm-nv-oob: ../../include/target.h unit-fwtpm-nv-oob.c + gcc -o $@ $^ $(CFLAGS) -I$(WOLFBOOT_LIB_WOLFTPM) \ + $(LDFLAGS) + unit-tpm-blob: ../../include/target.h unit-tpm-blob.c gcc -o $@ $^ $(CFLAGS) -I$(WOLFBOOT_LIB_WOLFTPM) -DWOLFBOOT_TPM \ -DWOLFTPM_USER_SETTINGS -DWOLFBOOT_TPM_SEAL -DWOLFBOOT_SIGN_RSA2048 \ diff --git a/tools/unit-tests/unit-fwtpm-nv-oob.c b/tools/unit-tests/unit-fwtpm-nv-oob.c new file mode 100644 index 0000000000..837e5aad42 --- /dev/null +++ b/tools/unit-tests/unit-fwtpm-nv-oob.c @@ -0,0 +1,139 @@ +/* unit-fwtpm-nv-oob.c + * + * Regression test: fwtpm_nv_read/write/erase use > instead of >= in the + * offset guard, so offset == WCS_FWTPM_NV_SIZE with size == 0 passes the + * guard and returns TPM_RC_SUCCESS instead of BAD_FUNC_ARG. + */ + +#include +#include +#include + +#define WOLFBOOT_TZ_FWTPM + +/* Minimal types/macros that fwtpm_callable.c uses from wolftpm headers. + * Poison the include guards of the heavy fwtpm headers so they become no-ops; + * we define the handful of types/symbols we actually need ourselves. */ +#define _FWTPM_H_ +#define _FWTPM_COMMAND_H_ +#define _FWTPM_NV_H_ + +/* tpm2_types.h only needs these guards poisoned to avoid the full wolfcrypt + * pull-in; the macros it actually provides (XMEMCPY/XMEMSET etc.) come from + * the guards below. */ +#define WOLFTPM2_NO_WOLFCRYPT + +typedef uint8_t byte; +typedef uint32_t word32; + +#define BAD_FUNC_ARG (-173) +#define TPM_RC_SUCCESS 0x000 +#define TPM_RC_FAILURE 0x101 +#define TPM_RC_INITIALIZE 0x100 + +#define XMEMCPY(d,s,l) memcpy((d),(s),(l)) +#define XMEMSET(b,c,l) memset((b),(c),(l)) + +#define TPM2_HEADER_SIZE 10 + +/* Minimal FWTPM_NV_HAL type matching the nested struct in the real FWTPM_CTX. + * Field layout must match so the static initialiser in fwtpm_callable.c works. */ +typedef struct FWTPM_NV_HAL_S { + int (*read)(void *ctx, word32 offset, byte *buf, word32 size); + int (*write)(void *ctx, word32 offset, const byte *buf, word32 size); + int (*erase)(void *ctx, word32 offset, word32 size); + void *ctx; + word32 maxSize; +} FWTPM_NV_HAL; + +/* Minimal FWTPM_CTX — only the field wcs_fwtpm_init/transmit actually touch */ +typedef struct { + int wasStarted; +} FWTPM_CTX; + +/* Stub symbols referenced by fwtpm_callable.c */ +unsigned int _start_heap; +unsigned int _heap_size; + +void *wolfboot_store_sbrk(unsigned int incr, uint8_t **heap, + uint8_t *start, uint32_t size) +{ + (void)incr; (void)heap; (void)start; (void)size; + return NULL; +} + +int FWTPM_Init(FWTPM_CTX *ctx) { (void)ctx; return 0; } +int FWTPM_NV_SetHAL(FWTPM_CTX *ctx, FWTPM_NV_HAL *hal) + { (void)ctx; (void)hal; return 0; } +int FWTPM_ProcessCommand(FWTPM_CTX *ctx, const byte *cmdBuf, int cmdSize, + byte *rspBuf, int *rspSize, int locality) +{ + (void)ctx; (void)cmdBuf; (void)cmdSize; + (void)rspBuf; (void)rspSize; (void)locality; + return TPM_RC_SUCCESS; +} + +/* Bring in the code under test as part of this translation unit so we can + * reach the static fwtpm_nv_hal struct and the static NV callbacks. */ +#include "../../src/fwtpm_callable.c" + +/* ── tests ──────────────────────────────────────────────────────────────── */ + +START_TEST(nv_read_rejects_offset_at_boundary) +{ + byte buf[8] = {0}; + ck_assert_int_eq( + fwtpm_nv_hal.read(fwtpm_nv_hal.ctx, WCS_FWTPM_NV_SIZE, buf, 0), + BAD_FUNC_ARG); +} +END_TEST + +START_TEST(nv_write_rejects_offset_at_boundary) +{ + byte buf[8] = {0}; + ck_assert_int_eq( + fwtpm_nv_hal.write(fwtpm_nv_hal.ctx, WCS_FWTPM_NV_SIZE, buf, 0), + BAD_FUNC_ARG); +} +END_TEST + +START_TEST(nv_erase_rejects_offset_at_boundary) +{ + ck_assert_int_eq( + fwtpm_nv_hal.erase(fwtpm_nv_hal.ctx, WCS_FWTPM_NV_SIZE, 0), + BAD_FUNC_ARG); +} +END_TEST + +/* Sanity: valid access at last byte must succeed */ +START_TEST(nv_read_accepts_last_valid_offset) +{ + byte buf[1] = {0}; + ck_assert_int_eq( + fwtpm_nv_hal.read(fwtpm_nv_hal.ctx, WCS_FWTPM_NV_SIZE - 1, buf, 1), + TPM_RC_SUCCESS); +} +END_TEST + +static Suite *fwtpm_nv_oob_suite(void) +{ + Suite *s = suite_create("fwtpm_nv_oob"); + TCase *tc = tcase_create("boundary_guard"); + tcase_add_test(tc, nv_read_rejects_offset_at_boundary); + tcase_add_test(tc, nv_write_rejects_offset_at_boundary); + tcase_add_test(tc, nv_erase_rejects_offset_at_boundary); + tcase_add_test(tc, nv_read_accepts_last_valid_offset); + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int fails; + Suite *s = fwtpm_nv_oob_suite(); + SRunner *sr = srunner_create(s); + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + return fails == 0 ? 0 : 1; +} From 9bfecfb617c12015ca4e724033cc7fb417a855df Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 15:39:32 +0200 Subject: [PATCH 17/37] Fix fwTPM unit test CI config --- .github/workflows/test-external-library-paths.yml | 4 +++- tools/unit-tests/Makefile | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test-external-library-paths.yml b/.github/workflows/test-external-library-paths.yml index 4d437547cb..9cc000c0d9 100644 --- a/.github/workflows/test-external-library-paths.yml +++ b/.github/workflows/test-external-library-paths.yml @@ -86,4 +86,6 @@ jobs: echo "=== Building unit tests with external paths ===" make -C tools/unit-tests \ WOLFBOOT_LIB_WOLFSSL="$(realpath ../external-libs/wolfssl)" \ - WOLFBOOT_LIB_WOLFPKCS11="$(realpath ../external-libs/wolfPKCS11)" + WOLFBOOT_LIB_WOLFPKCS11="$(realpath ../external-libs/wolfPKCS11)" \ + WOLFBOOT_LIB_WOLFPSA="$(realpath ../external-libs/wolfPSA)" \ + WOLFBOOT_LIB_WOLFTPM="$(realpath ../external-libs/wolfTPM)" diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 0175ace263..5fc5e6ccef 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -215,7 +215,7 @@ unit-fwtpm-stub: ../../include/target.h unit-fwtpm-stub.c unit-fwtpm-nv-oob: ../../include/target.h unit-fwtpm-nv-oob.c gcc -o $@ $^ $(CFLAGS) -I$(WOLFBOOT_LIB_WOLFTPM) \ - $(LDFLAGS) + -DWOLFTPM_USER_SETTINGS $(LDFLAGS) unit-tpm-blob: ../../include/target.h unit-tpm-blob.c gcc -o $@ $^ $(CFLAGS) -I$(WOLFBOOT_LIB_WOLFTPM) -DWOLFBOOT_TPM \ From f7bf241334e7ea8ffaf6769d30dccbd215d74461 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 16:23:31 +0200 Subject: [PATCH 18/37] Guard mcxn UDS zeroize helper --- hal/mcxn.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hal/mcxn.c b/hal/mcxn.c index 72d55a3bd6..da96b747d6 100644 --- a/hal/mcxn.c +++ b/hal/mcxn.c @@ -71,12 +71,14 @@ #include #include +#ifdef WOLFBOOT_UDS_UID_FALLBACK_FORTEST static NOINLINEFUNCTION void hal_uds_zeroize(void *ptr, size_t len) { volatile uint8_t *p = (volatile uint8_t *)ptr; while (len-- > 0U) *p++ = 0U; } +#endif /* Derive UDS from device UUID for software DICE testing. * NOT secure — UUID is publicly observable. Only enabled with From f20cd3022aa233ee765b60cde06af989a9c60bce Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 18:50:36 +0200 Subject: [PATCH 19/37] F-4972: zeroize stack wc_MlDsaKey after Free in keygen_ml_dsa cleanup wc_MlDsaKey_Free does not zero the struct; add wc_ForceZero(&key, sizeof(key)) immediately after, matching the explicit pattern already used in the adjacent keygen_xmss function. --- tools/keytools/keygen.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/keytools/keygen.c b/tools/keytools/keygen.c index d52cf02c7f..4f4ce3cd19 100644 --- a/tools/keytools/keygen.c +++ b/tools/keytools/keygen.c @@ -1269,6 +1269,7 @@ static void keygen_ml_dsa(const char *priv_fname, uint32_t id_mask) fclose(fpriv); if (key_init) wc_MlDsaKey_Free(&key); + wc_ForceZero(&key, sizeof(key)); if (priv != NULL) { wc_ForceZero(priv, priv_len); free(priv); From 652febdcefbc4b3e0c8ffcf41dbbe93af12d4058 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 18:51:58 +0200 Subject: [PATCH 20/37] F-4971: zeroize stack ed448_key after Free in keygen_ed448 cleanup wc_ed448_free does not zero the struct; add wc_ForceZero(&k, sizeof(k)) immediately after, matching the pattern used for ml_dsa (F-4972) and the priv[] zeroing already present on the preceding line. --- tools/keytools/keygen.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/keytools/keygen.c b/tools/keytools/keygen.c index 4f4ce3cd19..cd6f6b323e 100644 --- a/tools/keytools/keygen.c +++ b/tools/keytools/keygen.c @@ -859,6 +859,7 @@ static void keygen_ed448(const char *privkey, uint32_t id_mask) wc_ForceZero(priv, sizeof(priv)); if (key_init == 0) wc_ed448_free(&k); + wc_ForceZero(&k, sizeof(k)); if (exit_code != 0) exit(exit_code); } From ca3e76a9bcd1852ca2254350123bf0a89da2eb3f Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 18:52:49 +0200 Subject: [PATCH 21/37] F-4970: zeroize stack ecc_key after Free in keygen_ecc cleanup wc_ecc_free does not zero the struct; add wc_ForceZero(&k, sizeof(k)) immediately after, matching the pattern used for ed448 (F-4971), ml_dsa (F-4972), and the d[] / priv_der[] zeroing already present on the following lines. --- tools/keytools/keygen.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/keytools/keygen.c b/tools/keytools/keygen.c index cd6f6b323e..22236b2a98 100644 --- a/tools/keytools/keygen.c +++ b/tools/keytools/keygen.c @@ -725,6 +725,7 @@ static void keygen_ecc(const char *priv_fname, uint16_t ecc_key_size, fclose(fpriv); if (key_init) wc_ecc_free(&k); + wc_ForceZero(&k, sizeof(k)); wc_ForceZero(d, sizeof(d)); wc_ForceZero(priv_der, sizeof(priv_der)); From 793cec45e39364c31718c1b434e8797839438ac5 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 18:53:46 +0200 Subject: [PATCH 22/37] F-4969: zeroize stack RsaKey after FreeRsaKey in keygen_rsa cleanup --- tools/keytools/keygen.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/keytools/keygen.c b/tools/keytools/keygen.c index 22236b2a98..48033ee771 100644 --- a/tools/keytools/keygen.c +++ b/tools/keytools/keygen.c @@ -614,6 +614,7 @@ static void keygen_rsa(const char *keyfile, int kbits, uint32_t id_mask, wc_ForceZero(priv_der, sizeof(priv_der)); if (rsa_init) wc_FreeRsaKey(&k); + wc_ForceZero(&k, sizeof(k)); if (exit_code != 0) exit(exit_code); } From 3349bef30791f805023c8dd995d9248e98498f90 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 18:59:48 +0200 Subject: [PATCH 23/37] F-4966: fix nwords=0 truncation on sub-word write to 16-bit NOR flash When hal_flash_write is called with len=1 on a FLASH_CFI_WIDTH=16 device, nwords = 1/2 = 0 by integer truncation. The AMD CFI write-buffer protocol then receives (uint16_t)(0-1)=0xFFFF as the word count, the data loop runs zero times, and the confirm is issued with no data, silently dropping the write and potentially leaving the device in an invalid state. Add a read-modify-write path for the nwords==0 case: read the containing 16-bit word, update the target byte (preserving the other byte), and issue a correct single-word write-buffer sequence (word count 0 = 1 word). --- hal/nxp_t10xx.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/hal/nxp_t10xx.c b/hal/nxp_t10xx.c index ac8ea9e949..973dcffe46 100644 --- a/hal/nxp_t10xx.c +++ b/hal/nxp_t10xx.c @@ -3261,6 +3261,27 @@ int hal_flash_write(uint32_t address, const uint8_t *data, int len) sector, offset, xfer, pos); #endif + #if FLASH_CFI_WIDTH == 16 + /* sub-word (1-byte) write: read-modify-write the containing 16-bit word */ + if (nwords == 0) { + uint16_t word = FLASH_IO16_READ(sector, offset); + if ((address % 2) == 0) + word = ((uint16_t)data[pos] << 8) | (word & 0x00FFU); + else + word = (word & 0xFF00U) | (uint16_t)data[pos]; + hal_flash_unlock_sector(sector); + FLASH_IO8_WRITE(sector, offset, AMD_CMD_WRITE_TO_BUFFER); + FLASH_IO16_WRITE(sector, offset, 0); + FLASH_IO16_WRITE(sector, offset, word); + FLASH_IO8_WRITE(sector, offset, AMD_CMD_WRITE_BUFFER_CONFIRM); + hal_flash_status_wait(sector, 0x44, 200*1000); + address++; + pos++; + len--; + continue; + } + #endif + hal_flash_unlock_sector(sector); FLASH_IO8_WRITE(sector, offset, AMD_CMD_WRITE_TO_BUFFER); #if FLASH_CFI_WIDTH == 16 From 59581859683fe4703da123b1eb1aa32fdb01ef0d Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 19:00:42 +0200 Subject: [PATCH 24/37] F-4788: zeroize stack ed25519_key after Free in keygen_ed25519 cleanup --- tools/keytools/keygen.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/keytools/keygen.c b/tools/keytools/keygen.c index 48033ee771..1e1141955f 100644 --- a/tools/keytools/keygen.c +++ b/tools/keytools/keygen.c @@ -801,6 +801,7 @@ static void keygen_ed25519(const char *privkey, uint32_t id_mask) wc_ForceZero(priv, sizeof(priv)); if (key_init == 0) wc_ed25519_free(&k); + wc_ForceZero(&k, sizeof(k)); if (exit_code != 0) exit(exit_code); } From 051c41cdba41c6b04c8f451363f53e63a2f97cb9 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 19:02:36 +0200 Subject: [PATCH 25/37] F-4786: fix delta_base_version LE decode in base_diff for big-endian hosts sign_tool_find_header returns a raw pointer into LE-encoded TLV bytes. Aliasing it as uint32_t* and dereferencing produces the wrong value on big-endian build hosts (e.g. version 5 read as 0x05000000). Decode explicitly byte-by-byte like header_store_u32_le does for writes. --- tools/keytools/sign.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/tools/keytools/sign.c b/tools/keytools/sign.c index b23633a7b4..d256dc2a20 100644 --- a/tools/keytools/sign.c +++ b/tools/keytools/sign.c @@ -2317,7 +2317,8 @@ static int base_diff(const char *f_base, uint8_t *pubkey, uint32_t pubkey_sz, in int r; uint32_t patch_sz, patch_inv_sz; uint32_t patch_inv_off; - uint32_t *delta_base_version = NULL; + uint8_t *delta_base_version = NULL; + uint32_t delta_base_version_val = 0; uint16_t delta_base_version_sz = 0; WB_DIFF_CTX diff_ctx; int ret = -1; @@ -2378,12 +2379,20 @@ static int base_diff(const char *f_base, uint8_t *pubkey, uint32_t pubkey_sz, in #endif /* Check base image version */ - delta_base_version_sz = sign_tool_find_header((uint8_t *)base + 8, HDR_VERSION, (void *)&delta_base_version); - if ((delta_base_version_sz != sizeof(uint32_t)) || (*delta_base_version == 0)) { + delta_base_version_sz = sign_tool_find_header((uint8_t *)base + 8, HDR_VERSION, &delta_base_version); + if (delta_base_version_sz != sizeof(uint32_t)) { + printf("Could not read firmware version from base file %s\n", f_base); + goto cleanup; + } + delta_base_version_val = (uint32_t)delta_base_version[0] | + ((uint32_t)delta_base_version[1] << 8) | + ((uint32_t)delta_base_version[2] << 16) | + ((uint32_t)delta_base_version[3] << 24); + if (delta_base_version_val == 0) { printf("Could not read firmware version from base file %s\n", f_base); goto cleanup; } else { - printf("Delta base version: %u\n", *delta_base_version); + printf("Delta base version: %u\n", delta_base_version_val); } /* Retrieve the hash digest of the base image */ @@ -2588,7 +2597,7 @@ static int base_diff(const char *f_base, uint8_t *pubkey, uint32_t pubkey_sz, in /* Create delta file, with header, from the resulting patch */ ret = make_header_delta(pubkey, pubkey_sz, wolfboot_delta_file, CMD.output_diff_file, - *delta_base_version, patch_sz, patch_inv_off, patch_inv_sz, base_hash, base_hash_sz); + delta_base_version_val, patch_sz, patch_inv_off, patch_inv_sz, base_hash, base_hash_sz); cleanup: if (dest) { From 2a3cbf2c4c30512207b7f8780002448dbba02882 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 19:15:08 +0200 Subject: [PATCH 26/37] F-4784: erase psa_store object data on write-mode open to clear residual key material When an existing object is reopened in write mode and the new payload is shorter than the old one, bytes beyond the new payload end were never erased. wolfPSA_Store_Write only commits sectors covered by the new write, so the trailing bytes from the previous (longer) key persisted in flash and were recoverable via raw flash read. Add an erase pass over every sector of the KEYVAULT_OBJ_SIZE region in wolfPSA_Store_Open when opening an existing object for writing. Only the tok/obj-id prefix of the first sector is restored; the rest is left erased (0xFF) so a subsequent wolfPSA_Store_Write starts from a clean slate. New objects created by create_object are already clean and are skipped via the is_new flag. Add unit test test_shorter_overwrite_clears_tail that confirms the old 0xAA pattern is absent from tail bytes after a 200-byte key is overwritten with a 32-byte key. --- src/psa_store.c | 18 ++++++++++++ tools/unit-tests/unit-psa_store.c | 47 +++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/src/psa_store.c b/src/psa_store.c index 3737c8fad0..68caaf84f4 100644 --- a/src/psa_store.c +++ b/src/psa_store.c @@ -380,6 +380,7 @@ int wolfPSA_Store_Open(int type, unsigned long id1, unsigned long id2, int read, { struct store_handle *handle; uint8_t *buf; + int is_new = 0; /* Check if there is one handle available to open the slot */ handle = find_free_handle(); @@ -408,6 +409,7 @@ int wolfPSA_Store_Open(int type, unsigned long id1, unsigned long id2, int read, *store = NULL; return NOT_AVAILABLE_E; } + is_new = 1; } else { /* buf != NULL, readonly */ handle->hdr = find_object_header(type, id1, id2); if (!handle->hdr) { @@ -429,6 +431,22 @@ int wolfPSA_Store_Open(int type, unsigned long id1, unsigned long id2, int read, handle->flags &= ~STORE_FLAGS_READONLY; /* Truncate the slot when opening in write mode */ update_store_size(handle->hdr, 2 * sizeof(uint32_t)); + /* Erase the object data region so a shorter write does not leave + * residual key material from the previous (longer) payload. */ + if (!is_new) { + uint32_t obj_tok = ((uint32_t *)buf)[0]; + uint32_t obj_id_val = ((uint32_t *)buf)[1]; + uint32_t obj_off = (uint32_t)((uintptr_t)buf - (uintptr_t)vault_base); + uint32_t s; + for (s = 0; s < KEYVAULT_OBJ_SIZE; s += WOLFBOOT_SECTOR_SIZE) { + memset(cached_sector, 0xFF, WOLFBOOT_SECTOR_SIZE); + if (s == 0) { + ((uint32_t *)cached_sector)[0] = obj_tok; + ((uint32_t *)cached_sector)[1] = obj_id_val; + } + cache_commit(obj_off + s); + } + } } diff --git a/tools/unit-tests/unit-psa_store.c b/tools/unit-tests/unit-psa_store.c index ac2fbfeadc..e66261b8d3 100644 --- a/tools/unit-tests/unit-psa_store.c +++ b/tools/unit-tests/unit-psa_store.c @@ -222,6 +222,50 @@ START_TEST(test_find_object_search_stops_at_header_sector) } END_TEST +START_TEST(test_shorter_overwrite_clears_tail) +{ + enum { type = WOLFPSA_STORE_KEY }; + const unsigned long id1 = 3; + const unsigned long id2 = 4; + void *store = NULL; + unsigned char long_key[200]; + unsigned char short_key[32]; + uint8_t *obj_buf; + int ret, i; + + ret = mmap_file("/tmp/wolfboot-unit-psa-keyvault.bin", vault_base, + keyvault_size, NULL); + ck_assert_int_eq(ret, 0); + memset(vault_base, 0xFF, keyvault_size); + + memset(long_key, 0xAA, sizeof(long_key)); + memset(short_key, 0xBB, sizeof(short_key)); + + /* First write: 200-byte key */ + ret = wolfPSA_Store_Open(type, id1, id2, 0, &store); + ck_assert_int_eq(ret, 0); + ret = wolfPSA_Store_Write(store, long_key, 200); + ck_assert_int_eq(ret, 200); + wolfPSA_Store_Close(store); + + /* Second write: 32-byte key (shorter than original) */ + ret = wolfPSA_Store_Open(type, id1, id2, 0, &store); + ck_assert_int_eq(ret, 0); + ret = wolfPSA_Store_Write(store, short_key, 32); + ck_assert_int_eq(ret, 32); + wolfPSA_Store_Close(store); + + /* Tail bytes (beyond the new key) must NOT contain the old 0xAA pattern */ + obj_buf = find_object_buffer(type, id1, id2); + ck_assert_ptr_nonnull(obj_buf); + for (i = 2 * (int)sizeof(uint32_t) + 32; + i < 2 * (int)sizeof(uint32_t) + 200; i++) { + ck_assert_msg(obj_buf[i] != 0xAA, + "Old key material survives at offset %d: 0x%02x", i, obj_buf[i]); + } +} +END_TEST + Suite *wolfboot_suite(void) { Suite *s = suite_create("wolfBoot-psa-store"); @@ -230,17 +274,20 @@ Suite *wolfboot_suite(void) TCase *tcase_delete = tcase_create("delete_object"); TCase *tcase_delete_corrupted = tcase_create("delete_corrupted_pos"); TCase *tcase_find_bounds = tcase_create("find_bounds"); + TCase *tcase_tail = tcase_create("shorter_overwrite_clears_tail"); tcase_add_test(tcase_write, test_cross_sector_write_preserves_length); tcase_add_test(tcase_close, test_close_clears_handle_state); tcase_add_test(tcase_delete, test_delete_object_ignores_metadata_prefix); tcase_add_test(tcase_delete_corrupted, test_delete_object_corrupted_pos_no_oob); tcase_add_test(tcase_find_bounds, test_find_object_search_stops_at_header_sector); + tcase_add_test(tcase_tail, test_shorter_overwrite_clears_tail); suite_add_tcase(s, tcase_write); suite_add_tcase(s, tcase_close); suite_add_tcase(s, tcase_delete); suite_add_tcase(s, tcase_delete_corrupted); suite_add_tcase(s, tcase_find_bounds); + suite_add_tcase(s, tcase_tail); return s; } From a24f107976430e5803cc181549d05b2bb996a81a Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 19:21:59 +0200 Subject: [PATCH 27/37] F-4649: erase pkcs11_store object sectors on write-mode open to clear residual key material When an existing object was reopened in write mode, only hdr->size was reset; the data sectors were never erased. A subsequent shorter write left the prior key bytes in flash beyond hdr->size (physically readable via JTAG/SWD). Mirror the fix applied to psa_store in F-4784: track is_new, and when opening a pre-existing object in write mode erase every sector of the object region via cache_commit (0xFF fill, tok/obj id re-written in sector 0) before the size record is truncated. --- src/pkcs11_store.c | 19 ++++++++++ tools/unit-tests/unit-pkcs11_store.c | 52 ++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/src/pkcs11_store.c b/src/pkcs11_store.c index 8965d4ca0c..2b4a9c6369 100644 --- a/src/pkcs11_store.c +++ b/src/pkcs11_store.c @@ -381,6 +381,7 @@ int wolfPKCS11_Store_Open(int type, CK_ULONG id1, CK_ULONG id2, int read, { struct store_handle *handle; uint8_t *buf; + int is_new = 0; /* Check if there is one handle available to open the slot */ handle = find_free_handle(); @@ -409,6 +410,7 @@ int wolfPKCS11_Store_Open(int type, CK_ULONG id1, CK_ULONG id2, int read, *store = NULL; return NOT_AVAILABLE_E; } + is_new = 1; } else { /* buf != NULL, readonly */ handle->hdr = find_object_header(type, id1, id2); if (!handle->hdr) { @@ -430,6 +432,23 @@ int wolfPKCS11_Store_Open(int type, CK_ULONG id1, CK_ULONG id2, int read, handle->flags &= ~STORE_FLAGS_READONLY; /* Truncate the slot when opening in write mode */ update_store_size(handle->hdr, 2 * sizeof(uint32_t)); + /* Erase object data sectors to clear residual key material from a + * prior (longer) payload. New objects are already in a fresh sector + * from create_object(), so only do this for existing objects. */ + if (!is_new) { + uint32_t obj_tok = ((uint32_t *)buf)[0]; + uint32_t obj_id_val = ((uint32_t *)buf)[1]; + uint32_t obj_off = (uint32_t)((uintptr_t)buf - (uintptr_t)vault_base); + uint32_t s; + for (s = 0; s < KEYVAULT_OBJ_SIZE; s += WOLFBOOT_SECTOR_SIZE) { + memset(cached_sector, 0xFF, WOLFBOOT_SECTOR_SIZE); + if (s == 0) { + ((uint32_t *)cached_sector)[0] = obj_tok; + ((uint32_t *)cached_sector)[1] = obj_id_val; + } + cache_commit(obj_off + s); + } + } } diff --git a/tools/unit-tests/unit-pkcs11_store.c b/tools/unit-tests/unit-pkcs11_store.c index 200c72c11d..5291223f65 100644 --- a/tools/unit-tests/unit-pkcs11_store.c +++ b/tools/unit-tests/unit-pkcs11_store.c @@ -455,6 +455,55 @@ START_TEST(test_find_object_search_stops_at_header_sector) } END_TEST +/* Prove F-4649: shorter overwrite must not leave prior key bytes in flash. + * Write 512 bytes of 0xAA, then reopen and write 50 bytes of 0xBB. + * Raw flash at offsets [8+50 .. 8+512) must be 0xFF (erased), not 0xAA. */ +START_TEST(test_shorter_overwrite_erases_residual_key_material) +{ + const int type = DYNAMIC_TYPE_RSA; + const CK_ULONG id_tok = 42; + const CK_ULONG id_obj = 84; + void *store = NULL; + struct store_handle *h; + unsigned char large_key[512]; + unsigned char small_key[50]; + uint8_t *obj_flash; + uint32_t i; + int ret; + + memset(large_key, 0xAA, sizeof(large_key)); + memset(small_key, 0xBB, sizeof(small_key)); + + ret = mmap_file(vault_path, vault_base, keyvault_size, NULL); + ck_assert_int_eq(ret, 0); + memset(vault_base, 0xEE, keyvault_size); + + /* Write large key to a fresh slot */ + ret = wolfPKCS11_Store_Open(type, id_tok, id_obj, 0, &store); + ck_assert_int_eq(ret, 0); + ret = wolfPKCS11_Store_Write(store, large_key, sizeof(large_key)); + ck_assert_int_eq(ret, (int)sizeof(large_key)); + wolfPKCS11_Store_Close(store); + + /* Reopen in write mode and store a shorter key */ + ret = wolfPKCS11_Store_Open(type, id_tok, id_obj, 0, &store); + ck_assert_int_eq(ret, 0); + h = store; + obj_flash = h->buffer; + ret = wolfPKCS11_Store_Write(store, small_key, sizeof(small_key)); + ck_assert_int_eq(ret, (int)sizeof(small_key)); + wolfPKCS11_Store_Close(store); + + /* Flash beyond the new payload must be erased (0xFF), not old 0xAA */ + for (i = 2 * sizeof(uint32_t) + sizeof(small_key); + i < 2 * sizeof(uint32_t) + sizeof(large_key); i++) { + ck_assert_msg(obj_flash[i] == 0xFF, + "Residual key material at object offset %u: 0x%02x (expected 0xFF)", + i, obj_flash[i]); + } +} +END_TEST + Suite *wolfboot_suite(void) { /* Suite initialization */ @@ -466,18 +515,21 @@ Suite *wolfboot_suite(void) TCase* tcase_delete_object = tcase_create("delete_object"); TCase* tcase_delete_corrupted = tcase_create("delete_corrupted_pos"); TCase* tcase_find_bounds = tcase_create("find_bounds"); + TCase* tcase_remanence = tcase_create("shorter_overwrite_erases_residual"); 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); tcase_add_test(tcase_delete_corrupted, test_delete_object_corrupted_pos_no_oob); tcase_add_test(tcase_find_bounds, test_find_object_search_stops_at_header_sector); + tcase_add_test(tcase_remanence, test_shorter_overwrite_erases_residual_key_material); 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); suite_add_tcase(s, tcase_delete_corrupted); suite_add_tcase(s, tcase_find_bounds); + suite_add_tcase(s, tcase_remanence); return s; } From 63b58c54919502455055dce21d3184b9e74b6d84 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 19:26:34 +0200 Subject: [PATCH 28/37] F-4647: bound strlen to ATA_SECURITY_PASSWORD_LEN in passphrase path security_command_passphrase used strlen(passphrase) on a 32-byte binary buffer from TPM unsealing that carries no null-terminator guarantee, causing an OOB stack read whenever none of the 32 key bytes is zero. Replace strlen with strnlen(passphrase, ATA_SECURITY_PASSWORD_LEN) using a new constant (32, matching the ATA-8 ACS password field size) defined in ata.h. Also add a size check in sata_unlock_disk after sata_get_unlock_secret so a short or malformed unseal result is rejected before reaching the ATA command path. --- include/x86/ata.h | 2 ++ src/x86/ahci.c | 4 ++++ src/x86/ata.c | 3 ++- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/include/x86/ata.h b/include/x86/ata.h index d1b49e3243..47747644ea 100644 --- a/include/x86/ata.h +++ b/include/x86/ata.h @@ -106,6 +106,8 @@ enum ata_security_state ata_security_get_state(int); /* Constants for security set commands */ #define ATA_SECURITY_COMMAND_LEN (256 * 2) #define ATA_SECURITY_PASSWORD_OFFSET (1 * 2) +/* ATA-8 ACS: password field is words 1-16 = 32 bytes */ +#define ATA_SECURITY_PASSWORD_LEN 32 #define ATA_ERR_BUSY -2 #define ATA_ERR_OP_IN_PROGRESS -3 #define ATA_ERR_OP_NOT_IN_PROGRESS -4 diff --git a/src/x86/ahci.c b/src/x86/ahci.c index 059a9c8a74..e24b6069fe 100644 --- a/src/x86/ahci.c +++ b/src/x86/ahci.c @@ -436,6 +436,10 @@ int sata_unlock_disk(int drv, int freeze) r = sata_get_unlock_secret(secret, &secret_size); if (r != 0) goto cleanup; + if (secret_size != ATA_UNLOCK_DISK_KEY_SZ) { + r = -1; + goto cleanup; + } ata_st = ata_security_get_state(drv); wolfBoot_printf("ATA: Security state SEC%d\r\n", ata_st); #if defined(TARGET_x86_fsp_qemu) diff --git a/src/x86/ata.c b/src/x86/ata.c index a2cea6e34b..119b6f726a 100644 --- a/src/x86/ata.c +++ b/src/x86/ata.c @@ -463,7 +463,8 @@ static int security_command_passphrase(int drv, uint8_t ata_cmd, memset(buffer, 0, ATA_SECURITY_COMMAND_LEN); if (master) buffer[0] = 0x1; - memcpy(buffer + ATA_SECURITY_PASSWORD_OFFSET, passphrase, strlen(passphrase)); + memcpy(buffer + ATA_SECURITY_PASSWORD_OFFSET, passphrase, + strnlen(passphrase, ATA_SECURITY_PASSWORD_LEN)); if (slot < 0) { return slot; } From ef16945b2416c9c13c804476da8b8be2828c1e26 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 19:28:05 +0200 Subject: [PATCH 29/37] F-4420: zeroize XmssKey on all error paths in keygen_xmss Replace bare exit(1) calls with goto cleanup / exit_code pattern matching keygen_ml_dsa; wc_XmssKey_Free + wc_ForceZero now run on every error path after wc_XmssKey_Init succeeds, preventing XMSS private-state exposure. --- tools/keytools/keygen.c | 46 ++++++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/tools/keytools/keygen.c b/tools/keytools/keygen.c index 1e1141955f..3b976ff67c 100644 --- a/tools/keytools/keygen.c +++ b/tools/keytools/keygen.c @@ -982,12 +982,16 @@ static void keygen_xmss(const char *priv_fname, uint32_t id_mask) byte xmss_pub[XMSS_SHA256_PUBLEN]; char *xmss_params = getenv("XMSS_PARAMS"); word32 pub_len = sizeof(xmss_pub); + int exit_code = 0; + int key_init = 0; ret = wc_XmssKey_Init(&key, NULL, INVALID_DEVID); if (ret != 0) { fprintf(stderr, "error: wc_XmssKey_Init returned %d\n", ret); - exit(1); + exit_code = 1; + goto cleanup; } + key_init = 1; if (xmss_params == NULL) xmss_params = WOLFBOOT_XMSS_PARAMS; @@ -996,7 +1000,8 @@ static void keygen_xmss(const char *priv_fname, uint32_t id_mask) if (ret != 0) { fprintf(stderr, "error: wc_XmssKey_SetParamStr(%s)" \ " returned %d\n", xmss_params, ret); - exit(1); + exit_code = 1; + goto cleanup; } printf("info: using XMSS parameters: %s\n", xmss_params); @@ -1004,26 +1009,30 @@ static void keygen_xmss(const char *priv_fname, uint32_t id_mask) ret = wc_XmssKey_SetWriteCb(&key, xmss_write_key); if (ret != 0) { fprintf(stderr, "error: wc_XmssKey_SetWriteCb returned %d\n", ret); - exit(1); + exit_code = 1; + goto cleanup; } ret = wc_XmssKey_SetReadCb(&key, xmss_read_key); if (ret != 0) { fprintf(stderr, "error: wc_XmssKey_SetReadCb returned %d\n", ret); - exit(1); + exit_code = 1; + goto cleanup; } ret = wc_XmssKey_SetContext(&key, (void *) priv_fname); if (ret != 0) { fprintf(stderr, "error: wc_XmssKey_SetContext returned %d\n", ret); - exit(1); + exit_code = 1; + goto cleanup; } /* Make the key pair. */ ret = wc_XmssKey_MakeKey(&key, &rng); if (ret != 0) { fprintf(stderr, "error: wc_XmssKey_MakeKey returned %d\n", ret); - exit(1); + exit_code = 1; + goto cleanup; } /* Get the XMSS/XMSS^MT secret key length. */ @@ -1031,19 +1040,22 @@ static void keygen_xmss(const char *priv_fname, uint32_t id_mask) if (ret != 0 || priv_sz <= 0) { printf("error: wc_XmssKey_GetPrivLen returned %d\n", ret); - exit(1); + exit_code = 1; + goto cleanup; } ret = wc_XmssKey_ExportPubRaw(&key, xmss_pub, &pub_len); if (ret != 0) { fprintf(stderr, "error: wc_XmssKey_ExportPubRaw returned %d\n", ret); - exit(1); + exit_code = 1; + goto cleanup; } if (pub_len != sizeof(xmss_pub)) { fprintf(stderr, "error: wc_XmssKey_ExportPubRaw returned pub_len=%d\n" \ ", expected %d\n", pub_len, (int)sizeof(xmss_pub)); - exit(1); + exit_code = 1; + goto cleanup; } /* Append the public key to the private keyfile. */ @@ -1051,7 +1063,8 @@ static void keygen_xmss(const char *priv_fname, uint32_t id_mask) if (!fpriv) { fprintf(stderr, "error: fopen(%s, \"r+\") returned %d\n", priv_fname, ret); - exit(1); + exit_code = 1; + goto cleanup; } fseek(fpriv, priv_sz, SEEK_SET); @@ -1061,15 +1074,20 @@ static void keygen_xmss(const char *priv_fname, uint32_t id_mask) if (exportPubKey) { if (export_pubkey_file(priv_fname, xmss_pub, KEYSTORE_PUBKEY_SIZE_XMSS) != 0) { fprintf(stderr, "Unable to export public key to file\n"); - exit(1); + exit_code = 1; + goto cleanup; } } - keystore_add(AUTH_KEY_XMSS, xmss_pub, KEYSTORE_PUBKEY_SIZE_XMSS, priv_fname, id_mask); - wc_XmssKey_Free(&key); - wc_ForceZero(&key, sizeof(key)); +cleanup: + if (key_init) { + wc_XmssKey_Free(&key); + wc_ForceZero(&key, sizeof(key)); + } + if (exit_code) + exit(exit_code); } From aebecba524c9d84c4c5ae0412a6432b15231a53e Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 19:29:35 +0200 Subject: [PATCH 30/37] F-4419: zeroize LmsKey on all error paths in keygen_lms Replace bare exit(1) calls with goto cleanup / exit_code pattern matching keygen_xmss; wc_LmsKey_Free + wc_ForceZero now run on every error path after wc_LmsKey_Init succeeds, preventing LMS private-state exposure. --- tools/keytools/keygen.c | 42 +++++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/tools/keytools/keygen.c b/tools/keytools/keygen.c index 3b976ff67c..0482305dee 100644 --- a/tools/keytools/keygen.c +++ b/tools/keytools/keygen.c @@ -878,6 +878,8 @@ static void keygen_lms(const char *priv_fname, uint32_t id_mask) word32 pub_len = sizeof(lms_pub); int lms_levels, lms_height, lms_winternitz; char *env_lms_levels, *env_lms_height, *env_lms_winternitz; + int exit_code = 0; + int key_init = 0; lms_levels = LMS_LEVELS; lms_height = LMS_HEIGHT; @@ -896,15 +898,18 @@ static void keygen_lms(const char *priv_fname, uint32_t id_mask) ret = wc_LmsKey_Init(&key, NULL, INVALID_DEVID); if (ret != 0) { fprintf(stderr, "error: wc_LmsKey_Init returned %d\n", ret); - exit(1); + exit_code = 1; + goto cleanup; } + key_init = 1; ret = wc_LmsKey_SetParameters(&key, lms_levels, lms_height, lms_winternitz); if (ret != 0) { fprintf(stderr, "error: wc_LmsKey_SetParameters(%d, %d, %d)" \ " returned %d\n", lms_levels, lms_height, lms_winternitz, ret); - exit(1); + exit_code = 1; + goto cleanup; } printf("info: using LMS parameters: L%d-H%d-W%d\n", lms_levels, @@ -913,37 +918,43 @@ static void keygen_lms(const char *priv_fname, uint32_t id_mask) ret = wc_LmsKey_SetWriteCb(&key, lms_write_key); if (ret != 0) { fprintf(stderr, "error: wc_LmsKey_SetWriteCb returned %d\n", ret); - exit(1); + exit_code = 1; + goto cleanup; } ret = wc_LmsKey_SetReadCb(&key, lms_read_key); if (ret != 0) { fprintf(stderr, "error: wc_LmsKey_SetReadCb returned %d\n", ret); - exit(1); + exit_code = 1; + goto cleanup; } ret = wc_LmsKey_SetContext(&key, (void *) priv_fname); if (ret != 0) { fprintf(stderr, "error: wc_LmsKey_SetContext returned %d\n", ret); - exit(1); + exit_code = 1; + goto cleanup; } ret = wc_LmsKey_MakeKey(&key, &rng); if (ret != 0) { fprintf(stderr, "error: wc_LmsKey_MakeKey returned %d\n", ret); - exit(1); + exit_code = 1; + goto cleanup; } ret = wc_LmsKey_ExportPubRaw(&key, lms_pub, &pub_len); if (ret != 0) { fprintf(stderr, "error: wc_LmsKey_ExportPubRaw returned %d\n", ret); - exit(1); + exit_code = 1; + goto cleanup; } if (pub_len != sizeof(lms_pub)) { fprintf(stderr, "error: wc_LmsKey_ExportPubRaw returned pub_len=%d\n" \ ", expected %d\n", pub_len, (int)sizeof(lms_pub)); - exit(1); + exit_code = 1; + goto cleanup; } /* Append the public key to the private keyfile. */ @@ -951,7 +962,8 @@ static void keygen_lms(const char *priv_fname, uint32_t id_mask) if (!fpriv) { fprintf(stderr, "error: fopen(%s, \"r+\") returned %d\n", priv_fname, ret); - exit(1); + exit_code = 1; + goto cleanup; } fseek(fpriv, 64, SEEK_SET); @@ -961,14 +973,20 @@ static void keygen_lms(const char *priv_fname, uint32_t id_mask) if (exportPubKey) { if (export_pubkey_file(priv_fname, lms_pub, KEYSTORE_PUBKEY_SIZE_LMS) != 0) { fprintf(stderr, "Unable to export public key to file\n"); - exit(1); + exit_code = 1; + goto cleanup; } } keystore_add(AUTH_KEY_LMS, lms_pub, KEYSTORE_PUBKEY_SIZE_LMS, priv_fname, id_mask); - wc_LmsKey_Free(&key); - wc_ForceZero(&key, sizeof(key)); +cleanup: + if (key_init) { + wc_LmsKey_Free(&key); + wc_ForceZero(&key, sizeof(key)); + } + if (exit_code) + exit(exit_code); } #include "../xmss/xmss_common.h" From ad3bc88536b9d30621d0a128cbec55068a82ab50 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 19:36:01 +0200 Subject: [PATCH 31/37] F-4418: fix BSS-clear collision guard to use mem_size instead of file_size The collision guard in elf_load_image_mmu compared vaddr+file_size against the next unread program header, but the subsequent BSS-zero memset writes up to vaddr+mem_size. When mem_size > file_size and headers are not stack-cached (entry_count > ELF_MAX_PH), the memset could corrupt unread program headers. Change the guard to use mem_size so the full write range is checked. --- src/elf.c | 2 +- tools/unit-tests/Makefile | 5 + tools/unit-tests/unit-elf-bss-guard.c | 141 ++++++++++++++++++++++++++ 3 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 tools/unit-tests/unit-elf-bss-guard.c diff --git a/src/elf.c b/src/elf.c index e2a06b19cd..3df98603b8 100644 --- a/src/elf.c +++ b/src/elf.c @@ -191,7 +191,7 @@ int elf_load_image_mmu(uint8_t *image, uint32_t image_sz, uintptr_t *pentry, * Only protect headers [i+1, entry_count) not yet parsed. * Use memmove for safe in-place ELF loading (e.g., RAM boot). */ if (i + 1 >= entry_count || /* last header, nothing left to protect */ - (uint8_t*)vaddr + file_size <= (entry_off + ((i + 1) * entry_size)) || + (uint8_t*)vaddr + mem_size <= (entry_off + ((i + 1) * entry_size)) || (uint8_t*)vaddr > (entry_off + entry_count * entry_size)) { memmove((void*)vaddr, image + offset, file_size); diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 5fc5e6ccef..d4f163828d 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -66,6 +66,7 @@ TESTS+=unit-flash-erase-h7 TESTS+=unit-otp-keystore TESTS+=unit-x86-paging-oob TESTS+=unit-fwtpm-nv-oob +TESTS+=unit-elf-bss-guard # linux_loader.c is x86 32-bit only, so its unit tests need a working 32-bit # (multilib) toolchain. Probe whether "gcc -m32" can link, and only add the @@ -453,6 +454,10 @@ unit-x86-paging-oob: ../../include/target.h unit-x86-paging-oob.c gcc -o $@ unit-x86-paging-oob.c $(CFLAGS) \ -ffunction-sections -fdata-sections $(LDFLAGS) -Wl,--gc-sections +unit-elf-bss-guard: unit-elf-bss-guard.c + gcc -o $@ $< -I../../include -DWOLFBOOT_ELF -DARCH_FLASH_OFFSET=0 \ + -DWOLFBOOT_NO_PRINTF -g $(LDFLAGS) + %.o:%.c gcc -c -o $@ $^ $(CFLAGS) diff --git a/tools/unit-tests/unit-elf-bss-guard.c b/tools/unit-tests/unit-elf-bss-guard.c new file mode 100644 index 0000000000..4330651a94 --- /dev/null +++ b/tools/unit-tests/unit-elf-bss-guard.c @@ -0,0 +1,141 @@ +/* unit-elf-bss-guard.c + * + * Regression test for F-4418: elf_load_image_mmu collision guard uses + * file_size rather than mem_size, so a BSS-bearing PT_LOAD segment can + * overwrite program headers that have not been parsed yet. + * + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfBoot. + * + * wolfBoot is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfBoot is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA + */ + +#include +#include +#include + +#include "elf.h" + +/* Pull in elf.c directly (avoids a separate link step). */ +#include "../../src/elf.c" + +/* 4 KiB is plenty for the ELF64 header + 9 program headers (568 bytes). */ +#define TEST_IMG_SIZE 4096 + +static uint8_t g_image[TEST_IMG_SIZE]; + +/* + * Build a minimal ELF64 executable image in g_image with entry_count = 9 + * program headers (one more than ELF_MAX_PH = 8). That forces + * elf_load_image_mmu to leave the headers in the original image buffer + * rather than caching them on the stack. + * + * ph[0]: PT_LOAD, file_size=0, mem_size=56 (= sizeof elf64_program_header). + * vaddr is set to the runtime address of ph[1] so that the BSS + * zero-fill (memset) writes exactly over ph[1] when the guard is + * evaluated with file_size instead of mem_size. + * + * ph[1]: sentinel — type field set to ELF_PT_LOAD (1). With the bug the + * BSS memset zeroes ph[1].type to 0; with the fix the write is + * blocked and the sentinel is preserved. + * + * ph[2..8]: type=0, mem_size=0 — skipped by the loader. + */ +START_TEST(test_elf_bss_guard_no_header_corruption) +{ + elf64_header *hdr; + elf64_program_header *ph; + uintptr_t entry = 0; + int i, ret; + + memset(g_image, 0, sizeof(g_image)); + + hdr = (elf64_header *)g_image; + memcpy(hdr->ident, ELF_IDENT_STR, 4); + hdr->ident[ELF_CLASS_OFF] = ELF_CLASS_64; + hdr->ident[5] = ELF_ENDIAN_LITTLE; + hdr->type = ELF_HET_EXEC; + hdr->version = 1; + hdr->entry = 0x1000; + hdr->ph_offset = sizeof(elf64_header); /* 64 */ + hdr->ph_entry_size = sizeof(elf64_program_header); /* 56 */ + hdr->ph_entry_count = ELF_MAX_PH + 1; /* 9 */ + + ph = (elf64_program_header *)(g_image + sizeof(elf64_header)); + + /* + * ph[0]: BSS-only PT_LOAD segment. + * vaddr points at ph[1] so that a memset of mem_size bytes zeroes it. + * With the buggy guard (file_size check): + * vaddr + file_size = ph[1]_start + 0 = ph[1]_start <= ph[1]_start → TRUE + * → write proceeds; memset zeroes ph[1]. + * With the fixed guard (mem_size check): + * vaddr + mem_size = ph[1]_start + 56 > ph[1]_start → FALSE + * → write is skipped; ph[1] intact. + */ + ph[0].type = ELF_PT_LOAD; + ph[0].flags = 0; + ph[0].offset = 0; + ph[0].vaddr = (uint64_t)(uintptr_t)(ph + 1); /* = &ph[1] */ + ph[0].paddr = ph[0].vaddr; + ph[0].file_size = 0; + ph[0].mem_size = sizeof(elf64_program_header); /* 56 bytes of BSS */ + ph[0].align = 1; + + /* ph[1]: sentinel — preserve this type value to detect corruption. */ + ph[1].type = ELF_PT_LOAD; + ph[1].mem_size = 0; /* skipped by the "mem_size == 0" guard if reached */ + + /* ph[2..8]: empty, will be skipped. */ + for (i = 2; i <= ELF_MAX_PH; i++) { + ph[i].type = 0; + ph[i].mem_size = 0; + } + + ret = elf_load_image_mmu(g_image, sizeof(g_image), &entry, NULL); + ck_assert_int_eq(ret, 0); + + /* + * ph[1].type must not have been zeroed by the BSS memset from ph[0]. + * Failure here means the guard compared vaddr+file_size instead of + * vaddr+mem_size, allowing the memset to corrupt the unread header. + */ + ck_assert_msg(ph[1].type != 0, + "BSS memset from ph[0] corrupted ph[1].type: " + "collision guard must use mem_size, not file_size"); +} +END_TEST + +Suite *elf_bss_guard_suite(void) +{ + Suite *s = suite_create("ELF BSS guard"); + TCase *tc = tcase_create("bss-header-collision"); + tcase_add_test(tc, test_elf_bss_guard_no_header_corruption); + tcase_set_timeout(tc, 10); + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int fails; + Suite *s = elf_bss_guard_suite(); + SRunner *sr = srunner_create(s); + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + return fails; +} From 9e390e8fce0131afba1adea21820d8b505475195 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 19:39:51 +0200 Subject: [PATCH 32/37] F-4414: replace strtol with strtoul for fw_version to fix saturation on 32-bit-long platforms strtol saturates to LONG_MAX (INT32_MAX) and sets errno=ERANGE for version strings above 2147483647 on Windows LLP64 and 32-bit hosts, silently encoding the wrong version. strtoul covers the full uint32_t range on all platforms (ULONG_MAX >= UINT32_MAX). Add explicit out-of-range error to match existing pattern (lines 2063-2071). --- tools/keytools/sign.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tools/keytools/sign.c b/tools/keytools/sign.c index d256dc2a20..b8a82270a6 100644 --- a/tools/keytools/sign.c +++ b/tools/keytools/sign.c @@ -1443,7 +1443,17 @@ static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz, /* No pad bytes, version is aligned */ /* Append Version field */ - fw_version32 = strtol(CMD.fw_version, NULL, 10); + { + unsigned long tmp_ver; + errno = 0; + tmp_ver = strtoul(CMD.fw_version, NULL, 10); + if (errno == ERANGE || tmp_ver > (unsigned long)UINT32_MAX) { + fprintf(stderr, "Error: firmware version out of range: %s\n", + CMD.fw_version); + goto failure; + } + fw_version32 = (uint32_t)tmp_ver; + } header_append_tag_u32(header, &header_idx, HDR_VERSION, fw_version32); /* Append pad bytes, so timestamp val field is 8-byte aligned */ From 82142f870de279c037babe9cff1c3cbbcbe7e564 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 19:41:37 +0200 Subject: [PATCH 33/37] F-4412: add equal-version emergency rollback test to pin >= operator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds test_emergency_rollback_equal_versions which exercises the cur_ver == upd_ver path in the rollback_needed gate (update_flash.c:952). The existing test only used cur_ver=2/upd_ver=1 (strictly greater), so the >= → > mutation survived; this test catches it by asserting IMG_STATE_SUCCESS after a same-version rollback. --- tools/unit-tests/unit-update-flash.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tools/unit-tests/unit-update-flash.c b/tools/unit-tests/unit-update-flash.c index 8105104f06..f3145ff562 100644 --- a/tools/unit-tests/unit-update-flash.c +++ b/tools/unit-tests/unit-update-flash.c @@ -988,6 +988,28 @@ START_TEST (test_emergency_rollback) { cleanup_flash(); } +START_TEST (test_emergency_rollback_equal_versions) { + uint8_t testing_flags[5] = { IMG_STATE_TESTING, 'B', 'O', 'O', 'T' }; + uint8_t st = 0; + reset_mock_stats(); + prepare_flash(); + add_payload(PART_BOOT, 1, TEST_SIZE_SMALL); + add_payload(PART_UPDATE, 1, TEST_SIZE_SMALL); + /* Set the testing flag in the last five bytes of the BOOT partition */ + hal_flash_unlock(); + hal_flash_write(WOLFBOOT_PARTITION_BOOT_ADDRESS + WOLFBOOT_PARTITION_SIZE - 5, + testing_flags, 5); + hal_flash_lock(); + + wolfBoot_start(); + ck_assert(!wolfBoot_panicked); + ck_assert(wolfBoot_staged_ok); + /* After equal-version emergency rollback, boot partition must be SUCCESS */ + ck_assert_int_eq(wolfBoot_get_partition_state(PART_BOOT, &st), 0); + ck_assert_uint_eq(st, IMG_STATE_SUCCESS); + cleanup_flash(); +} + START_TEST (test_emergency_rollback_failure_due_to_bad_update) { uint8_t testing_flags[5] = { IMG_STATE_TESTING, 'B', 'O', 'O', 'T' }; uint8_t wrong_update_magic[4] = { 'G', 'O', 'L', 'F' }; @@ -1531,6 +1553,7 @@ Suite *wolfboot_suite(void) tcase_add_test(zero_size_update, test_zero_size_update_rejected); tcase_add_test(invalid_sha, test_invalid_sha); tcase_add_test(emergency_rollback, test_emergency_rollback); + tcase_add_test(emergency_rollback, test_emergency_rollback_equal_versions); tcase_add_test(emergency_rollback_failure_due_to_bad_update, test_emergency_rollback_failure_due_to_bad_update); tcase_add_test(empty_boot_partition_update, test_empty_boot_partition_update); tcase_add_test(empty_boot_but_update_sha_corrupted_denied, test_empty_boot_but_update_sha_corrupted_denied); From 893973a207d5ebffb818e0bd9103ac302a974d5f Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 21:10:32 +0200 Subject: [PATCH 34/37] Handle fdt splice size errors (addressed Mattia's comment) --- .gitignore | 1 + src/fdt.c | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index c215a1e7db..847f11b361 100644 --- a/.gitignore +++ b/.gitignore @@ -199,6 +199,7 @@ tools/unit-tests/unit-linux-loader-syssize tools/unit-tests/unit-mpusize tools/unit-tests/unit-otp-keystore tools/unit-tests/unit-tpm-api-names +tools/unit-tests/unit-elf-bss-guard # Elf preprocessing tools diff --git a/src/fdt.c b/src/fdt.c index 7d944d91f5..13a3d2e851 100644 --- a/src/fdt.c +++ b/src/fdt.c @@ -281,9 +281,15 @@ static void fdt_del_last_string_(void *fdt, const char *s) static int fdt_splice_(void *fdt, void *splicepoint, int oldlen, int newlen) { + int data_size; char *p, *end; + + data_size = fdt_data_size_(fdt); + if (data_size < 0) + return data_size; + p = splicepoint; - end = (char*)fdt + fdt_data_size_(fdt); + end = (char*)fdt + data_size; if (((p + oldlen) < p) || ((p + oldlen) > end)) { return -FDT_ERR_BADOFFSET; } From 88befb6b709fd73c89b198e45c1e6219d3901e68 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 21:34:22 +0200 Subject: [PATCH 35/37] Preserve neighboring store objects on truncate (addressed Fenrir's review comment) --- src/pkcs11_store.c | 42 ++++++++++++++++++++++++++++++------------ src/psa_store.c | 42 ++++++++++++++++++++++++++++++------------ 2 files changed, 60 insertions(+), 24 deletions(-) diff --git a/src/pkcs11_store.c b/src/pkcs11_store.c index 2b4a9c6369..a53679aec6 100644 --- a/src/pkcs11_store.c +++ b/src/pkcs11_store.c @@ -360,6 +360,35 @@ static void update_store_size(struct obj_hdr *hdr, uint32_t size) cache_commit(0); } +static void erase_object_payload(uint8_t *buf) +{ + uint32_t erase_off; + uint32_t erase_end; + uint32_t sector_base; + + erase_off = (uint32_t)((uintptr_t)buf - (uintptr_t)vault_base) + + (2U * sizeof(uint32_t)); + erase_end = (uint32_t)((uintptr_t)buf - (uintptr_t)vault_base) + + KEYVAULT_OBJ_SIZE; + sector_base = erase_off - (erase_off % WOLFBOOT_SECTOR_SIZE); + + while (sector_base < erase_end) { + uint32_t erase_start = erase_off; + uint32_t erase_stop = sector_base + WOLFBOOT_SECTOR_SIZE; + + if (erase_start < sector_base) + erase_start = sector_base; + if (erase_stop > erase_end) + erase_stop = erase_end; + + memcpy(cached_sector, vault_base + sector_base, WOLFBOOT_SECTOR_SIZE); + memset(cached_sector + (erase_start - sector_base), 0xFF, + erase_stop - erase_start); + cache_commit(sector_base); + sector_base += WOLFBOOT_SECTOR_SIZE; + } +} + /* Find a free handle in openstores_handles[] array * to manage the interaction with the API. * @@ -436,18 +465,7 @@ int wolfPKCS11_Store_Open(int type, CK_ULONG id1, CK_ULONG id2, int read, * prior (longer) payload. New objects are already in a fresh sector * from create_object(), so only do this for existing objects. */ if (!is_new) { - uint32_t obj_tok = ((uint32_t *)buf)[0]; - uint32_t obj_id_val = ((uint32_t *)buf)[1]; - uint32_t obj_off = (uint32_t)((uintptr_t)buf - (uintptr_t)vault_base); - uint32_t s; - for (s = 0; s < KEYVAULT_OBJ_SIZE; s += WOLFBOOT_SECTOR_SIZE) { - memset(cached_sector, 0xFF, WOLFBOOT_SECTOR_SIZE); - if (s == 0) { - ((uint32_t *)cached_sector)[0] = obj_tok; - ((uint32_t *)cached_sector)[1] = obj_id_val; - } - cache_commit(obj_off + s); - } + erase_object_payload(buf); } } diff --git a/src/psa_store.c b/src/psa_store.c index 68caaf84f4..5be5570831 100644 --- a/src/psa_store.c +++ b/src/psa_store.c @@ -359,6 +359,35 @@ static void update_store_size(struct obj_hdr *hdr, uint32_t size) cache_commit(0); } +static void erase_object_payload(uint8_t *buf) +{ + uint32_t erase_off; + uint32_t erase_end; + uint32_t sector_base; + + erase_off = (uint32_t)((uintptr_t)buf - (uintptr_t)vault_base) + + (2U * sizeof(uint32_t)); + erase_end = (uint32_t)((uintptr_t)buf - (uintptr_t)vault_base) + + KEYVAULT_OBJ_SIZE; + sector_base = erase_off - (erase_off % WOLFBOOT_SECTOR_SIZE); + + while (sector_base < erase_end) { + uint32_t erase_start = erase_off; + uint32_t erase_stop = sector_base + WOLFBOOT_SECTOR_SIZE; + + if (erase_start < sector_base) + erase_start = sector_base; + if (erase_stop > erase_end) + erase_stop = erase_end; + + memcpy(cached_sector, vault_base + sector_base, WOLFBOOT_SECTOR_SIZE); + memset(cached_sector + (erase_start - sector_base), 0xFF, + erase_stop - erase_start); + cache_commit(sector_base); + sector_base += WOLFBOOT_SECTOR_SIZE; + } +} + /* Find a free handle in openstores_handles[] array * to manage the interaction with the API. * @@ -434,18 +463,7 @@ int wolfPSA_Store_Open(int type, unsigned long id1, unsigned long id2, int read, /* Erase the object data region so a shorter write does not leave * residual key material from the previous (longer) payload. */ if (!is_new) { - uint32_t obj_tok = ((uint32_t *)buf)[0]; - uint32_t obj_id_val = ((uint32_t *)buf)[1]; - uint32_t obj_off = (uint32_t)((uintptr_t)buf - (uintptr_t)vault_base); - uint32_t s; - for (s = 0; s < KEYVAULT_OBJ_SIZE; s += WOLFBOOT_SECTOR_SIZE) { - memset(cached_sector, 0xFF, WOLFBOOT_SECTOR_SIZE); - if (s == 0) { - ((uint32_t *)cached_sector)[0] = obj_tok; - ((uint32_t *)cached_sector)[1] = obj_id_val; - } - cache_commit(obj_off + s); - } + erase_object_payload(buf); } } From 590164eff1252620745aac39459e7ad15e89032e Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 22:48:02 +0200 Subject: [PATCH 36/37] Fix x86 FSP freestanding build regressions --- src/x86/ata.c | 7 ++++++- src/x86/mptable.c | 8 ++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/x86/ata.c b/src/x86/ata.c index 119b6f726a..bb6dd1fae2 100644 --- a/src/x86/ata.c +++ b/src/x86/ata.c @@ -457,14 +457,19 @@ static int security_command_passphrase(int drv, uint8_t ata_cmd, struct hba_cmd_table *tbl; struct fis_reg_h2d *cmdfis; struct ata_drive *ata = &ATA_Drv[drv]; + size_t passphrase_len = 0; int ret; int slot = prepare_cmd_h2d_slot(drv, buffer, ATA_SECURITY_COMMAND_LEN, 1); memset(buffer, 0, ATA_SECURITY_COMMAND_LEN); if (master) buffer[0] = 0x1; + while (passphrase_len < ATA_SECURITY_PASSWORD_LEN && + passphrase[passphrase_len] != '\0') { + passphrase_len++; + } memcpy(buffer + ATA_SECURITY_PASSWORD_OFFSET, passphrase, - strnlen(passphrase, ATA_SECURITY_PASSWORD_LEN)); + passphrase_len); if (slot < 0) { return slot; } diff --git a/src/x86/mptable.c b/src/x86/mptable.c index 441ba3c6af..0edd871d9e 100644 --- a/src/x86/mptable.c +++ b/src/x86/mptable.c @@ -33,7 +33,7 @@ /* TGL mptable */ static struct mptable _mptable = { .mpf = { - .signature = "_MP_", + .signature = {'_', 'M', 'P', '_'}, .phy_addr = (int)MPTABLE_LOAD_BASE + sizeof(struct mp_float), .length = 1, .spec_rev = 0x4, @@ -45,7 +45,7 @@ static struct mptable _mptable = { .feature5 = 0 }, .mpc_table = { - .signature = MPC_SIGNATURE, + .signature = {'P', 'C', 'M', 'P'}, .base_table_len = sizeof(struct mp_conf_table_header) + sizeof(struct mp_conf_entry_processor) * MP_CPU_NUM_ENTRY + sizeof(struct mp_conf_entry_bus) * MP_BUS_NUM_ENTRY + @@ -248,7 +248,7 @@ static struct mptable _mptable = { /* MPtables for qemu */ struct mptable _mptable = { .mpf = { - .signature = "_MP_", + .signature = {'_', 'M', 'P', '_'}, .phy_addr = (int)MPTABLE_LOAD_BASE + sizeof(struct mp_float), .length = 1, .spec_rev = 1, @@ -260,7 +260,7 @@ struct mptable _mptable = { .feature5 = 0 }, .mpc_table = { - .signature = MPC_SIGNATURE, + .signature = {'P', 'C', 'M', 'P'}, .base_table_len = sizeof(struct mp_conf_table_header) + sizeof(struct mp_conf_entry_processor) * MP_CPU_NUM_ENTRY + sizeof(struct mp_conf_entry_bus) * MP_BUS_NUM_ENTRY + From 423c1d48b3b6333b5e24ee92d13febef641b329f Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 11 Jun 2026 00:02:33 +0200 Subject: [PATCH 37/37] Revert misguided secret_size guard in sata_unlock_disk F-4647 added an exact-size guard in sata_unlock_disk(): if (secret_size != ATA_UNLOCK_DISK_KEY_SZ) { r = -1; goto cleanup; } on the assumption that the disk unlock secret is a fixed ATA_UNLOCK_DISK_KEY_SZ (32) byte key. That assumption is wrong: the secret is a variable-length base64 string produced from ATA_SECRET_RANDOM_BYTES (21) random bytes, i.e. 28 encoded chars plus a NUL = 29 bytes. The guard therefore rejected every valid unseal result (29 != 32) and panicked on every boot, failing the fsp_qemu_test CI job ("Secret 29 bytes" -> "wolfBoot: PANIC!"). The Fenrir F-4647 finding was only partly off: the real issue it flagged -- an out-of-bounds read from an unbounded copy of a non-NUL-terminated passphrase buffer -- is genuine and remains fixed in security_command_passphrase() (the passphrase copy is length-bounded to ATA_SECURITY_PASSWORD_LEN). Only the additional sata_unlock_disk size guard was based on a wrong premise. Remove the guard and document why none belongs there: the secret is a variable-length string (so an equality check is wrong), and a size guard is unnecessary because sata_get_unlock_secret() passes sizeof(secret) as the wolfBoot_unseal() capacity, so secret_size can never exceed the buffer. --- src/x86/ahci.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/x86/ahci.c b/src/x86/ahci.c index e24b6069fe..bb87a7326e 100644 --- a/src/x86/ahci.c +++ b/src/x86/ahci.c @@ -436,10 +436,15 @@ int sata_unlock_disk(int drv, int freeze) r = sata_get_unlock_secret(secret, &secret_size); if (r != 0) goto cleanup; - if (secret_size != ATA_UNLOCK_DISK_KEY_SZ) { - r = -1; - goto cleanup; - } + /* No secret_size check here on purpose: the unlock secret is a + * variable-length base64 string (ATA_SECRET_RANDOM_BYTES random bytes + * encoded, e.g. 29 bytes), not a fixed ATA_UNLOCK_DISK_KEY_SZ key, so an + * equality check is wrong. A size guard is also unnecessary: + * sata_get_unlock_secret() passes sizeof(secret) (ATA_UNLOCK_DISK_KEY_SZ) + * as the wolfBoot_unseal() capacity, so secret_size can never exceed the + * buffer. The non-NUL-terminated read in the ATA passphrase path is bounded + * separately by strnlen(passphrase, ATA_SECURITY_PASSWORD_LEN) in + * security_command_passphrase(). */ ata_st = ata_security_get_state(drv); wolfBoot_printf("ATA: Security state SEC%d\r\n", ata_st); #if defined(TARGET_x86_fsp_qemu)