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/.gitignore b/.gitignore index 539fdcd240..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 @@ -207,6 +208,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/hal/mcxn.c b/hal/mcxn.c index b8d8495adb..da96b747d6 100644 --- a/hal/mcxn.c +++ b/hal/mcxn.c @@ -71,6 +71,15 @@ #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 * WOLFBOOT_UDS_UID_FALLBACK_FORTEST. */ @@ -121,8 +130,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 +145,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 */ } 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 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/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/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/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/disk.c b/src/disk.c index 782c002f77..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; @@ -169,6 +170,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/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/src/fdt.c b/src/fdt.c index 0ced19d709..13a3d2e851 100644 --- a/src/fdt.c +++ b/src/fdt.c @@ -96,7 +96,11 @@ static inline int fdt_data_size_(void *fdt) { /* the last portion of a FDT is the DT string, so use its offset and size to * determine total size */ - return fdt_off_dt_strings(fdt) + fdt_size_dt_strings(fdt); + uint64_t off = (uint64_t)fdt_off_dt_strings(fdt); + uint64_t sz = (uint64_t)fdt_size_dt_strings(fdt); + if (off + sz > (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) @@ -277,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; } @@ -329,7 +339,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 +810,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/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/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/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; } 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/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/src/pkcs11_store.c b/src/pkcs11_store.c index 8965d4ca0c..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. * @@ -381,6 +410,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 +439,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 +461,12 @@ 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) { + erase_object_payload(buf); + } } diff --git a/src/psa_store.c b/src/psa_store.c index 0b8fb6d8c2..5be5570831 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 { @@ -353,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. * @@ -374,6 +409,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(); @@ -402,6 +438,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) { @@ -423,6 +460,11 @@ 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) { + erase_object_payload(buf); + } } 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/src/x86/ahci.c b/src/x86/ahci.c index 059a9c8a74..bb87a7326e 100644 --- a/src/x86/ahci.c +++ b/src/x86/ahci.c @@ -436,6 +436,15 @@ int sata_unlock_disk(int drv, int freeze) r = sata_get_unlock_secret(secret, &secret_size); if (r != 0) 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) diff --git a/src/x86/ata.c b/src/x86/ata.c index a2cea6e34b..bb6dd1fae2 100644 --- a/src/x86/ata.c +++ b/src/x86/ata.c @@ -457,13 +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; - memcpy(buffer + ATA_SECURITY_PASSWORD_OFFSET, passphrase, strlen(passphrase)); + while (passphrase_len < ATA_SECURITY_PASSWORD_LEN && + passphrase[passphrase_len] != '\0') { + passphrase_len++; + } + memcpy(buffer + ATA_SECURITY_PASSWORD_OFFSET, passphrase, + passphrase_len); if (slot < 0) { return slot; } 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/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 + 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/keytools/keygen.c b/tools/keytools/keygen.c index d52cf02c7f..0482305dee 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); } @@ -725,6 +726,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)); @@ -799,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); } @@ -859,6 +862,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); } @@ -874,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; @@ -892,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, @@ -909,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. */ @@ -947,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); @@ -957,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" @@ -978,12 +1000,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; @@ -992,7 +1018,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); @@ -1000,26 +1027,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. */ @@ -1027,19 +1058,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. */ @@ -1047,7 +1081,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); @@ -1057,15 +1092,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); } @@ -1269,6 +1309,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); diff --git a/tools/keytools/sign.c b/tools/keytools/sign.c index b23633a7b4..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 */ @@ -2317,7 +2327,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 +2389,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 +2607,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) { 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() 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); diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 6bd15d38e0..d4f163828d 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -64,6 +64,9 @@ TESTS+=unit-fit-fpga TESTS+=unit-mpusize 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 @@ -211,6 +214,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) \ + -DWOLFTPM_USER_SETTINGS $(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 \ @@ -443,6 +450,14 @@ 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 + +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-disk.c b/tools/unit-tests/unit-disk.c index 40dd08741f..086c5f9429 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) @@ -559,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 * ============================================================ */ @@ -625,6 +648,55 @@ 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_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. @@ -954,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"); @@ -961,6 +1034,8 @@ 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_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); 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; +} 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; 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; 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; +} 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; 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); 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); 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; } diff --git a/tools/unit-tests/unit-psa_store.c b/tools/unit-tests/unit-psa_store.c index 432a5bff19..e66261b8d3 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 }; @@ -184,22 +222,72 @@ 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"); 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 *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; } 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; 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); 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; +}