end_address was computed as address + len - 1 (inclusive last byte) but the
loop used strict p < end_address, so a non-page-aligned len caused the final
page to be skipped. Change to end_address = address + len (exclusive) to
match stm32u5.c, the claimed reference implementation.
Add WOLFBOOT_UNIT_TEST_FLASH_ERASE guards to hal/stm32u3.c and hal/stm32u3.h
(DMB/ISB/DSB, FLASH_NS_CR/SR register macros, and all non-erase functions) so
that hal_flash_erase() can be compiled and tested in isolation on the host.
Add tools/unit-tests/unit-flash-erase-u3.c with four check-based tests; the
regression case (len = PAGE_SIZE + 1) failed before this fix.
end_address = address + len - 1 (inclusive) with p < end_address (strict) skips
the final page when len is not a multiple of FLASH_PAGE_SIZE. Change to exclusive
end_address = address + len, matching the fix applied to stm32g0/l0/wb. Add
WOLFBOOT_UNIT_TEST_FLASH_ERASE guards so the function can be included in isolation
on the host, and add unit-flash-erase-c0 to the test suite.
end_address was computed as address + len - 1 (inclusive) but the loop
used strict less-than, so the final page was skipped when len was not a
multiple of FLASH_PAGE_SIZE. Use an exclusive end (address + len) to
match the fix applied to stm32h5.c, stm32wb.c, and stm32l0.c.
Add WOLFBOOT_UNIT_TEST_FLASH_ERASE guard to hal/stm32g0.c and a new
unit test (unit-flash-erase-g0) that proves the regression and confirms
the fix.
end_address was computed as address + len - 1 (inclusive) but paired with a
strict p < end_address guard, so the last page was skipped whenever len was
not a multiple of FLASH_PAGE_SIZE. Change end_address to the exclusive form
address + len so the loop covers all pages that overlap the requested range.
Add unit-flash-erase-l0 regression test (mirrors the unit-flash-erase-wb
harness) with guards in hal/stm32l0.c allowing the HAL to be compiled on the
host; the unaligned-len test fails before the fix and passes after.
end_address = address + len - 1 (inclusive) combined with p < end_address
(strict less-than) skipped the last page when len % FLASH_PAGE_SIZE == 1.
Use the exclusive form end_address = address + len, mirroring the stm32h5
fix. Add WOLFBOOT_UNIT_TEST_FLASH_ERASE guards and a new unit test that
proves the regression before the fix and passes after.
Mirror the DISABLE_BACKUP treatment: print a $(warning ...) so the
build operator gets an unmistakable signal that anti-rollback
enforcement has been disabled.
The --custom-tlv TAG LEN VAL path in make_header_ex() passed
&CMD.custom_tlv[i].val (a raw uint64_t pointer) directly to
header_append_tag(), which does a memcpy. On big-endian build hosts the
first LEN bytes of the uint64_t are the high bytes, producing the wrong
LE encoding (e.g. 4-byte value 0x12345678 encodes as 00 00 00 00). The
fix serialises through header_store_u64_le() before calling
header_append_tag(), matching the pattern already used by
header_append_tag_u16/u32/u64 for all system TLVs.
Add unit-sign-custom-tlv-le.py to verify the LE byte encoding of 4-byte
and 8-byte custom TLV values in the signed image header.
Equal-version and newer-update cases were untested, allowing mutations of the
comparator or the addr assignment at libwolfboot.c:1446-1448 to survive. Add
two tests to unit-update-ram-nofixed.c that directly verify retval and *addr
for boot_v==update_v (must prefer primary) and update_v>boot_v (must prefer
update partition).
All four PS dispatch arms (PS_SET, PS_GET, PS_GET_INFO, PS_REMOVE) cast
in_vec[N].base to a typed pointer but only checked for NULL, not that
the caller-supplied .len covers the target type. An NS caller could pass
len=0 (or len < sizeof) with a valid non-NULL base, causing the secure
side to read past the declared buffer. Mirror the pattern already used
in wolfboot_crypto_dispatch (F-3541) and the attestation handler.
The firmware encryption key and nonce were left in the NVM_CACHE_SIZE
stack buffer after hal_flash_write returned, until the frame was
naturally overwritten. Add ForceZero guarded by the same preprocessor
condition that declares the stack-local ENCRYPT_CACHE.
update_ram.c stored the firmware versions into int locals via
(int)wolfBoot_current_firmware_version() / (int)wolfBoot_update_firmware_version().
Under WOLFBOOT_FIXED_PARTITIONS those macros resolve to wolfBoot_get_image_version(),
which returns uint32_t. Versions >= 0x80000000 became negative when cast to int,
failed the ">= 0" clamp, and left boot_v/update_v at 0. With both partition
versions above INT_MAX, max_v collapsed to 0 and the rollback guard
"(max_v > 0U) && (active_v < max_v)" was silently skipped, allowing a rolled-back
image to be staged after a fallback. Same fundamental pattern as F-4411 (hwswap)
and the documented 3736 (update_disk).
wolfBoot_get_image_version() returns uint32_t with 0 as its only "invalid" value,
so the negative-clamp logic was both unnecessary and harmful. Use the direct
uint32_t reads.
Add tools/unit-tests/unit-update-ram-noramboot.c, a WOLFBOOT_NO_RAMBOOT build of
wolfBoot_start (the configuration that uses wolfBoot_open_image() and therefore
actually reaches the rollback guard). The rollback test drives BOOT
(v0x80000005, marked oversize so its open is rejected) and UPDATE (v0x80000003,
valid): the boot path falls back to UPDATE and must deny the downgrade. The test
fails before the fix (UPDATE staged, do_boot reached) and passes after. The
existing unit-update-ram target uses the RAMBOOT path, where wolfBoot_ramboot()
has its own separate int-cast of the version that rejects all high-version
images before the guard is reached; that path cannot exercise this guard and is
left unchanged.
update_flash_hwswap.c stored the firmware versions into int locals via
(int)wolfBoot_current_firmware_version() / (int)wolfBoot_update_firmware_version().
Under WOLFBOOT_FIXED_PARTITIONS those macros resolve to wolfBoot_get_image_version(),
which returns uint32_t. Versions >= 0x80000000 became negative when cast to int,
failed the ">= 0" clamp, and left boot_v/update_v at 0. With both partition
versions above INT_MAX, max_v collapsed to 0 and the rollback guard
"(max_v > 0U) && (active_v < max_v)" was silently skipped, allowing a rolled-back
image to be staged. This was a regression introduced in 90670fcd; the original
code used uint32_t directly.
wolfBoot_get_blob_version() returns uint32_t with 0 as its only "invalid" value,
so the negative-clamp logic was both unnecessary and harmful. Restore the direct
uint32_t reads.
Add tools/unit-tests/unit-update-flash-hwswap.c which drives wolfBoot_start with
BOOT (v0x80000002, unbootable) and UPDATE (v0x80000001, valid): the boot path
falls back to UPDATE and must deny the downgrade. The test fails before the fix
(do_boot reached) and passes after.
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.
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.
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).
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.
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.
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.
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.
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.
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.
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.
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).
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.
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.
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.
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.
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.
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.
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.
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.
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<totalsize),
and the subsequent memcpy writes the property-name string directly over the
FDT header, corrupting magic, totalsize, and struct offsets.
Fix by computing the sum in 64-bit in fdt_data_size_ and returning
-FDT_ERR_BADOFFSET on overflow; add a symmetric early-return overflow check
in fdt_splice_string_ before the pointer is formed; and propagate the error
through fdt_shrink so it does not silently store a zero totalsize.
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.
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.
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.
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.
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.
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.
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.
Daniele Lacamera
Mattia Moffa
Mattia Moffa