Fenrir fixes 2026 06 05#788
Merged
Merged
Conversation
…le-fetch The ARM_TEE_PS_SET branch of arm_tee_psa_call() read in_vec[1].len directly from non-secure memory four times: the WOLFBOOT_PS_MAX_DATA bounds check, the null-data guard, the copy gate, the XMEMCPY length, and the entry->size assignment. in_vec lives in NS RAM, and on ARMv8-M with AIRCR.PRIS=0 a higher-priority NS interrupt can preempt Secure thread-mode execution (and an NS-accessible DMA engine can mutate NS RAM independently) between the bounds check and the copy. An NS attacker racing either mechanism could grow the length past WOLFBOOT_PS_MAX_DATA after it was validated, overflowing the fixed entry->data[512] buffer in Secure SRAM into adjacent Secure globals. Snapshot the length once into a Secure-stack local (data_len) right after capturing the in_vec bases and use only that local for every subsequent check, the XMEMCPY operand, and entry->size. This follows the standard PSA/CMSE practice of copying NS-supplied scalars to the Secure stack before validating and using them.
…ted vault A power fault during cache_commit(0) can leave a node header in the keyvault with valid magic/tok/obj/type but pos left as erased flash (0xFFFFFFFF). On the next boot find_object_buffer() detects the data-sector mismatch and calls delete_object(), which reaches bitmap_put(0xFFFFFFFF, 0). bitmap_put computed octet = pos/8 and indexed cached_sector[4 + octet] with no bounds check, writing ~512 MB past the static sector buffer. Reject pos >= KEYVAULT_MAX_ITEMS so a corrupted header can no longer turn into an out-of-bounds write. Adds a unit test that seeds a node with pos=PKCS11_INVALID_ID and confirms delete_object() no longer faults.
e820_add_entry_cb() appended every FSP-supplied resource descriptor into boot_params->e820_table[] with no check against E820_MAX_ENTRIES_ZEROPAGE (128). A HOB list with more than 128 EFI_HOB_TYPE_RESOURCE_DESCRIPTOR entries therefore wrote FSP-controlled addr/size/type triples past the fixed-size table into the stack-allocated boot_params in load_linux(), corrupting adjacent fields and the saved return address. Reject any entry once the table is full (return non-zero, which aborts the HOB iteration). e820_entries stays uint8_t since it is a fixed-offset field in the Linux zero-page layout and the cap makes the 256 wrap unreachable. Add unit-linux-loader-e820 regression test (x86 32bit, standalone) that feeds 200 descriptors and asserts the table never overflows.
fdt_next_tag() advanced its struct cursor with offset += sizeof(struct fdt_property) - FDT_TAGSIZE + fdt32_to_cpu(*lenp); using unsigned arithmetic. A property whose len field is 0xFFFFFFFF wrapped this to a +7 advance, so the malformed node slipped past the fdt_offset_ptr() bounds check at the end of fdt_next_tag(). The bogus length then propagated up through fdt_get_property_by_offset() / fdt_getprop() and was returned by fit_load_image() as *lenp = -1. In the MMU FIT boot path, wolfBoot_start() (src/update_ram.c:465) aliases that out-parameter through (int*)&dts_size, turning -1 into a uint32_t 0xFFFFFFFF, and the only guard before the relocation is dts_ptr != NULL, so memcpy(WOLFBOOT_LOAD_DTS_ADDRESS, dts_ptr, 0xFFFFFFFF) ran (CWE-680). A FIT subimage with no "load" property reaches this with the inner copy skipped, so the giant size hits the outer DTS relocation directly. Fix at the root: a property value can never exceed the blob, so reject any FDT_PROP whose declared length is greater than fdt_totalsize() before the cursor arithmetic. This closes the wrap for every caller of fdt_next_tag() (including the other fit_load_image() sinks), not just the DTS path. Legitimate properties (len <= size_dt_struct < totalsize) are unaffected. Add a regression test to unit-fdt: a hand-built FIT whose /images/kernel-1 "data" property declares len=0xFFFFFFFF must make fit_load_image_ex() fail closed (return NULL) instead of handing back a live pointer with a negative length. The test fails before this change and passes after.
load_linux() computed the protected-mode kernel size as the uint32_t product param.hdr.syssize * 16 (src/x86/linux_loader.c), where syssize is copied verbatim from the (authenticated) bzImage at offset 0x1f4. The multiplication wraps for any syssize > 0x0FFFFFFF: syssize=0x10000000 yields kernel_size=0 (DoS), and syssize=0x1FFFFFFF/0xFFFFFFFF yields kernel_size=0xFFFFFFF0 (~4 GiB). That value fed straight into memcpy((uint8_t*)KERNEL_LOAD_ADDRESS, linux_image + param_size, kernel_size) with no cap, overwriting wolfBoot stage2, FSP data, and the heap (CWE-190 -> CWE-680). Fix at the root: linux_kernel_size() computes syssize * 16 in 64-bit and rejects the image (panic) when the result is zero or does not fit in the destination window [KERNEL_LOAD_ADDRESS, tolum). tolum is the top of low usable memory the FSP already reports and that the ELF boot path uses as its load upper bound (src/boot_x86_fsp_payload.c). The kernel load only runs under WOLFBOOT_FSP (the non-FSP path panics earlier at the memory map step), so tolum is always available there. Add unit-linux-loader-syssize regression test (x86 32bit, standalone) that feeds the PoC overflow values and asserts they are rejected while a legitimate kernel and the exact-fit boundary are accepted.
…eck_address_range The eleven CSME_NSE_API TPM veneers in src/tpm.c are cmse_nonsecure_entry gateways when built with TZEN=1 (e.g. config/examples/stm32h5-tz-tpm.config, which exposes them to the non-secure STM32H5 test app). Each accepted a typed pointer from the non-secure caller and immediately used it as a dereference or memset/XMEMSET target on the Secure side -- memset(caps), memset(handles), memset(getTime), XMEMSET(quoteResult), and the in/out forwards to TPM2_GetCapability / TPM2_ParseAttest. Because Secure code can write Secure SRAM, a malicious non-secure caller could pass a Secure pointer and turn any of these veneers into a confused-deputy write primitive against Secure memory. Validate every non-secure-supplied pointer with cmse_check_address_range (CMSE_NONSECURE, plus CMSE_MPU_READWRITE for write targets) before the first use, returning BAD_FUNC_ARG (or NULL for the string helpers) when the range is not accessible from the non-secure world. The check is wrapped in WOLFBOOT_TPM_NS_RW/WOLFBOOT_TPM_NS_R, guarded by __ARM_FEATURE_CMSE == 3U so non-CMSE builds (where there is no security boundary) collapse to a plain non-NULL pass-through. Buffer sizes use the caller-provided/known capacities (name_sz, error_sz, *certSz, PCR digest size). Verified by building wolfboot.elf with the stm32h5-tz-tpm config (-mcmse); the bug itself is a TrustZone Secure/Non-secure partitioning issue that cannot be exercised on the host unit-test build.
…heck_address_range wcs_get_random is a cmse_nonsecure_entry secure gateway (wc_callable.o is in SECURE_OBJS, built with -mcmse when WOLFCRYPT_TZ=1, e.g. config/examples/stm32l5-wolfcrypt-tz.config). The rand pointer and size arrive directly from the non-secure caller and were passed unchecked to wc_RNG_GenerateBlock, which writes size bytes through rand. Because Secure code can write Secure SRAM, a malicious NS caller could pass a Secure pointer and turn the veneer into a confused-deputy write primitive against Secure memory (RNG output aimed at key buffers, RNG state, or a Secure stack return address). Validate the full rand/size range with cmse_check_address_range (CMSE_NONSECURE | CMSE_MPU_READWRITE) before the write, returning BAD_FUNC_ARG when the range is not accessible from the non-secure world. The check is wrapped in WOLFBOOT_WCS_NS_RW, guarded by __ARM_FEATURE_CMSE == 3U so non-CMSE builds (no security boundary) collapse to a plain non-NULL pass-through. The range check already rejects oversized lengths that overrun NS-accessible memory, so no separate size cap is needed. Same fix pattern as F-4644 (TPM veneers in tpm.c). Verified by compiling src/wc_callable.c with cortex-m33 -mcmse -DWOLFCRYPT_SECURE_MODE; the bug is a TrustZone partitioning issue that cannot be exercised on the host unit-test build.
wcs_fwtpm_transmit is a cmse_nonsecure_entry veneer that receives the command buffer (cmd), response buffer (rsp) and response-size pointer (rspSz) directly from the non-secure caller. It only checked for NULL and size bounds, then passed the pointers to FWTPM_ProcessCommand (reading cmd, writing rsp) and dereferenced rspSz. A non-secure caller could aim rsp at Secure SRAM (confused-deputy write, forging TPM responses) or cmd at Secure memory (leak through the command path). Validate each NS-supplied range with cmse_check_address_range before first use: cmd as read-only (cmdSz), rspSz as read-write (sizeof) before dereferencing it, and rsp as read-write (rspCapacity). The checks are wrapped in WCS_FWTPM_NS_R/WCS_FWTPM_NS_RW, guarded by __ARM_FEATURE_CMSE == 3U so non-CMSE builds collapse to a non-NULL pass-through. Matches the existing guards in wc_callable.c and tpm.c. Verified by building wolfboot.elf with the stm32h5-tz-fwtpm config (-mcmse); the bug is a TrustZone Secure/Non-secure partitioning issue that cannot be exercised on the host unit-test build.
…or stall A hostile or malformed PCIe endpoint can return a BAR size-probe readback whose address bits (31:4) are all zero but is itself non-zero (e.g. 0x8, only the prefetch indicator). This passes the bar_value == 0 guard, yields bar_align == 0, and makes length = (~bar_align) + 1 wrap to 0 in uint32_t. With length == 0 the allocator cursor *base = bar_value + length is left unchanged, so the next BAR is programmed onto the same MMIO address, colliding the windows of all following devices. Treat a BAR with no writable address bits as unimplemented and skip it via restore_bar before computing length. Legitimate MMIO BARs always have at least one writable address bit; IO BARs already force the high bits, so bar_align is never 0 for them. Add a unit test that simulates the malformed probe readback and verifies the BAR is restored (not programmed) and does not collide with the next BAR.
…veneer wolfBoot_nsc_get_partition_state is a cmse_nonsecure_entry secure-gateway veneer (compiled with CSME_NSE_API under __WOLFBOOT && TZEN). The output pointer st arrives directly from the non-secure caller and was forwarded unchecked to wolfBoot_get_partition_state, which performs *st = *state unconditionally once the partition magic check passes (libwolfboot.c:735). Because the veneer runs in Secure state with full write access to Secure SRAM, a malicious NS caller could aim st at Secure memory and turn the veneer into a confused-deputy 1-byte write primitive (the partition-state byte, e.g. 0x00/0xFF) against Secure SRAM (magic fields, key-store flags, version counters). Validate the st range with cmse_check_address_range (CMSE_NONSECURE | CMSE_MPU_READWRITE) before forwarding, returning -1 when the range is not accessible from the non-secure world. The check is wrapped in WOLFBOOT_NSC_NS_RW, guarded by __ARM_FEATURE_CMSE == 3U so non-CMSE builds (no security boundary) collapse to a plain non-NULL pass-through. Same fix pattern as F-4416/F-4417/F-4644 (wc_callable.c, fwtpm_callable.c, tpm.c). Verified by compiling the veneer with cortex-m33 -mcmse: the expected bl cmse_check_address_range is emitted before the callee. The bug is a TrustZone Secure/Non-secure partitioning issue that cannot be exercised on the host unit-test build (the TZEN veneer block is not compiled there).
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR hardens several security-sensitive and parsing/loader paths against malformed/untrusted inputs (overflow, OOB writes, TOCTOU, and non-secure pointer misuse) and adds regression tests for the new guardrails.
Changes:
- Add CMSE non-secure pointer range validation to multiple NS-callable veneers (TPM/WCS/wolfBoot NSC APIs).
- Add bounds/overflow protections in x86 linux loader (syssize size calc, e820 table bound), PCI BAR programming, FDT property parsing, and PKCS11 bitmap updates.
- Add/extend unit tests to cover the new regression scenarios (PKCS11 corrupted pos, PCI zero-align BAR, FIT oversized property len, linux loader syssize/e820).
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/unit-tests/unit-pkcs11_store.c | Adds regression test ensuring corrupted hdr->pos does not trigger bitmap OOB write. |
| tools/unit-tests/unit-pci.c | Extends PCI test harness to simulate hostile BAR probe results; adds zero-align BAR regression test. |
| tools/unit-tests/unit-linux-loader-syssize.c | Adds standalone 32-bit regression test for syssize multiplication overflow/bounds. |
| tools/unit-tests/unit-linux-loader-e820.c | Adds standalone 32-bit regression test for e820 table bounds handling. |
| tools/unit-tests/unit-fdt.c | Adds regression test for rejecting oversized FIT/FDT property lengths. |
| tools/unit-tests/Makefile | Wires new linux-loader standalone tests into the unit test build. |
| src/x86/linux_loader.c | Adds e820 entry bound; adds 64-bit-safe syssize kernel size helper and enforces load window limit. |
| src/wc_callable.c | Adds CMSE non-secure range validation for wcs_get_random output buffer. |
| src/tpm.c | Adds CMSE non-secure range validation for TPM NS-callable API buffers. |
| src/pkcs11_store.c | Guards bitmap_put against out-of-range positions to avoid OOB writes. |
| src/pci.c | Skips malformed/unimplemented BARs with zero alignment to avoid allocator cursor wrap/stall. |
| src/libwolfboot.c | Adds CMSE non-secure range validation for wolfBoot_nsc_get_partition_state output pointer. |
| src/fwtpm_callable.c | Adds CMSE non-secure range validation for fwtpm transmit command/response buffers. |
| src/fdt.c | Rejects oversized property length early to prevent cursor wrap and downstream giant memcpy sizes. |
| src/arm_tee_psa_ipc.c | Fixes TOCTOU double-fetch by snapshotting NS-supplied input length before checking/copying. |
| .gitignore | Ignores the new standalone unit test binaries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mattia-moffa
approved these changes
Jun 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
3bf9a76 F-4337: validate NS pointer in wolfBoot_nsc_get_partition_state CMSE veneer
5360130 F-4338: skip MMIO BAR with zero alignment to prevent length-wrap cursor stall
2ebc304 F-4416: validate NS pointers in wcs_fwtpm_transmit CMSE veneer
e1dd13e F-4417: validate NS pointer in wcs_get_random CMSE veneer with cmse_check_address_range
091b4ea F-4644: validate NS pointers in TPM CSME_NSE_API veneers with cmse_check_address_range
d175c81 F-4645: bound load_linux kernel size to prevent syssize*16 overflow
2766450 F-4710: reject oversized FDT property length to prevent ~4GB FIT memcpy
a8a9eec F-4711: bound e820 entries to prevent boot_params stack overflow
277e801 F-4965: bound hdr->pos in bitmap_put to prevent OOB write from corrupted vault
8cff2e8 F-5672: cache NS in_vec[1].len in ARM_TEE_PS_SET to close TOCTOU double-fetch