From f598b3fd4119d9f823386915d973cbffcf22f8e7 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 09:46:27 +0200 Subject: [PATCH 01/21] F-6878: delta: reject non-multiple SECTOR_SIZE/DELTA_BLOCK_SIZE at build The per-sector fill loop in wolfBoot_delta_update() advances in DELTA_BLOCK_SIZE steps, so a WOLFBOOT_SECTOR_SIZE that is not a multiple of DELTA_BLOCK_SIZE writes past the one-sector SWAP partition and misaligns the resume path. Enforce the invariant with a #error and add a negative build test (unit-delta-sector-align.py). Verification: full unit suite 1096 checks, 0 failures; new test fails pre-fix (misaligned config built), passes post-fix (build rejected). --- src/update_flash.c | 7 +++ tools/unit-tests/Makefile | 13 +++++ tools/unit-tests/unit-delta-sector-align.py | 64 +++++++++++++++++++++ 3 files changed, 84 insertions(+) create mode 100644 tools/unit-tests/unit-delta-sector-align.py diff --git a/src/update_flash.c b/src/update_flash.c index c2a27e48c8..77bc0ed1cb 100644 --- a/src/update_flash.c +++ b/src/update_flash.c @@ -623,6 +623,13 @@ static int RAMFUNCTION wolfBoot_swap_and_final_erase(int resume) # define DELTA_BLOCK_SIZE 1024 #endif + /* The per-sector fill loop advances in DELTA_BLOCK_SIZE steps, so a + * sector that is not a multiple of the block size would be written + * past the one-sector SWAP partition and misalign the resume path. */ + #if (WOLFBOOT_SECTOR_SIZE % DELTA_BLOCK_SIZE) != 0 + #error "Delta update: WOLFBOOT_SECTOR_SIZE % DELTA_BLOCK_SIZE != 0" + #endif + static inline uint32_t wb_delta_im2n(uint32_t val) { #ifdef BIG_ENDIAN_ORDER diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index c34d235e40..234ccf7735 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -221,6 +221,7 @@ run: $(TESTS) python3 unit-sign-delta-tlv.py || exit 1 python3 unit-sign-delta-cert-inv-off.py || exit 1 python3 unit-sign-delta-basehash-cleanup.py || exit 1 + python3 unit-delta-sector-align.py || exit 1 python3 unit-sign-custom-tlv-le.py || exit 1 python3 unit-sign-custom-tlv-large.py || exit 1 python3 unit-sign-custom-tlv-pubkey-der.py || exit 1 @@ -264,6 +265,14 @@ unit-update-flash-delta:CFLAGS+=-DMOCK_PARTITIONS -DWOLFBOOT_NO_SIGN -DUNIT_TEST -DWOLFBOOT_HASH_SHA256 -DPRINTF_ENABLED -DEXT_FLASH -DPART_UPDATE_EXT -DPART_SWAP_EXT \ -DDELTA_UPDATES -DDELTA_BLOCK_SIZE=512 -D__WOLFBOOT \ -DWOLFBOOT_ORIGIN=MOCK_ADDRESS_BOOT -DBOOTLOADER_PARTITION_SIZE=WOLFBOOT_PARTITION_SIZE +# Intentionally misaligned config: the mock WOLFBOOT_SECTOR_SIZE (0x400) is +# not a multiple of DELTA_BLOCK_SIZE (1536). Must fail to build with the +# delta alignment #error. Used by unit-delta-sector-align.py only, so it is +# not part of $(TESTS). +unit-update-flash-delta-misalign:CFLAGS+=-DMOCK_PARTITIONS -DWOLFBOOT_NO_SIGN -DUNIT_TEST_AUTH \ + -DWOLFBOOT_HASH_SHA256 -DPRINTF_ENABLED -DEXT_FLASH -DPART_UPDATE_EXT -DPART_SWAP_EXT \ + -DDELTA_UPDATES -DDELTA_BLOCK_SIZE=1536 -D__WOLFBOOT \ + -DWOLFBOOT_ORIGIN=MOCK_ADDRESS_BOOT -DBOOTLOADER_PARTITION_SIZE=WOLFBOOT_PARTITION_SIZE unit-update-flash-self-update:CFLAGS+=-DMOCK_PARTITIONS -DWOLFBOOT_NO_SIGN -DUNIT_TEST_AUTH \ -DWOLFBOOT_HASH_SHA256 -DPRINTF_ENABLED -DEXT_FLASH -DPART_UPDATE_EXT -DPART_SWAP_EXT \ -DRAM_CODE -DARCH_SIM -DUNIT_TEST_SELF_UPDATE_ONLY \ @@ -808,6 +817,10 @@ unit-update-flash-delta: ../../include/target.h unit-update-flash.c gcc -o $@ unit-update-flash.c ../../src/image.c ../../src/delta.c \ $(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/sha256.c $(CFLAGS) $(LDFLAGS) +unit-update-flash-delta-misalign: ../../include/target.h unit-update-flash.c + gcc -o $@ unit-update-flash.c ../../src/image.c ../../src/delta.c \ + $(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/sha256.c $(CFLAGS) $(LDFLAGS) + unit-update-flash-enc:CFLAGS+=-DMOCK_PARTITIONS -DWOLFBOOT_NO_SIGN -DUNIT_TEST_AUTH \ -DWOLFBOOT_HASH_SHA256 -DPRINTF_ENABLED -DEXT_FLASH -DPART_UPDATE_EXT \ -DPART_SWAP_EXT -DEXT_ENCRYPTED -DENCRYPT_WITH_CHACHA -DHAVE_CHACHA \ diff --git a/tools/unit-tests/unit-delta-sector-align.py b/tools/unit-tests/unit-delta-sector-align.py new file mode 100644 index 0000000000..644d0439b6 --- /dev/null +++ b/tools/unit-tests/unit-delta-sector-align.py @@ -0,0 +1,64 @@ +#!/usr/bin/env python3 +# unit-delta-sector-align.py +# +# Build-time invariant for DELTA_UPDATES: the per-sector fill loop in +# wolfBoot_delta_update() (src/update_flash.c) calls wb_patch() with +# DELTA_BLOCK_SIZE chunks until WOLFBOOT_SECTOR_SIZE bytes are produced, so +# the reconstructed image is only sector-aligned when WOLFBOOT_SECTOR_SIZE is +# a multiple of DELTA_BLOCK_SIZE. A mismatched config used to compile fine +# and corrupt the delta apply (write past the one-sector SWAP partition, +# misaligned resume); the build now rejects it with an #error. +# +# This test builds the real update_flash.c (via unit-update-flash.c) with a +# deliberately misaligned DELTA_BLOCK_SIZE and asserts the build fails with +# the invariant message. +# +# 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 + +import os +import subprocess +import sys + +MESSAGE = "WOLFBOOT_SECTOR_SIZE % DELTA_BLOCK_SIZE != 0" + + +def main(): + # The misalign target is not in $(TESTS) and its rule does not track + # src/update_flash.c (included via unit-update-flash.c), so drop any + # stale artifact to force a real rebuild. + try: + os.remove("unit-update-flash-delta-misalign") + except FileNotFoundError: + pass + p = subprocess.run(["make", "unit-update-flash-delta-misalign"], + capture_output=True, text=True) + if p.returncode == 0: + print("FAIL: misaligned DELTA_BLOCK_SIZE config built, " + "expected the sector alignment #error") + return 1 + if MESSAGE not in (p.stdout + p.stderr): + print("FAIL: build failed for the wrong reason:\n") + print(p.stderr[-2000:]) + return 1 + print("PASS: misaligned DELTA_BLOCK_SIZE rejected at build time") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From 455c1c6cc623359695452ae741052d3e4515b69d Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 09:52:22 +0200 Subject: [PATCH 02/21] F-7067: elf scatter: fix PART_IS_EXT arg and check load result In the DISABLE_BACKUP branch of wolfBoot_update() the ELF-scatter restore block passed the boot struct by value to the pointer-taking PART_IS_EXT macro, so DISABLE_BACKUP + WOLFBOOT_ELF_FLASH_SCATTER + EXT_FLASH did not even compile, and the load result was discarded. Mirror the wolfBoot_start() pattern (PART_IS_EXT(&boot), panic on load failure), drop the dead base local, and add a compile check for the combination (unit-elf-scatter-db-build.py) guarding the one test that needs a DISABLE_BACKUP-excluded symbol. Verification: full unit suite 1096 checks, 0 failures; new test fails pre-fix (struct vs pointer compile error), passes post-fix. --- src/update_flash.c | 8 +++- tools/unit-tests/Makefile | 13 ++++++ tools/unit-tests/unit-elf-scatter-db-build.py | 46 +++++++++++++++++++ tools/unit-tests/unit-update-flash.c | 4 ++ 4 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 tools/unit-tests/unit-elf-scatter-db-build.py diff --git a/src/update_flash.c b/src/update_flash.c index 77bc0ed1cb..5f398d7130 100644 --- a/src/update_flash.c +++ b/src/update_flash.c @@ -1332,12 +1332,16 @@ static int RAMFUNCTION wolfBoot_update(int fallback_allowed) #else /* DISABLE_BACKUP */ #ifdef WOLFBOOT_ELF_FLASH_SCATTER unsigned long entry; - void* base = (void*)WOLFBOOT_PARTITION_BOOT_ADDRESS; wolfBoot_printf("ELF Scattered image digest check\n"); if (wolfBoot_check_flash_image_elf(PART_BOOT, &entry) < 0) { wolfBoot_printf("ELF Scattered image digest check: failed. Restoring " "scattered image...\n"); - wolfBoot_load_flash_image_elf(PART_BOOT, &entry, PART_IS_EXT(boot)); + if (wolfBoot_load_flash_image_elf(PART_BOOT, &entry, + PART_IS_EXT(&boot)) < 0) { + wolfBoot_printf( + "ELF: [UPDATE] ERROR: could not store scattered image\n"); + wolfBoot_panic(); + } if (wolfBoot_check_flash_image_elf(PART_BOOT, &entry) < 0) { wolfBoot_printf( "Fatal: Could not verify digest after scattering. Panic().\n"); diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 234ccf7735..cd1e1b1056 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -222,6 +222,7 @@ run: $(TESTS) python3 unit-sign-delta-cert-inv-off.py || exit 1 python3 unit-sign-delta-basehash-cleanup.py || exit 1 python3 unit-delta-sector-align.py || exit 1 + python3 unit-elf-scatter-db-build.py || exit 1 python3 unit-sign-custom-tlv-le.py || exit 1 python3 unit-sign-custom-tlv-large.py || exit 1 python3 unit-sign-custom-tlv-pubkey-der.py || exit 1 @@ -273,6 +274,15 @@ unit-update-flash-delta-misalign:CFLAGS+=-DMOCK_PARTITIONS -DWOLFBOOT_NO_SIGN -D -DWOLFBOOT_HASH_SHA256 -DPRINTF_ENABLED -DEXT_FLASH -DPART_UPDATE_EXT -DPART_SWAP_EXT \ -DDELTA_UPDATES -DDELTA_BLOCK_SIZE=1536 -D__WOLFBOOT \ -DWOLFBOOT_ORIGIN=MOCK_ADDRESS_BOOT -DBOOTLOADER_PARTITION_SIZE=WOLFBOOT_PARTITION_SIZE +# Compile-only check for the DISABLE_BACKUP + WOLFBOOT_ELF_FLASH_SCATTER + +# EXT_FLASH combination: the ELF restore block in wolfBoot_update() must +# compile (it used to pass a struct where PART_IS_EXT expects a pointer). +# Used by unit-elf-scatter-db-build.py only, so it is not part of $(TESTS). +unit-update-flash-elf-scatter-db:CFLAGS+=-DMOCK_PARTITIONS -DWOLFBOOT_NO_SIGN -DUNIT_TEST_AUTH \ + -DWOLFBOOT_HASH_SHA256 -DPRINTF_ENABLED -DEXT_FLASH -DPART_UPDATE_EXT -DPART_SWAP_EXT \ + -DWOLFBOOT_ELF_FLASH_SCATTER -DWOLFBOOT_ELF -DIMAGE_HEADER_SIZE=256 \ + -DDISABLE_BACKUP -D__WOLFBOOT \ + -DWOLFBOOT_ORIGIN=MOCK_ADDRESS_BOOT -DBOOTLOADER_PARTITION_SIZE=WOLFBOOT_PARTITION_SIZE unit-update-flash-self-update:CFLAGS+=-DMOCK_PARTITIONS -DWOLFBOOT_NO_SIGN -DUNIT_TEST_AUTH \ -DWOLFBOOT_HASH_SHA256 -DPRINTF_ENABLED -DEXT_FLASH -DPART_UPDATE_EXT -DPART_SWAP_EXT \ -DRAM_CODE -DARCH_SIM -DUNIT_TEST_SELF_UPDATE_ONLY \ @@ -821,6 +831,9 @@ unit-update-flash-delta-misalign: ../../include/target.h unit-update-flash.c gcc -o $@ unit-update-flash.c ../../src/image.c ../../src/delta.c \ $(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/sha256.c $(CFLAGS) $(LDFLAGS) +unit-update-flash-elf-scatter-db: ../../include/target.h unit-update-flash.c FORCE + gcc -c -o /dev/null unit-update-flash.c $(CFLAGS) + unit-update-flash-enc:CFLAGS+=-DMOCK_PARTITIONS -DWOLFBOOT_NO_SIGN -DUNIT_TEST_AUTH \ -DWOLFBOOT_HASH_SHA256 -DPRINTF_ENABLED -DEXT_FLASH -DPART_UPDATE_EXT \ -DPART_SWAP_EXT -DEXT_ENCRYPTED -DENCRYPT_WITH_CHACHA -DHAVE_CHACHA \ diff --git a/tools/unit-tests/unit-elf-scatter-db-build.py b/tools/unit-tests/unit-elf-scatter-db-build.py new file mode 100644 index 0000000000..576b79b1fb --- /dev/null +++ b/tools/unit-tests/unit-elf-scatter-db-build.py @@ -0,0 +1,46 @@ +#!/usr/bin/env python3 +# unit-elf-scatter-db-build.py +# +# Compile check for the DISABLE_BACKUP + WOLFBOOT_ELF_FLASH_SCATTER + +# EXT_FLASH combination: the ELF-scatter restore block in +# wolfBoot_update() (src/update_flash.c) used to pass the boot struct by +# value to the pointer-taking PART_IS_EXT macro, so this configuration +# failed to build and the branch could never be exercised. It must now +# compile, with the load result checked like in wolfBoot_start(). +# +# 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 + +import subprocess +import sys + + +def main(): + p = subprocess.run(["make", "unit-update-flash-elf-scatter-db"], + capture_output=True, text=True) + if p.returncode != 0: + print("FAIL: DISABLE_BACKUP + ELF scatter + EXT_FLASH " + "does not compile:\n") + print(p.stderr[-2000:]) + return 1 + print("PASS: DISABLE_BACKUP + ELF scatter + EXT_FLASH compiles") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tools/unit-tests/unit-update-flash.c b/tools/unit-tests/unit-update-flash.c index dad19d97cd..26837d42c5 100644 --- a/tools/unit-tests/unit-update-flash.c +++ b/tools/unit-tests/unit-update-flash.c @@ -1280,6 +1280,7 @@ START_TEST (test_empty_boot_but_update_sha_corrupted_denied) { cleanup_flash(); } +#ifndef DISABLE_BACKUP START_TEST (test_swap_resume_noop) { reset_mock_stats(); @@ -1291,6 +1292,7 @@ START_TEST (test_swap_resume_noop) cleanup_flash(); } END_TEST +#endif START_TEST (test_diffbase_version_reads) { @@ -1868,7 +1870,9 @@ Suite *wolfboot_suite(void) 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); +#ifndef DISABLE_BACKUP tcase_add_test(swap_resume, test_swap_resume_noop); +#endif tcase_add_test(diffbase_version, test_diffbase_version_reads); tcase_add_test(diffbase_version, test_diffbase_version_reads_from_little_endian_bytes); tcase_add_test(get_total_size, test_get_total_size_preserves_uint32_range); From 88af5b6472245e028de98f6d9c3497643c018c15 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 10:00:54 +0200 Subject: [PATCH 03/21] F-7390: x86 fsp: drop dead FSP auth scaffolding, fix comment STAGE1_AUTH only authenticates the stage2 wolfBoot payload; the FSP-M and FSP-S blobs are executed unverified. Remove the dead fsp_m/ret declarations in start(), the orphaned .sig_fsp_s placeholder section (no stage1 linker script places it), and correct the comment that claimed the FSPs were authenticated. Add a compile check for the STAGE1_AUTH variant (unit-x86-fsp-stage1auth-build.py), which the unit test CI never builds for lack of an i686 toolchain. Verification: full unit suite 1096 checks, 0 failures; both STAGE1_AUTH variants of boot_x86_fsp.c compile clean; no references to sig_fsp_s remain. --- src/boot_x86_fsp.c | 13 ++---- tools/unit-tests/Makefile | 10 ++++ .../unit-x86-fsp-stage1auth-build.py | 46 +++++++++++++++++++ 3 files changed, 60 insertions(+), 9 deletions(-) create mode 100644 tools/unit-tests/unit-x86-fsp-stage1auth-build.py diff --git a/src/boot_x86_fsp.c b/src/boot_x86_fsp.c index 1d03c67e36..85edaf170f 100644 --- a/src/boot_x86_fsp.c +++ b/src/boot_x86_fsp.c @@ -48,12 +48,12 @@ #ifndef STAGE1_AUTH -/* When STAGE1_AUTH is disabled, create dummy images to fill - * the space used by wolfBoot manifest headers to authenticate FSPs +/* When STAGE1_AUTH is disabled, fill the stage2 manifest header slot with + * a zeroed placeholder so the image layout matches the authenticated build. + * Only the stage2 wolfBoot payload is authenticated; the FSP-M and FSP-S + * blobs are outside the scope of STAGE1_AUTH. */ #define HEADER_SIZE IMAGE_HEADER_SIZE -const uint8_t __attribute__((section(".sig_fsp_s"))) - empty_sig_fsp_s[HEADER_SIZE] = {}; const uint8_t __attribute__((section(".sig_wolfboot_raw"))) empty_sig_wolfboot_raw[HEADER_SIZE] = {}; #endif @@ -543,11 +543,6 @@ void start(uint32_t stack_base, uint32_t stack_top, uint64_t timestamp, uint16_t type; uint32_t esp; -#ifdef STAGE1_AUTH - int ret; - struct wolfBoot_image fsp_m; -#endif - (void)stack_top; (void)timestamp; (void)bist; diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index cd1e1b1056..bbafc640e3 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -223,6 +223,7 @@ run: $(TESTS) python3 unit-sign-delta-basehash-cleanup.py || exit 1 python3 unit-delta-sector-align.py || exit 1 python3 unit-elf-scatter-db-build.py || exit 1 + python3 unit-x86-fsp-stage1auth-build.py || exit 1 python3 unit-sign-custom-tlv-le.py || exit 1 python3 unit-sign-custom-tlv-large.py || exit 1 python3 unit-sign-custom-tlv-pubkey-der.py || exit 1 @@ -710,6 +711,15 @@ unit-boot-x86-fsp: ../../include/target.h unit-boot-x86_fsp.c -DUCODE0_ADDRESS=0 -ffunction-sections -fdata-sections $(LDFLAGS) \ -Wl,--gc-sections +# Compile-only check for the STAGE1_AUTH variant of boot_x86_fsp.c. The real +# stage1 authentication build needs an i686 toolchain the unit test CI does +# not have, so this keeps the variant from silently rotting. Used by +# unit-x86-fsp-stage1auth-build.py only, so it is not part of $(TESTS). +unit-boot-x86-fsp-stage1auth: ../../include/target.h unit-boot-x86_fsp.c FORCE + gcc -c -o /dev/null unit-boot-x86_fsp.c $(CFLAGS) \ + -DWOLFBOOT_LOAD_BASE=0x100000 -DWOLFBOOT_FSP -DUCODE0_ADDRESS=0 \ + -DSTAGE1_AUTH + unit-loader-tpm-init: ../../include/target.h unit-loader-tpm-init.c gcc -o $@ $^ $(CFLAGS) -I$(WOLFBOOT_LIB_WOLFTPM) -DWOLFBOOT_LOADER_MAIN -DWOLFBOOT_TPM \ -DWOLFTPM_USER_SETTINGS \ diff --git a/tools/unit-tests/unit-x86-fsp-stage1auth-build.py b/tools/unit-tests/unit-x86-fsp-stage1auth-build.py new file mode 100644 index 0000000000..2b261b7f52 --- /dev/null +++ b/tools/unit-tests/unit-x86-fsp-stage1auth-build.py @@ -0,0 +1,46 @@ +#!/usr/bin/env python3 +# unit-x86-fsp-stage1auth-build.py +# +# Compile check for the STAGE1_AUTH variant of src/boot_x86_fsp.c. The real +# stage1 authentication build needs an i686 toolchain the unit test CI does +# not have, so this compile-only target keeps the variant from silently +# rotting: it used to carry dead FSP-M verification scaffolding (an unused +# struct wolfBoot_image and int in start()) and a comment claiming the FSPs +# were authenticated, when only the stage2 wolfBoot payload is. +# +# 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 + +import subprocess +import sys + + +def main(): + p = subprocess.run(["make", "unit-boot-x86-fsp-stage1auth"], + capture_output=True, text=True) + if p.returncode != 0: + print("FAIL: STAGE1_AUTH variant of boot_x86_fsp.c " + "does not compile:\n") + print(p.stderr[-2000:]) + return 1 + print("PASS: STAGE1_AUTH variant of boot_x86_fsp.c compiles") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From 77fe733e7ff9bd53101c25168b8d8aac00d1bd4b Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 10:24:11 +0200 Subject: [PATCH 04/21] F-7391: library hal: reject undersized files, bound hash to loaded size main() loaded a file of any size and parsed it as a manifest: header fields (size, TLVs) were read past the end of the heap allocation, and a header claiming a larger fw_size drove the image hash over an unbounded range. Reject files smaller than IMAGE_HEADER_SIZE before parsing and clamp fw_size to the bytes actually loaded. --- .github/workflows/test-library.yml | 12 ++++++++++++ hal/library.c | 20 +++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test-library.yml b/.github/workflows/test-library.yml index 5c33d2c6ed..6b270dd231 100644 --- a/.github/workflows/test-library.yml +++ b/.github/workflows/test-library.yml @@ -158,6 +158,18 @@ jobs: echo "./test-lib test_v1_signed.bin" ./test-lib test_v1_signed.bin + + # A file smaller than the image header must be rejected up front + # instead of being parsed (header fields would be read past the + # end of the allocation). + printf 'WOLF' > tiny.bin + ./test-lib tiny.bin > tiny.out 2>&1 || true + if ! grep -q "too small" tiny.out; then + echo "FAIL: undersized file was not rejected before parsing" + cat tiny.out + exit 1 + fi + echo "PASS: undersized file rejected" ./test-lib test_v1_signed.bin 2>&1 | grep "Firmware Valid" - name: Run test-lib (expect failure) diff --git a/hal/library.c b/hal/library.c index 9bf5ef9f7c..9a60336cb5 100644 --- a/hal/library.c +++ b/hal/library.c @@ -102,6 +102,7 @@ int do_boot(uint32_t* v) } static uintptr_t gImage; +static size_t gImageSize; #ifdef NO_FILESYSTEM static const uint8_t test_img[] = { 0x57, 0x4F, 0x4C, 0x46, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x04, 0x00, 0x01, @@ -139,6 +140,12 @@ int wolfBoot_start(void) goto exit; } + /* The loaded file may be shorter than the firmware size the header + * claims; bound the hash range to the bytes actually loaded. */ + if (os_image.fw_size > (uint32_t)(gImageSize - IMAGE_HEADER_SIZE)) { + os_image.fw_size = (uint32_t)(gImageSize - IMAGE_HEADER_SIZE); + } + if ((ret = wolfBoot_verify_integrity(&os_image)) < 0) { goto exit; } @@ -177,24 +184,35 @@ int main(int argc, const char* argv[]) #ifdef NO_FILESYSTEM gImage = (uintptr_t)test_img; + gImageSize = sizeof(test_img); #else if (argc > 1) { size_t sz = 0, bread; + long fsz; FILE* img = fopen(argv[1], "rb"); if (img == NULL) { wolfBoot_printf("failed to open %s!\n", argv[1]); return -3; } fseek(img, 0, SEEK_END); - sz = ftell(img); + fsz = ftell(img); fseek(img, 0, SEEK_SET); + if ((fsz < 0) || ((size_t)fsz < IMAGE_HEADER_SIZE)) { + wolfBoot_printf("image file too small: %ld bytes " + "(minimum %d)\n", fsz, IMAGE_HEADER_SIZE); + ret = -4; + goto close_img; + } + sz = (size_t)fsz; + gImage = (uintptr_t)malloc(sz); if (((void*)gImage) == NULL) { wolfBoot_printf("failed to malloc %zu bytes for image\n", sz); ret = -1; goto close_img; } + gImageSize = sz; bread = fread((void*)gImage, 1, sz, img); if (bread != sz) { From a2d5543ebb2ebe73fd1b66d0e658ce8f821cf9fd Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 10:32:13 +0200 Subject: [PATCH 05/21] F-9740: update_disk: FSP low-mem rejection falls back to other slot The WOLFBOOT_FSP low-memory size check was the only per-slot rejection in the boot retry loop that used a bare break, so an image whose header-declared fw_size exceeded the tolum window aborted the boot instead of trying the other slot. Every other rejection in the same loop switches partitions and retries; match that. --- src/update_disk.c | 3 +- tools/unit-tests/Makefile | 12 +- tools/unit-tests/unit-update-disk-fsp.c | 277 ++++++++++++++++++++++++ 3 files changed, 290 insertions(+), 2 deletions(-) create mode 100644 tools/unit-tests/unit-update-disk-fsp.c diff --git a/src/update_disk.c b/src/update_disk.c index a0a87cc95c..66ada95228 100644 --- a/src/update_disk.c +++ b/src/update_disk.c @@ -702,7 +702,8 @@ void RAMFUNCTION wolfBoot_start(void) (uint32_t)(uintptr_t)load_address)) { wolfBoot_printf("Image size %u doesn't fit in low memory\r\n", os_image.fw_size); - break; + selected ^= 1; + continue; } /* Log memory load */ x86_log_memory_load((uint32_t)(uintptr_t)load_address, diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index bbafc640e3..6d2ada2db0 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -63,7 +63,7 @@ TESTS:=unit-parser unit-parser-large-header unit-fdt unit-extflash unit-string \ unit-update-flash-hook \ unit-update-flash-self-update \ unit-update-flash-enc unit-update-flash-enc-full unit-update-ram unit-update-ram-uboot unit-update-ram-enc unit-update-ram-enc-nopart unit-update-ram-nofixed unit-update-ram-noramboot unit-update-flash-hwswap unit-pkcs11_store unit-psa_store unit-wolfhsm_flash_hal unit-disk \ - unit-update-disk unit-update-disk-oob unit-update-disk-fit unit-multiboot unit-boot-x86-fsp unit-loader-tpm-init unit-qspi-flash unit-fwtpm-stub unit-tpm-rsa-exp \ + unit-update-disk unit-update-disk-fsp unit-update-disk-oob unit-update-disk-fit unit-multiboot unit-boot-x86-fsp unit-loader-tpm-init unit-qspi-flash unit-fwtpm-stub unit-tpm-rsa-exp \ unit-image-nopart unit-image-sha384 unit-image-sha3-384 unit-image-dts \ unit-image-dts-sha384 unit-image-dts-sha3-384 unit-store-sbrk \ unit-tpm-blob unit-policy-create unit-policy-sign unit-rot-auth unit-sdhci-response-bits \ @@ -899,6 +899,16 @@ unit-update-flash-hwswap: ../../include/target.h unit-update-flash-hwswap.c unit-update-disk: ../../include/target.h unit-update-disk.c gcc -o $@ unit-update-disk.c $(CFLAGS) $(LDFLAGS) +# WOLFBOOT_FSP (x86) boot path of update_disk.c: the low-memory (tolum) +# size check must reject a slot and fall back to the other one, like every +# other per-slot rejection in the retry loop. +unit-update-disk-fsp:CFLAGS+=-DMOCK_PARTITIONS -DPRINTF_ENABLED -DWOLFBOOT_FSP \ + -DUCODE0_ADDRESS=0 -DWOLFBOOT_LOAD_BASE=0x100000 \ + -DWOLFBOOT_ORIGIN=MOCK_ADDRESS_BOOT \ + -DBOOTLOADER_PARTITION_SIZE=WOLFBOOT_PARTITION_SIZE +unit-update-disk-fsp: ../../include/target.h unit-update-disk-fsp.c + gcc -o $@ unit-update-disk-fsp.c $(CFLAGS) $(LDFLAGS) + unit-update-disk-oob: ../../include/target.h unit-update-disk-oob.c gcc -o $@ unit-update-disk-oob.c $(CFLAGS) $(LDFLAGS) diff --git a/tools/unit-tests/unit-update-disk-fsp.c b/tools/unit-tests/unit-update-disk-fsp.c new file mode 100644 index 0000000000..8a775afb0f --- /dev/null +++ b/tools/unit-tests/unit-update-disk-fsp.c @@ -0,0 +1,277 @@ +/* unit-update-disk-fsp.c + * + * Unit tests for the WOLFBOOT_FSP (x86 FSP) boot path of update_disk.c: + * the low-memory (tolum) size check must reject a slot and try the other + * one, like every other per-slot rejection in the retry loop. + */ + +#define WOLFBOOT_UPDATE_DISK +#define WOLFBOOT_SELF_UPDATE_MONOLITHIC +#define RAM_CODE +#define WOLFBOOT_SELF_HEADER +#define IMAGE_HEADER_SIZE 256 +#define BOOT_PART_A 0 +#define BOOT_PART_B 1 +#define MOCK_ADDRESS_BOOT 0xCD000000 + +#include +#include +#include +#include + +#include "hal.h" +#include "target.h" +#include "wolfboot/wolfboot.h" +#include "image.h" +#include "loader.h" +#include "stage2_params.h" +#include "x86/common.h" + +#define TEST_PAYLOAD_SIZE 64 + +static uint8_t load_buffer[TEST_PAYLOAD_SIZE]; +#define WOLFBOOT_LOAD_ADDRESS ((uintptr_t)load_buffer) + +static uint8_t part_a_image[IMAGE_HEADER_SIZE + TEST_PAYLOAD_SIZE]; +static uint8_t part_b_image[IMAGE_HEADER_SIZE + TEST_PAYLOAD_SIZE]; +static int mock_do_boot_called; +static const uint32_t *mock_boot_address; +static struct stage2_parameter mock_stage2_params; + +static void set_u16_le(uint8_t *dst, uint16_t value) +{ + dst[0] = (uint8_t)(value & 0xFF); + dst[1] = (uint8_t)(value >> 8); +} + +static void set_u32_le(uint8_t *dst, uint32_t value) +{ + dst[0] = (uint8_t)(value & 0xFF); + dst[1] = (uint8_t)((value >> 8) & 0xFF); + dst[2] = (uint8_t)((value >> 16) & 0xFF); + dst[3] = (uint8_t)((value >> 24) & 0xFF); +} + +/* fw_size may legitimately claim more than the bytes actually present in + * the partition buffer: the low-memory check must reject such a slot + * before the payload is read. */ +static void build_image(uint8_t *image, uint32_t version, uint32_t fw_size, + uint8_t fill) +{ + memset(image, 0, IMAGE_HEADER_SIZE + TEST_PAYLOAD_SIZE); + set_u32_le(image, WOLFBOOT_MAGIC); + set_u32_le(image + sizeof(uint32_t), fw_size); + set_u16_le(image + IMAGE_HEADER_OFFSET, HDR_VERSION); + set_u16_le(image + IMAGE_HEADER_OFFSET + sizeof(uint16_t), 4); + set_u32_le(image + IMAGE_HEADER_OFFSET + 2 * sizeof(uint16_t), version); + memset(image + IMAGE_HEADER_SIZE, fill, TEST_PAYLOAD_SIZE); +} + +static void reset_mocks(void) +{ + memset(load_buffer, 0, sizeof(load_buffer)); + build_image(part_a_image, 7, TEST_PAYLOAD_SIZE, 0xA1); + build_image(part_b_image, 7, TEST_PAYLOAD_SIZE, 0xB2); + mock_do_boot_called = 0; + mock_boot_address = NULL; + /* The image must fit between load_address and tolum: exactly one + * TEST_PAYLOAD_SIZE payload. */ + mock_stage2_params.tolum = + (uint32_t)((uintptr_t)load_buffer + TEST_PAYLOAD_SIZE); + wolfBoot_panicked = 0; +} + +/* --- mocks ---------------------------------------------------------- */ + +int disk_init(int drv) +{ + (void)drv; + return 0; +} + +int disk_open(int drv) +{ + (void)drv; + return 0; +} + +void disk_close(int drv) +{ + (void)drv; +} + +int disk_part_read(int drv, int part, uint64_t off, uint64_t sz, uint8_t *buf) +{ + uint8_t *image; + uint64_t max = IMAGE_HEADER_SIZE + TEST_PAYLOAD_SIZE; + + (void)drv; + image = (part == BOOT_PART_B) ? part_b_image : part_a_image; + if ((off > max) || (sz > (max - off))) + return -1; + memcpy(buf, image + off, (size_t)sz); + return (int)sz; +} + +uint32_t wolfBoot_get_blob_version(uint8_t *blob) +{ + uint8_t *p = blob + IMAGE_HEADER_OFFSET; + uint8_t *end = blob + IMAGE_HEADER_SIZE; + uint16_t type; + uint16_t len; + uint32_t version = 0; + + while (((uintptr_t)p + 4) <= (uintptr_t)end) { + type = (uint16_t)(p[0] | (p[1] << 8)); + if (type == 0) + break; + len = (uint16_t)(p[2] | (p[3] << 8)); + if (type == HDR_VERSION) { + memcpy(&version, p + 4, sizeof(version)); + break; + } + p += 4 + len; + } + return version; +} + +int wolfBoot_open_image_address(struct wolfBoot_image* img, uint8_t* image) +{ + uint32_t magic; + uint32_t fw_size; + + memcpy(&magic, image, sizeof(magic)); + if (magic != WOLFBOOT_MAGIC) + return -1; + memset(img, 0, sizeof(*img)); + img->hdr = image; + memcpy(&fw_size, image + sizeof(uint32_t), sizeof(fw_size)); + img->fw_size = fw_size; + img->fw_base = image + IMAGE_HEADER_SIZE; + img->hdr_ok = 1; + return 0; +} + +int wolfBoot_verify_integrity(struct wolfBoot_image* img) +{ + img->sha_ok = 1; + return 0; +} + +int wolfBoot_verify_authenticity(struct wolfBoot_image* img) +{ + img->signature_ok = 1; + return 0; +} + +int wolfBoot_get_dts_size(void *dts_addr, uint32_t capacity) +{ + (void)capacity; + (void)dts_addr; + return -1; +} + +struct stage2_parameter *stage2_get_parameters(void) +{ + return &mock_stage2_params; +} + +void x86_log_memory_load(uint32_t start, uint32_t end, const char *name) +{ + (void)start; + (void)end; + (void)name; +} + +void hal_prepare_boot(void) +{ +} + +int hal_flash_protect(haladdr_t address, int len) +{ + (void)address; + (void)len; + return 0; +} + +void do_boot(const uint32_t *address) +{ + mock_do_boot_called++; + mock_boot_address = address; +} + +#include "update_disk.c" + +/* --- tests ---------------------------------------------------------- */ + +START_TEST(test_fsp_oversized_slot_falls_back_to_other_slot) +{ + /* Slot A declares a payload that does not fit in low memory; slot B + * holds a good image. Boot must continue with B instead of aborting. */ + reset_mocks(); + build_image(part_a_image, 7, TEST_PAYLOAD_SIZE * 2, 0xA1); + build_image(part_b_image, 7, TEST_PAYLOAD_SIZE, 0xB2); + + wolfBoot_start(); + + ck_assert_int_eq(wolfBoot_panicked, 0); + ck_assert_int_eq(mock_do_boot_called, 1); + ck_assert_ptr_eq(mock_boot_address, (const uint32_t *)WOLFBOOT_LOAD_ADDRESS); + ck_assert_int_eq(memcmp(load_buffer, part_b_image + IMAGE_HEADER_SIZE, + TEST_PAYLOAD_SIZE), 0); +} +END_TEST + +START_TEST(test_fsp_both_slots_oversized_panics) +{ + /* Neither slot fits: the retry loop must exhaust and panic. */ + reset_mocks(); + build_image(part_a_image, 7, TEST_PAYLOAD_SIZE * 2, 0xA1); + build_image(part_b_image, 7, TEST_PAYLOAD_SIZE * 2, 0xB2); + + wolfBoot_start(); + + ck_assert_int_gt(wolfBoot_panicked, 0); + ck_assert_int_eq(mock_do_boot_called, 0); +} +END_TEST + +START_TEST(test_fsp_fitting_slot_boots) +{ + /* Both slots fit and versions are equal: primary (A) boots. */ + reset_mocks(); + + wolfBoot_start(); + + ck_assert_int_eq(wolfBoot_panicked, 0); + ck_assert_int_eq(mock_do_boot_called, 1); + ck_assert_ptr_eq(mock_boot_address, (const uint32_t *)WOLFBOOT_LOAD_ADDRESS); + ck_assert_int_eq(memcmp(load_buffer, part_a_image + IMAGE_HEADER_SIZE, + TEST_PAYLOAD_SIZE), 0); +} +END_TEST + +Suite *wolfboot_suite(void) +{ + Suite *s = suite_create("wolfBoot"); + TCase *tc = tcase_create("update-disk-fsp"); + + tcase_add_test(tc, test_fsp_oversized_slot_falls_back_to_other_slot); + tcase_add_test(tc, test_fsp_both_slots_oversized_panics); + tcase_add_test(tc, test_fsp_fitting_slot_boots); + suite_add_tcase(s, tc); + + return s; +} + +int main(void) +{ + int fails; + Suite *s = wolfboot_suite(); + SRunner *sr = srunner_create(s); + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + return fails; +} From d244fd9b7e7a58aee7b7d4aa2ba8ed50ad1892e4 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 10:44:34 +0200 Subject: [PATCH 06/21] F-9745: x86_64_efi: inclusive mem path end, reject zero-size image The UEFI MEMMAP_DEVICE_PATH EndingAddress is inclusive (last valid byte), but x86_64_efi_do_boot() set it to boot_addr + size, describing every image as one byte longer than it is; the AArch64 sibling already uses size - 1. A zero-size image would underflow that computation and hand an empty range to LoadImage, so reject it up front, as the AArch64 sibling does. The local panic() returns to the unit test under UNIT_TEST so the zero-size path is observable. --- hal/x86_64_efi.c | 19 +++- tools/unit-tests/unit-efi-x86-open-image.c | 113 +++++++++++++++++++++ 2 files changed, 131 insertions(+), 1 deletion(-) diff --git a/hal/x86_64_efi.c b/hal/x86_64_efi.c index c9c7eb2db5..8b80af0ae8 100644 --- a/hal/x86_64_efi.c +++ b/hal/x86_64_efi.c @@ -96,7 +96,14 @@ void *hal_get_dts_update_address(void) static void panic() { +#ifdef UNIT_TEST + /* The unit test observes wolfBoot_panicked and needs to get back; + * on target this never returns. */ + wolfBoot_panic(); + return; +#else while(1) {} +#endif } void RAMFUNCTION x86_64_efi_do_boot(uint32_t *boot_addr, uint8_t *dts_address) @@ -116,6 +123,14 @@ void RAMFUNCTION x86_64_efi_do_boot(uint32_t *boot_addr, uint8_t *dts_address) size = (uint32_t *)(manifest + 4); + /* Guard against a zero-size image: EndingAddress below would underflow + * and an empty range would be handed to LoadImage. */ + if (*size == 0) { + wolfBoot_printf("invalid zero-size image\n"); + panic(); + return; /* Never reached on target, where panic() does not return */ + } + /* Authenticated kernel command line from the verified image's HDR_CMDLINE * TLV (covered by the signature); NULL if the image carries none. */ kernel_cmdline = wolfBoot_efi_get_cmdline(manifest, &kernel_cmdline_bytes); @@ -124,7 +139,9 @@ void RAMFUNCTION x86_64_efi_do_boot(uint32_t *boot_addr, uint8_t *dts_address) mem_path_device->Header.SubType = EFI_DEVICE_PATH_PROTOCOL_MEM_SUBTYPE; mem_path_device->MemoryType = EfiLoaderData; mem_path_device->StartingAddress = (EFI_PHYSICAL_ADDRESS)boot_addr; - mem_path_device->EndingAddress = (EFI_PHYSICAL_ADDRESS)((uint8_t*)boot_addr+*size); + /* MEMMAP_DEVICE_PATH EndingAddress is inclusive (last valid byte). */ + mem_path_device->EndingAddress = + (EFI_PHYSICAL_ADDRESS)((uintptr_t)boot_addr + *size - 1); SetDevicePathNodeLength(&mem_path_device->Header, sizeof(MEMMAP_DEVICE_PATH)); diff --git a/tools/unit-tests/unit-efi-x86-open-image.c b/tools/unit-tests/unit-efi-x86-open-image.c index b4f9624852..5caa045bdb 100644 --- a/tools/unit-tests/unit-efi-x86-open-image.c +++ b/tools/unit-tests/unit-efi-x86-open-image.c @@ -324,6 +324,45 @@ static EFI_STATUS EFIAPI mock_free_pages_fn(EFI_PHYSICAL_ADDRESS Memory, return EFI_SUCCESS; } +/* --- x86_64_efi_do_boot() mocks -------------------------------------- */ + +static EFI_SYSTEM_TABLE mock_st; +static EFI_HANDLE mock_image_handle; +static int mock_load_image_calls; +static int mock_start_image_calls; +static MEMMAP_DEVICE_PATH captured_dp[2]; +static EFI_PHYSICAL_ADDRESS captured_src_addr; +static UINTN captured_src_size; + +/* Captures the memory device path handed to LoadImage so the tests can + * check its address range. */ +static EFI_STATUS EFIAPI mock_load_image(BOOLEAN BootPolicy, + EFI_HANDLE ParentImageHandle, EFI_DEVICE_PATH_PROTOCOL *DevicePath, + VOID *SourceBuffer, UINTN SourceSize, EFI_HANDLE *ImageHandle) +{ + (void)BootPolicy; + (void)ParentImageHandle; + if (DevicePath != NULL) { + memcpy(captured_dp, DevicePath, sizeof(captured_dp)); + captured_src_addr = (EFI_PHYSICAL_ADDRESS)(uintptr_t)SourceBuffer; + captured_src_size = SourceSize; + } + if (ImageHandle != NULL) + *ImageHandle = (EFI_HANDLE)0xBEEF; + mock_load_image_calls++; + return EFI_SUCCESS; +} + +static EFI_STATUS EFIAPI mock_start_image(EFI_HANDLE ImageHandle, + UINTN *ExitDataSize, CHAR16 **ExitData) +{ + (void)ImageHandle; + (void)ExitDataSize; + (void)ExitData; + mock_start_image_calls++; + return EFI_SUCCESS; +} + /* wolfBoot symbols referenced by the HAL file, not exercised by the tests. */ int wolfBoot_printf(const char *fmt, ...) { @@ -391,10 +430,21 @@ static void setup(void) memset(&mock_bs, 0, sizeof(mock_bs)); mock_bs.AllocatePages = mock_allocate_pages; mock_bs.FreePages = mock_free_pages_fn; + mock_bs.LoadImage = mock_load_image; + mock_bs.StartImage = mock_start_image; mock_alloc_fail = 0; mock_free_pages = 0; mock_close_count = 0; mock_read_status = EFI_SUCCESS; + mock_load_image_calls = 0; + mock_start_image_calls = 0; + wolfBoot_panicked = 0; + + /* x86_64_efi_do_boot() reads these statics; efi_main() would set them + * on target. */ + mock_st.BootServices = &mock_bs; + gSystemTable = &mock_st; + gImageHandle = &mock_image_handle; memset(&mock_file_proto, 0, sizeof(mock_file_proto)); mock_file_proto.Revision = 0x00120000; @@ -586,6 +636,67 @@ START_TEST(test_open_image_header_boundary) } END_TEST +/* The memory device path must describe exactly the image bytes: the + * UEFI MEMMAP_DEVICE_PATH EndingAddress is inclusive (last valid byte), + * so it is boot_addr + size - 1, and the path must end with an end node. */ +START_TEST(test_do_boot_mem_path_end_inclusive) +{ + uint32_t fw_size = 1024; + uint32_t *boot_addr; + uint8_t image[IMAGE_HEADER_SIZE + 1024]; + int i; + + memset(image, 0, sizeof(image)); + memcpy(image, "WOLF", 4); + memcpy(image + 4, &fw_size, sizeof(fw_size)); + for (i = 0; i < 1024; i++) + image[IMAGE_HEADER_SIZE + i] = (uint8_t)(i & 0xFF); + boot_addr = (uint32_t *)(image + IMAGE_HEADER_SIZE); + + x86_64_efi_do_boot(boot_addr, NULL); + + ck_assert_int_eq(wolfBoot_panicked, 0); + ck_assert_int_eq(mock_load_image_calls, 1); + ck_assert_int_eq(mock_start_image_calls, 1); + ck_assert_uint_eq(captured_src_addr, + (EFI_PHYSICAL_ADDRESS)(uintptr_t)boot_addr); + ck_assert_uint_eq(captured_src_size, fw_size); + ck_assert_uint_eq(captured_dp[0].Header.Type, + EFI_DEVICE_PATH_PROTOCOL_HW_TYPE); + ck_assert_uint_eq(captured_dp[0].Header.SubType, + EFI_DEVICE_PATH_PROTOCOL_MEM_SUBTYPE); + ck_assert_uint_eq(captured_dp[0].StartingAddress, + (EFI_PHYSICAL_ADDRESS)(uintptr_t)boot_addr); + ck_assert_uint_eq(captured_dp[0].EndingAddress, + (EFI_PHYSICAL_ADDRESS)((uintptr_t)boot_addr + fw_size - 1)); + /* the path must end with an end node */ + ck_assert_uint_eq(captured_dp[1].Header.Type, END_DEVICE_PATH_TYPE); + ck_assert_uint_eq(captured_dp[1].Header.SubType, + END_ENTIRE_DEVICE_PATH_SUBTYPE); +} +END_TEST + +/* A zero-size image must be rejected before LoadImage: the inclusive end + * address would underflow and an empty range would be loaded. */ +START_TEST(test_do_boot_zero_size_panics) +{ + uint32_t fw_size = 0; + uint32_t *boot_addr; + uint8_t image[IMAGE_HEADER_SIZE + 16]; + + memset(image, 0, sizeof(image)); + memcpy(image, "WOLF", 4); + memcpy(image + 4, &fw_size, sizeof(fw_size)); + boot_addr = (uint32_t *)(image + IMAGE_HEADER_SIZE); + + x86_64_efi_do_boot(boot_addr, NULL); + + ck_assert_int_gt(wolfBoot_panicked, 0); + ck_assert_int_eq(mock_load_image_calls, 0); + ck_assert_int_eq(mock_start_image_calls, 0); +} +END_TEST + Suite *efi_x86_open_image_suite(void) { Suite *s = suite_create("efi-x86-open-image"); @@ -598,6 +709,8 @@ Suite *efi_x86_open_image_suite(void) tcase_add_test(tc, test_open_image_read_failure); tcase_add_test(tc, test_open_image_alloc_failure); tcase_add_test(tc, test_open_image_header_boundary); + tcase_add_test(tc, test_do_boot_mem_path_end_inclusive); + tcase_add_test(tc, test_do_boot_zero_size_panics); suite_add_tcase(s, tc); return s; From eb3d587fe27f8f46fc284c8b665e5ddf939d707f Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 10:57:08 +0200 Subject: [PATCH 07/21] F-9746: x86_64_efi: fix do_boot prototype mismatch, drop unused param boot_x86_64.c declared x86_64_efi_do_boot(uint8_t *) while the HAL defines (uint32_t *, uint8_t *): the linker connected the incompatible pair and the call was undefined behavior, mostly latent because the second parameter was discarded. Match the AArch64 sibling: single const uint32_t *boot_addr in the declaration, definition and call, and remove the unused dts_address parameter. --- hal/x86_64_efi.c | 15 ++++----- src/boot_x86_64.c | 4 +-- tools/unit-tests/unit-efi-x86-open-image.c | 39 ++++++++++++++++++++-- 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/hal/x86_64_efi.c b/hal/x86_64_efi.c index 8b80af0ae8..c99dcbd693 100644 --- a/hal/x86_64_efi.c +++ b/hal/x86_64_efi.c @@ -106,12 +106,10 @@ static void panic() #endif } -void RAMFUNCTION x86_64_efi_do_boot(uint32_t *boot_addr, uint8_t *dts_address) +void RAMFUNCTION x86_64_efi_do_boot(const uint32_t *boot_addr) { - uint32_t *size; - uint8_t* manifest = ((uint8_t*)boot_addr) - IMAGE_HEADER_SIZE; - - (void)dts_address; /* Unused for now */ + const uint32_t *size; + const uint8_t* manifest = ((const uint8_t*)boot_addr) - IMAGE_HEADER_SIZE; MEMMAP_DEVICE_PATH mem_path_device[2]; EFI_HANDLE kernelImageHandle; @@ -121,7 +119,7 @@ void RAMFUNCTION x86_64_efi_do_boot(uint32_t *boot_addr, uint8_t *dts_address) EFI_LOADED_IMAGE *kernel_li = NULL; EFI_GUID lipGuid = EFI_LOADED_IMAGE_PROTOCOL_GUID; - size = (uint32_t *)(manifest + 4); + size = (const uint32_t *)(manifest + 4); /* Guard against a zero-size image: EndingAddress below would underflow * and an empty range would be handed to LoadImage. */ @@ -138,7 +136,8 @@ void RAMFUNCTION x86_64_efi_do_boot(uint32_t *boot_addr, uint8_t *dts_address) mem_path_device->Header.Type = EFI_DEVICE_PATH_PROTOCOL_HW_TYPE; mem_path_device->Header.SubType = EFI_DEVICE_PATH_PROTOCOL_MEM_SUBTYPE; mem_path_device->MemoryType = EfiLoaderData; - mem_path_device->StartingAddress = (EFI_PHYSICAL_ADDRESS)boot_addr; + mem_path_device->StartingAddress = + (EFI_PHYSICAL_ADDRESS)(uintptr_t)boot_addr; /* MEMMAP_DEVICE_PATH EndingAddress is inclusive (last valid byte). */ mem_path_device->EndingAddress = (EFI_PHYSICAL_ADDRESS)((uintptr_t)boot_addr + *size - 1); @@ -153,7 +152,7 @@ void RAMFUNCTION x86_64_efi_do_boot(uint32_t *boot_addr, uint8_t *dts_address) 0, /* bool */ gImageHandle, (EFI_DEVICE_PATH*)mem_path_device, - boot_addr, + (void*)(uintptr_t)boot_addr, *size, &kernelImageHandle); if (status != EFI_SUCCESS) { diff --git a/src/boot_x86_64.c b/src/boot_x86_64.c index 865d866540..6b5b4c7c4f 100644 --- a/src/boot_x86_64.c +++ b/src/boot_x86_64.c @@ -34,7 +34,7 @@ extern unsigned int __bss_end__; static volatile unsigned int cpu_id; extern unsigned int *END_STACK; -extern void RAMFUNCTION x86_64_efi_do_boot(uint8_t *kernel); +extern void RAMFUNCTION x86_64_efi_do_boot(const uint32_t *boot_addr); #if defined(MMU) || defined(WOLFBOOT_FDT) void RAMFUNCTION do_boot(const uint32_t *app_offset, const uint32_t* dts_offset) @@ -42,7 +42,7 @@ void RAMFUNCTION do_boot(const uint32_t *app_offset, const uint32_t* dts_offset) void RAMFUNCTION do_boot(const uint32_t *app_offset) #endif { - x86_64_efi_do_boot((uint8_t *)app_offset); + x86_64_efi_do_boot(app_offset); } #endif /* TARGET_X86_64_EFI */ diff --git a/tools/unit-tests/unit-efi-x86-open-image.c b/tools/unit-tests/unit-efi-x86-open-image.c index 5caa045bdb..58cb7b080e 100644 --- a/tools/unit-tests/unit-efi-x86-open-image.c +++ b/tools/unit-tests/unit-efi-x86-open-image.c @@ -388,6 +388,11 @@ uint16_t wolfBoot_find_header(uint8_t *haystack, uint16_t type, uint8_t **ptr) /* Pull in the code under test (its statics become visible here). */ #include "../../hal/x86_64_efi.c" +/* The caller too: with the definition already in this translation unit, + * its extern declaration is checked against it, so a prototype drift + * between the two files is a compile error. */ +#include "../../src/boot_x86_64.c" + /* The tests pass their own CHAR16 filename (the mock ignores the content). * The build rule uses -fshort-wchar like the real x86_64_efi build, so * efi_main's L"..." literals are valid 16-bit CHAR16 strings here too. */ @@ -653,7 +658,7 @@ START_TEST(test_do_boot_mem_path_end_inclusive) image[IMAGE_HEADER_SIZE + i] = (uint8_t)(i & 0xFF); boot_addr = (uint32_t *)(image + IMAGE_HEADER_SIZE); - x86_64_efi_do_boot(boot_addr, NULL); + x86_64_efi_do_boot(boot_addr); ck_assert_int_eq(wolfBoot_panicked, 0); ck_assert_int_eq(mock_load_image_calls, 1); @@ -689,7 +694,7 @@ START_TEST(test_do_boot_zero_size_panics) memcpy(image + 4, &fw_size, sizeof(fw_size)); boot_addr = (uint32_t *)(image + IMAGE_HEADER_SIZE); - x86_64_efi_do_boot(boot_addr, NULL); + x86_64_efi_do_boot(boot_addr); ck_assert_int_gt(wolfBoot_panicked, 0); ck_assert_int_eq(mock_load_image_calls, 0); @@ -697,6 +702,35 @@ START_TEST(test_do_boot_zero_size_panics) } END_TEST +/* The caller (do_boot in src/boot_x86_64.c) must hand its app_offset to + * the HAL unmodified: same pointer in LoadImage's SourceBuffer and in the + * memory device path. A caller/callee prototype mismatch (e.g. the old + * uint8_t * declaration) would truncate or reinterpret it. */ +START_TEST(test_do_boot_transfers_app_offset) +{ + uint32_t fw_size = 1024; + uint32_t *boot_addr; + uint8_t image[IMAGE_HEADER_SIZE + 1024]; + int i; + + memset(image, 0, sizeof(image)); + memcpy(image, "WOLF", 4); + memcpy(image + 4, &fw_size, sizeof(fw_size)); + for (i = 0; i < 1024; i++) + image[IMAGE_HEADER_SIZE + i] = (uint8_t)(i & 0xFF); + boot_addr = (uint32_t *)(image + IMAGE_HEADER_SIZE); + + do_boot(boot_addr); + + ck_assert_int_eq(wolfBoot_panicked, 0); + ck_assert_int_eq(mock_load_image_calls, 1); + ck_assert_uint_eq(captured_src_addr, + (EFI_PHYSICAL_ADDRESS)(uintptr_t)boot_addr); + ck_assert_uint_eq(captured_dp[0].StartingAddress, + (EFI_PHYSICAL_ADDRESS)(uintptr_t)boot_addr); +} +END_TEST + Suite *efi_x86_open_image_suite(void) { Suite *s = suite_create("efi-x86-open-image"); @@ -711,6 +745,7 @@ Suite *efi_x86_open_image_suite(void) tcase_add_test(tc, test_open_image_header_boundary); tcase_add_test(tc, test_do_boot_mem_path_end_inclusive); tcase_add_test(tc, test_do_boot_zero_size_panics); + tcase_add_test(tc, test_do_boot_transfers_app_offset); suite_add_tcase(s, tc); return s; From 785a285ca751b220ebfdfce17d5ce76e065808f9 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 11:13:32 +0200 Subject: [PATCH 08/21] F-11025: update_flash: document deliberately inert PCR extend block The PCR-extension block in wolfBoot_unlock_disk() is guarded with !defined(ARCH_SIM) while the function itself only builds for ARCH_SIM, so it can never compile. The exclusion is deliberate (eb2978ab: do not extend the unseal PCR on the simulator, or the secret becomes un-unsealable), not an oversight. Clarify the comment with the intended build scope instead of removing the block: the code and the WOLFBOOT_NO_UNSEAL_PCR_EXTEND option exist for the day the unlock-disk path is ported to a non-sim target. --- src/update_flash.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/update_flash.c b/src/update_flash.c index 5f398d7130..c129eff574 100644 --- a/src/update_flash.c +++ b/src/update_flash.c @@ -1499,7 +1499,11 @@ int wolfBoot_unlock_disk(void) /* TODO: Unlock disk */ - /* Extend a PCR from the mask to prevent future unsealing */ + /* Extend a PCR from the mask to prevent future unsealing. + * Non-sim only: extending on the simulator would lock the + * PCR and block future unseals (eb2978ab). The function is + * ARCH_SIM-only today, so the block is inert until the + * unlock path is ported. */ #if !defined(ARCH_SIM) && !defined(WOLFBOOT_NO_UNSEAL_PCR_EXTEND) { uint32_t pcrMask; From 55711243c66a0b10bb5d10fdfb1a0e5621ec4f52 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 11:24:13 +0200 Subject: [PATCH 09/21] F-11026: image: declare wolfHSM DER sig length as word32 wc_ecc_rs_raw_to_sig() takes a word32* outlen, but the wolfHSM client/server path in wolfBoot_verify_signature_ecc() declared the buffer length as size_t and cast the pointer. On a 64-bit big-endian target the API reads the high (zero) half, so the DER conversion sees outlen 0 and the write-back lands in the wrong half of the size_t. Declare tmpSigSz as word32 and pass &tmpSigSz directly; this also matches the word32 sigLen of wc_ecc_verify_hash() below. Add a host compile check for the WOLFBOOT_ENABLE_WOLFHSM_CLIENT build of image.c, which unit CI did not cover (only the PIC32CZ cross build). --- src/image.c | 4 +- tools/unit-tests/Makefile | 8 ++++ .../unit-image-wolfhsm-client-build.py | 46 +++++++++++++++++++ 3 files changed, 56 insertions(+), 2 deletions(-) create mode 100755 tools/unit-tests/unit-image-wolfhsm-client-build.py diff --git a/src/image.c b/src/image.c index 3492d2928f..8914ba9940 100644 --- a/src/image.c +++ b/src/image.c @@ -326,7 +326,7 @@ static void wolfBoot_verify_signature_ecc(uint8_t key_slot, defined(WOLFBOOT_ENABLE_WOLFHSM_SERVER) uint8_t tmpSigBuf[ECC_MAX_SIG_SIZE] = {0}; - size_t tmpSigSz = sizeof(tmpSigBuf); + word32 tmpSigSz = sizeof(tmpSigBuf); #if defined(WOLFBOOT_ENABLE_WOLFHSM_CLIENT) || \ (defined(WOLFBOOT_ENABLE_WOLFHSM_SERVER) && \ @@ -386,7 +386,7 @@ static void wolfBoot_verify_signature_ecc(uint8_t key_slot, and left-zero-padded, and the conversion strips the padding. */ ret = wc_ecc_rs_raw_to_sig(sig, (word32)point_sz, &sig[point_sz], (word32)point_sz, - (byte*)&tmpSigBuf, (word32*)&tmpSigSz); + (byte*)&tmpSigBuf, &tmpSigSz); /* Verify the (temporary) DER representation of the signature */ if (ret == 0) { VERIFY_FN(img, &verify_res, wc_ecc_verify_hash, tmpSigBuf, tmpSigSz, diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 6d2ada2db0..6efd8e0b86 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -223,6 +223,7 @@ run: $(TESTS) python3 unit-sign-delta-basehash-cleanup.py || exit 1 python3 unit-delta-sector-align.py || exit 1 python3 unit-elf-scatter-db-build.py || exit 1 + python3 unit-image-wolfhsm-client-build.py || exit 1 python3 unit-x86-fsp-stage1auth-build.py || exit 1 python3 unit-sign-custom-tlv-le.py || exit 1 python3 unit-sign-custom-tlv-large.py || exit 1 @@ -844,6 +845,13 @@ unit-update-flash-delta-misalign: ../../include/target.h unit-update-flash.c unit-update-flash-elf-scatter-db: ../../include/target.h unit-update-flash.c FORCE gcc -c -o /dev/null unit-update-flash.c $(CFLAGS) +unit-image-wolfhsm-client-build:CFLAGS+=-I$(WOLFBOOT_LIB_WOLFHSM) \ + -DWOLFHSM_CFG_NO_SYS_TIME -DMOCK_PARTITIONS -DWOLFBOOT_HASH_SHA256 \ + -DWOLFBOOT_SIGN_ECC256 -DWOLFBOOT_ENABLE_WOLFHSM_CLIENT \ + -DIMAGE_HEADER_SIZE=256 -D__WOLFBOOT +unit-image-wolfhsm-client-build: ../../include/target.h ../../src/image.c FORCE + gcc -c -o /dev/null ../../src/image.c $(CFLAGS) + unit-update-flash-enc:CFLAGS+=-DMOCK_PARTITIONS -DWOLFBOOT_NO_SIGN -DUNIT_TEST_AUTH \ -DWOLFBOOT_HASH_SHA256 -DPRINTF_ENABLED -DEXT_FLASH -DPART_UPDATE_EXT \ -DPART_SWAP_EXT -DEXT_ENCRYPTED -DENCRYPT_WITH_CHACHA -DHAVE_CHACHA \ diff --git a/tools/unit-tests/unit-image-wolfhsm-client-build.py b/tools/unit-tests/unit-image-wolfhsm-client-build.py new file mode 100755 index 0000000000..626ec43035 --- /dev/null +++ b/tools/unit-tests/unit-image-wolfhsm-client-build.py @@ -0,0 +1,46 @@ +#!/usr/bin/env python3 +# unit-image-wolfhsm-client-build.py +# +# Compile check for the WOLFBOOT_ENABLE_WOLFHSM_CLIENT path in +# wolfBoot_verify_signature_ecc() (src/image.c). The raw-to-DER +# signature conversion passes the output length to +# wc_ecc_rs_raw_to_sig(), which expects a word32*; this branch was +# only built by the PIC32CZ cross CI, so keep it compiling in the +# host unit CI as well. +# +# 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 + +import subprocess +import sys + + +def main(): + p = subprocess.run(["make", "unit-image-wolfhsm-client-build"], + capture_output=True, text=True) + if p.returncode != 0: + print("FAIL: WOLFBOOT_ENABLE_WOLFHSM_CLIENT image.c " + "does not compile:\n") + print(p.stderr[-2000:]) + return 1 + print("PASS: WOLFBOOT_ENABLE_WOLFHSM_CLIENT image.c compiles") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From 0e2a46375018aa3f76088bde09682278d9e5ad31 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 11:53:12 +0200 Subject: [PATCH 10/21] F-11047: image: validate ELF scatter segments before flash writes wolfBoot_load_flash_image_elf() ignored read_flash_fwimage failures and used the program-header fields unchecked, so a failed read consumed indeterminate stack data and a malformed (but signed) ELF could drive an out-of-bounds source read and erase/write at an unintended destination. Check the ELF header and program header reads, then validate each PT_LOAD segment before copying: file_size fits a 32-bit length, the source stays inside the manifest image, the paddr range fits the destination address width, and the destination stays inside the boot partition (mirrors the sibling check-function validation). Check the copy result. New unit tests in unit-image-elf-scatter.c cover the load path: a valid restore (positive control) and rejections for source past fw_size, paddr range overflow, destination outside the boot partition, and a program header that cannot be read. The first three fail pre-fix (the mock flash layer also catches the two out-of-range destinations with its own address check); the phdr read-failure case consumed uninitialized stack data pre-fix (valgrind: conditional jump on uninitialised value at the is_loadable check). --- src/image.c | 64 +++++++- tools/unit-tests/unit-image-elf-scatter.c | 189 +++++++++++++++++++++- 2 files changed, 245 insertions(+), 8 deletions(-) diff --git a/src/image.c b/src/image.c index 8914ba9940..c3ea6566bb 100644 --- a/src/image.c +++ b/src/image.c @@ -2438,7 +2438,11 @@ int wolfBoot_load_flash_image_elf(int part, unsigned long* entry_out, int ext_fl /* Get the elf header from the image into a local buffer. We may overread * the buffer depending on architecture */ memset(elfHdrBuf, 0, sizeof(elfHdrBuf)); - read_flash_fwimage(&boot, 0, elfHdrBuf, sizeof(elfHeaderMaxBuf)); + if (read_flash_fwimage(&boot, 0, elfHdrBuf, + sizeof(elfHeaderMaxBuf)) != 0) { + wolfBoot_printf("ELF: [STORE] ERROR: could not read ELF header\n"); + return -1; + } if (elf_open(elfHdrBuf, &is_elf32) != 0) { return -1; } @@ -2472,11 +2476,17 @@ int wolfBoot_load_flash_image_elf(int part, unsigned long* entry_out, int ext_fl unsigned long paddr, filesz, offset; int is_loadable; uintptr_t load_addr; + uint64_t seg_start; /* Read the current program header into a local buffer */ if (is_elf32) { elf32_program_header p32; - read_flash_fwimage(&boot, entry_off, &p32, sizeof(p32)); + if (read_flash_fwimage(&boot, entry_off, &p32, + sizeof(p32)) != 0) { + wolfBoot_printf("ELF: [STORE] ERROR: could not read " + "program header\n"); + return -1; + } is_loadable = (p32.type == ELF_PT_LOAD); paddr = (unsigned long)p32.paddr; offset = (unsigned long)p32.offset; @@ -2485,7 +2495,12 @@ int wolfBoot_load_flash_image_elf(int part, unsigned long* entry_out, int ext_fl } else { elf64_program_header p64; - read_flash_fwimage(&boot, entry_off, &p64, sizeof(p64)); + if (read_flash_fwimage(&boot, entry_off, &p64, + sizeof(p64)) != 0) { + wolfBoot_printf("ELF: [STORE] ERROR: could not read " + "program header\n"); + return -1; + } is_loadable = (p64.type == ELF_PT_LOAD); paddr = (unsigned long)p64.paddr; offset = (unsigned long)p64.offset; @@ -2498,12 +2513,49 @@ int wolfBoot_load_flash_image_elf(int part, unsigned long* entry_out, int ext_fl return -1; } - load_addr = (uintptr_t)(paddr + BASE_OFF); + /* Validate the segment before writing: the source must stay + * inside the manifest image, the paddr range must fit the + * destination (uintptr_t) width, and the destination must stay + * inside the boot partition. Reject instead of writing. */ + if (filesz > UINT32_MAX) { + wolfBoot_printf("ELF: [STORE] ERROR: segment file_size " + "%lu does not fit a 32-bit length\n", + (unsigned long)filesz); + return -1; + } + if (offset > (uint64_t)boot.fw_size || + filesz > (uint64_t)boot.fw_size - offset) { + wolfBoot_printf("ELF: [STORE] ERROR: segment offset %lu + " + "size %lu exceeds image size %u\n", + (unsigned long)offset, + (unsigned long)filesz, boot.fw_size); + return -1; + } + seg_start = paddr + (uint64_t)BASE_OFF; + if (seg_start < paddr || + seg_start > (uint64_t)UINTPTR_MAX - filesz) { + wolfBoot_printf("ELF: [STORE] ERROR: segment paddr range " + "overflows\n"); + return -1; + } + load_addr = (uintptr_t)seg_start; + if (load_addr < (uintptr_t)boot.hdr || + load_addr + filesz > + (uintptr_t)boot.hdr + (uintptr_t)WOLFBOOT_PARTITION_SIZE) { + wolfBoot_printf("ELF: [STORE] ERROR: segment destination " + "outside boot partition\n"); + return -1; + } + wolfBoot_printf("ELF: [STORE] Writing loadable segment: " "loadaddr=0x%08lx, offset=0x%08lx, size=%lu\n", (unsigned long)load_addr, offset, filesz); - copy_flash_buffered((uintptr_t)(image + offset), load_addr, filesz, - ext_flash, ext_flash); + if (copy_flash_buffered((uintptr_t)(image + offset), load_addr, + filesz, ext_flash, ext_flash) != 0) { + wolfBoot_printf("ELF: [STORE] ERROR: could not write " + "loadable segment\n"); + return -1; + } entry_off += ph_size; } diff --git a/tools/unit-tests/unit-image-elf-scatter.c b/tools/unit-tests/unit-image-elf-scatter.c index c5c6510362..c53760d1f4 100644 --- a/tools/unit-tests/unit-image-elf-scatter.c +++ b/tools/unit-tests/unit-image-elf-scatter.c @@ -568,10 +568,184 @@ START_TEST(test_elf_scatter_paddr_beyond_pointer_width_rejected) END_TEST #endif +/* --- wolfBoot_load_flash_image_elf() (the store/restore path) --- + * + * The load function walks the same program header table and copies each + * PT_LOAD segment from the manifest (fw_base + offset) to its scattered + * destination (paddr + BASE_OFF). In this harness the boot partition is + * mmap'd at MOCK_ADDRESS_BOOT (== WOLFBOOT_PARTITION_BOOT_ADDRESS) and + * BASE_OFF is 0, so a valid destination is any address inside + * [MOCK_ADDRESS_BOOT, MOCK_ADDRESS_BOOT + WOLFBOOT_PARTITION_SIZE). + * The mock flash layer fails the test on any erase/write outside the + * known partition ranges, so an unvalidated paddr cannot walk off into + * unmapped memory here. */ + +#define LOAD_DEST (MOCK_ADDRESS_BOOT + 0x4000) + +static void set_load_paddr(uint64_t paddr) +{ + uint8_t *manifest = (uint8_t *)(uintptr_t)MOCK_ADDRESS_BOOT; + elf64_program_header *ph = (elf64_program_header *) + (manifest + IMAGE_HEADER_SIZE + sizeof(elf64_header)); + + ph->paddr = paddr; +} + +START_TEST(test_elf_scatter_load_valid_image_restores) +{ + unsigned long entry = 0; + uint8_t *manifest = (uint8_t *)(uintptr_t)MOCK_ADDRESS_BOOT; + uint8_t *source = manifest + IMAGE_HEADER_SIZE + ELF_HDR_SZ; + uint8_t *dest = (uint8_t *)(uintptr_t)LOAD_DEST; + unsigned int i; + int ret; + + map_boot_partition(); + + build_scattered_image(); + set_load_paddr(LOAD_DEST); + for (i = 0; i < SEG_SIZE; i++) { + source[i] = (uint8_t)(0x30U + i); + } + + ret = wolfBoot_load_flash_image_elf(PART_BOOT, &entry, 0); + + ck_assert_int_eq(ret, 0); + for (i = 0; i < SEG_SIZE; i++) { + ck_assert_uint_eq(dest[i], source[i]); + } + + unmap_boot_partition(); +} +END_TEST + +/* A segment whose file layout (offset + file_size) extends past the + * manifest image must be rejected before any flash write. Pre-fix the + * source read walked past fw_size and the copy still "succeeded" + * (ret 0). */ +START_TEST(test_elf_scatter_load_segment_beyond_fw_size_rejected) +{ + unsigned long entry = 0; + struct seg_spec segs[1]; + int ret; + + map_boot_partition(); + + memset(seg2_flash, 0, sizeof(seg2_flash)); + segs[0].offset = ELF_HDR_SZ; + segs[0].filesz = SEG1_SIZE; /* 0x2000 > the manifest layout slack */ + segs[0].paddr = LOAD_DEST; + segs[0].payload = seg2_flash; + segs[0].fillsz = 0; + + build_scattered_image_n(segs, 1, IMG_FW_SIZE); + + ret = wolfBoot_load_flash_image_elf(PART_BOOT, &entry, 0); + + /* Pre-fix this returned 0: the mock happily erased/wrote the valid + * destination with bytes read past fw_size. */ + ck_assert_int_eq(ret, -1); + + unmap_boot_partition(); +} +END_TEST + +/* A paddr whose segment range overflows the address space must be + * rejected before any flash access. Pre-fix the wrapped load_addr drove + * the mock into its out-of-range erase check (fail("Invalid address")). */ +START_TEST(test_elf_scatter_load_paddr_range_overflow_rejected) +{ + unsigned long entry = 0; + struct seg_spec segs[1]; + int ret; + + map_boot_partition(); + + memset(seg2_flash, 0, sizeof(seg2_flash)); + segs[0].offset = ELF_HDR_SZ; + segs[0].filesz = SEG_SIZE; + segs[0].paddr = UINT64_MAX - 4; /* +SEG_SIZE wraps past UINT64_MAX */ + segs[0].payload = seg2_flash; + segs[0].fillsz = SEG_SIZE; + + build_scattered_image_n(segs, 1, IMG_FW_SIZE); + + ret = wolfBoot_load_flash_image_elf(PART_BOOT, &entry, 0); + + ck_assert_int_eq(ret, -1); + + unmap_boot_partition(); +} +END_TEST + +/* A destination outside the boot partition must be rejected before any + * flash access. Pre-fix the mock's out-of-range erase check caught it + * with fail("Invalid address"); on real hardware this is an + * erase/write at an unintended location. */ +START_TEST(test_elf_scatter_load_dest_outside_partition_rejected) +{ + unsigned long entry = 0; + struct seg_spec segs[1]; + int ret; + + map_boot_partition(); + + memset(seg2_flash, 0, sizeof(seg2_flash)); + segs[0].offset = ELF_HDR_SZ; + segs[0].filesz = SEG_SIZE; + segs[0].paddr = (uint64_t)(MOCK_ADDRESS_BOOT + WOLFBOOT_PARTITION_SIZE + + 0x1000); + segs[0].payload = seg2_flash; + segs[0].fillsz = SEG_SIZE; + + build_scattered_image_n(segs, 1, IMG_FW_SIZE); + + ret = wolfBoot_load_flash_image_elf(PART_BOOT, &entry, 0); + + ck_assert_int_eq(ret, -1); + + unmap_boot_partition(); +} +END_TEST + +/* A program header that lies past the end of the manifest image cannot + * be read; the load must abort instead of consuming the uninitialized + * header locals. The PHT sits at fw offset 64 (right after the 64-byte + * ELF header, the only offset check_scatter_format accepts), so a + * fw_size of 100 makes the 56-byte phdr read (64 + 56) run past + * fw_size. Pre-fix the read failure was ignored and p64 held + * indeterminate stack data (valgrind: conditional jump on uninitialised + * value at the is_loadable check). */ +START_TEST(test_elf_scatter_load_phdr_read_failure_rejected) +{ + unsigned long entry = 0; + struct seg_spec segs[1]; + int ret; + + map_boot_partition(); + + memset(seg2_flash, 0, sizeof(seg2_flash)); + segs[0].offset = ELF_HDR_SZ; + segs[0].filesz = SEG_SIZE; + segs[0].paddr = LOAD_DEST; + segs[0].payload = seg2_flash; + segs[0].fillsz = SEG_SIZE; + + build_scattered_image_n(segs, 1, 100); /* PHT extends past fw_size */ + + ret = wolfBoot_load_flash_image_elf(PART_BOOT, &entry, 0); + + ck_assert_int_eq(ret, -1); + + unmap_boot_partition(); +} +END_TEST + Suite *elf_scatter_suite(void) { - Suite *s = suite_create("ELF flash-scatter image check"); - TCase *tc = tcase_create("wolfBoot_check_flash_image_elf"); + Suite *s = suite_create("ELF flash-scatter image check"); + TCase *tc = tcase_create("wolfBoot_check_flash_image_elf"); + TCase *tc_load = tcase_create("wolfBoot_load_flash_image_elf"); tcase_add_test(tc, test_elf_scatter_valid_image_verifies_ok); tcase_add_test(tc, test_elf_scatter_corrupted_segment_rejected); tcase_add_test(tc, test_elf_scatter_filesz_over_32bit_rejected); @@ -582,6 +756,17 @@ Suite *elf_scatter_suite(void) #endif tcase_set_timeout(tc, 10); suite_add_tcase(s, tc); + + tcase_add_test(tc_load, test_elf_scatter_load_valid_image_restores); + tcase_add_test(tc_load, + test_elf_scatter_load_segment_beyond_fw_size_rejected); + tcase_add_test(tc_load, + test_elf_scatter_load_paddr_range_overflow_rejected); + tcase_add_test(tc_load, + test_elf_scatter_load_dest_outside_partition_rejected); + tcase_add_test(tc_load, test_elf_scatter_load_phdr_read_failure_rejected); + tcase_set_timeout(tc_load, 10); + suite_add_tcase(s, tc_load); return s; } From a6628a324a23021b8a20831667658ab98c58c6e9 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 13:40:37 +0200 Subject: [PATCH 11/21] F-12108: image: clamp sha block/peek to fw_size get_sha_block() accepted offset == fw_size and always read WOLFBOOT_SHA_BLOCK_SIZE bytes; wolfBoot_peek_image() always reported the full block size. Both could hand callers a window past the end of the image. Reject offset >= fw_size, clamp the external read to the bytes remaining, and report the clamped size (0 when no bytes remain). Hash callers already clamp their input, so this is API hardening; pinned by test_peek_image_bounds (internal + ext cases). --- src/image.c | 29 ++++++++++++++--- tools/unit-tests/unit-image.c | 59 +++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/src/image.c b/src/image.c index c3ea6566bb..fc0a144d9b 100644 --- a/src/image.c +++ b/src/image.c @@ -1028,12 +1028,21 @@ static uint8_t ext_hash_block[WOLFBOOT_SHA_BLOCK_SIZE] XALIGNED(4); static uint8_t *get_sha_block(struct wolfBoot_image *img, uint32_t offset) { uint8_t *p; - if (offset > img->fw_size) +#ifdef EXT_FLASH + uint32_t read_sz; +#endif + + if (offset >= img->fw_size) return NULL; #ifdef EXT_FLASH if (PART_IS_EXT(img)) { - ext_flash_check_read((uintptr_t)(img->fw_base) + offset, ext_hash_block, - WOLFBOOT_SHA_BLOCK_SIZE); + /* Read only the bytes that remain in the image: the block + * window must not extend past fw_size. */ + read_sz = WOLFBOOT_SHA_BLOCK_SIZE; + if (read_sz > img->fw_size - offset) + read_sz = img->fw_size - offset; + ext_flash_check_read((uintptr_t)(img->fw_base) + offset, + ext_hash_block, read_sz); return ext_hash_block; } #endif @@ -2882,8 +2891,18 @@ uint8_t* wolfBoot_peek_image(struct wolfBoot_image *img, uint32_t offset, uint32_t* sz) { uint8_t* p = get_sha_block(img, offset); - if (sz) - *sz = WOLFBOOT_SHA_BLOCK_SIZE; + + if (sz) { + if (p == NULL) { + *sz = 0; + } + else { + *sz = WOLFBOOT_SHA_BLOCK_SIZE; + if (*sz > img->fw_size - offset) { + *sz = img->fw_size - offset; + } + } + } return p; } diff --git a/tools/unit-tests/unit-image.c b/tools/unit-tests/unit-image.c index 1ae990701f..d1f4835ce0 100644 --- a/tools/unit-tests/unit-image.c +++ b/tools/unit-tests/unit-image.c @@ -648,6 +648,9 @@ START_TEST(test_sha_ops) ck_assert_ptr_eq(retp, ext_hash_block); retp = wolfBoot_peek_image(&test_img, offset, &sz); ck_assert_ptr_eq(retp, ext_hash_block); + /* Full block fits in this image (fw_size 0x1000): the reported + * size stays the block size. The clamped tail case is covered by + * test_peek_image_bounds. */ ck_assert_uint_eq(sz, WOLFBOOT_SHA_BLOCK_SIZE); /* Test image hash */ @@ -682,6 +685,61 @@ START_TEST(test_sha_ops) } END_TEST +START_TEST(test_peek_image_bounds) +{ + static uint8_t FlashImg[0x1000]; + struct wolfBoot_image test_img; + uint32_t sz; + uint8_t *retp; + + /* Internal image: a peek at offset == fw_size must be rejected + * (no bytes remain) and the reported size must be clamped to the + * bytes that remain in the image. */ + memset(&test_img, 0, sizeof(test_img)); + test_img.part = PART_BOOT; + test_img.fw_size = 0x100; + test_img.fw_base = FlashImg; + + sz = 0xFFFF; + retp = wolfBoot_peek_image(&test_img, 0x100, &sz); + ck_assert_ptr_null(retp); + ck_assert_uint_eq(sz, 0); + + sz = 0xFFFF; + retp = wolfBoot_peek_image(&test_img, 0xF0, &sz); + ck_assert_ptr_nonnull(retp); + ck_assert_uint_eq(sz, 0x10); + + /* A block that fits fully must still report the full block size. */ + memset(&test_img, 0, sizeof(test_img)); + test_img.part = PART_BOOT; + test_img.fw_size = 0x1000; + test_img.fw_base = FlashImg; + + sz = 0xFFFF; + retp = wolfBoot_peek_image(&test_img, 0, &sz); + ck_assert_ptr_nonnull(retp); + ck_assert_uint_eq(sz, WOLFBOOT_SHA_BLOCK_SIZE); + + /* External image: same contract through the ext flash reader. */ + memset(&test_img, 0, sizeof(test_img)); + test_img.part = PART_UPDATE; + test_img.fw_base = 0; + test_img.fw_size = test_img_len; + ext_flash_write(0, test_img_v200000000_signed_bin, test_img_len); + + sz = 0xFFFF; + retp = wolfBoot_peek_image(&test_img, test_img_len, &sz); + ck_assert_ptr_null(retp); + ck_assert_uint_eq(sz, 0); + + sz = 0xFFFF; + retp = wolfBoot_peek_image(&test_img, test_img_len - 10, &sz); + ck_assert_ptr_nonnull(retp); + ck_assert_uint_eq(sz, 10); +} +END_TEST + START_TEST(test_headers) { struct wolfBoot_image img; @@ -1250,6 +1308,7 @@ Suite *wolfboot_suite(void) TCase* tcase_sha_ops = tcase_create("sha_ops"); tcase_set_timeout(tcase_sha_ops, 20); tcase_add_test(tcase_sha_ops, test_sha_ops); + tcase_add_test(tcase_sha_ops, test_peek_image_bounds); suite_add_tcase(s, tcase_sha_ops); TCase* tcase_headers = tcase_create("headers"); From b5ab88a50aeb85dce1d7748880daad20dbe31131 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 13:56:55 +0200 Subject: [PATCH 12/21] F-9744: store: reject negative length before unsigned arithmetic wolfPKCS11/PSA_Store_Read/Write added int len to unsigned in_buffer_offset before validation: a sufficiently negative len wrapped, hit the truncation branch, and was replaced by the remaining object or capacity bytes, bypassing Write's later len < 0 guard. Reject len < 0 first in all four functions (post-clamp guard kept). --- src/pkcs11_store.c | 6 ++++ src/psa_store.c | 6 ++++ tools/unit-tests/unit-pkcs11_store.c | 49 +++++++++++++++++++++++++++ tools/unit-tests/unit-psa_store.c | 50 ++++++++++++++++++++++++++++ 4 files changed, 111 insertions(+) diff --git a/src/pkcs11_store.c b/src/pkcs11_store.c index a53679aec6..c7135d5d9f 100644 --- a/src/pkcs11_store.c +++ b/src/pkcs11_store.c @@ -489,6 +489,9 @@ int wolfPKCS11_Store_Read(void* store, unsigned char* buffer, int len) if ((handle == NULL) || (handle->hdr == NULL) || (handle->buffer == NULL)) return -1; + if (len < 0) + return -1; + obj_size = handle->hdr->size; if (obj_size > KEYVAULT_OBJ_SIZE) return -1; @@ -522,6 +525,9 @@ int wolfPKCS11_Store_Write(void* store, unsigned char* buffer, int len) if ((handle->flags & STORE_FLAGS_READONLY) != 0) return -1; + if (len < 0) + return -1; + obj_size = handle->hdr->size; if (obj_size > KEYVAULT_OBJ_SIZE) return -1; diff --git a/src/psa_store.c b/src/psa_store.c index 5c3daaad5a..f540deb883 100644 --- a/src/psa_store.c +++ b/src/psa_store.c @@ -499,6 +499,9 @@ int wolfPSA_Store_Read(void* store, unsigned char* buffer, int len) if ((handle == NULL) || (handle->hdr == NULL) || (handle->buffer == NULL)) return -1; + if (len < 0) + return -1; + obj_size = handle->hdr->size; if (obj_size > KEYVAULT_OBJ_SIZE) return -1; @@ -532,6 +535,9 @@ int wolfPSA_Store_Write(void* store, unsigned char* buffer, int len) if ((handle->flags & STORE_FLAGS_READONLY) != 0) return -1; + if (len < 0) + return -1; + obj_size = handle->hdr->size; if (obj_size > KEYVAULT_OBJ_SIZE) return -1; diff --git a/tools/unit-tests/unit-pkcs11_store.c b/tools/unit-tests/unit-pkcs11_store.c index 5291223f65..f556eecc71 100644 --- a/tools/unit-tests/unit-pkcs11_store.c +++ b/tools/unit-tests/unit-pkcs11_store.c @@ -504,6 +504,52 @@ START_TEST(test_shorter_overwrite_erases_residual_key_material) } END_TEST +/* A negative length must be rejected before it enters the unsigned + * offset arithmetic: pre-fix, a sufficiently negative len wrapped to a + * large unsigned value in 'in_buffer_offset + len', entered the + * truncation branch, and was silently replaced by the remaining object + * bytes (Read) or the remaining capacity (Write, bypassing the later + * len < 0 guard). */ +START_TEST(test_store_rejects_negative_len) +{ + CK_ULONG id_tok = 1; + CK_ULONG id_obj = 42; + int type = DYNAMIC_TYPE_ECC; + int ret; + void *store = NULL; + unsigned char rd[16]; + unsigned char wr[16]; + + ret = mmap_file(vault_path, vault_base, keyvault_size, NULL); + ck_assert(ret == 0); + memset(vault_base, 0xEE, keyvault_size); + + /* Create the object with 3 bytes of content */ + ret = wolfPKCS11_Store_Open(type, id_tok, id_obj, 0, &store); + ck_assert_msg(ret == 0, "Failed to open the vault: %d", ret); + ret = wolfPKCS11_Store_Write(store, (unsigned char *)"abc", 3); + ck_assert_int_eq(ret, 3); + wolfPKCS11_Store_Close(store); + + /* Read: pre-fix the negative len was replaced by the 3 remaining + * bytes and copied out; post-fix it is rejected. */ + ret = wolfPKCS11_Store_Open(type, id_tok, id_obj, 1, &store); + ck_assert_msg(ret == 0, "Failed to reopen the vault: %d", ret); + ret = wolfPKCS11_Store_Read(store, rd, -32768); + ck_assert_int_eq(ret, -1); + wolfPKCS11_Store_Close(store); + + /* Write: pre-fix the negative len was clamped to the remaining + * capacity (4088) and read that many bytes from the 16-byte buffer + * below; post-fix it is rejected. */ + ret = wolfPKCS11_Store_Open(type, id_tok, id_obj, 0, &store); + ck_assert_msg(ret == 0, "Failed to reopen the vault for write: %d", ret); + ret = wolfPKCS11_Store_Write(store, wr, -32768); + ck_assert_int_eq(ret, -1); + wolfPKCS11_Store_Close(store); +} +END_TEST + Suite *wolfboot_suite(void) { /* Suite initialization */ @@ -516,6 +562,7 @@ Suite *wolfboot_suite(void) 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* tcase_neg_len = tcase_create("rejects_negative_len"); 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); @@ -523,6 +570,7 @@ Suite *wolfboot_suite(void) 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); + tcase_add_test(tcase_neg_len, test_store_rejects_negative_len); suite_add_tcase(s, tcase_store_and_load_objs); suite_add_tcase(s, tcase_cross_sector_write); suite_add_tcase(s, tcase_close); @@ -530,6 +578,7 @@ Suite *wolfboot_suite(void) suite_add_tcase(s, tcase_delete_corrupted); suite_add_tcase(s, tcase_find_bounds); suite_add_tcase(s, tcase_remanence); + suite_add_tcase(s, tcase_neg_len); return s; } diff --git a/tools/unit-tests/unit-psa_store.c b/tools/unit-tests/unit-psa_store.c index b6bc280023..7c04b37ed0 100644 --- a/tools/unit-tests/unit-psa_store.c +++ b/tools/unit-tests/unit-psa_store.c @@ -290,6 +290,53 @@ START_TEST(test_cache_commit_zeroizes_cached_sector) } END_TEST +/* A negative length must be rejected before it enters the unsigned + * offset arithmetic: pre-fix, a sufficiently negative len wrapped to a + * large unsigned value in 'in_buffer_offset + len', entered the + * truncation branch, and was silently replaced by the remaining object + * bytes (Read) or the remaining capacity (Write, bypassing the later + * len < 0 guard). */ +START_TEST(test_store_rejects_negative_len) +{ + enum { type = WOLFPSA_STORE_KEY }; + const unsigned long id1 = 31; + const unsigned long id2 = 33; + void *store = NULL; + unsigned char rd[16]; + unsigned char wr[16]; + 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, 0xEE, keyvault_size); + + /* Create the object with 3 bytes of content */ + ret = wolfPSA_Store_Open(type, id1, id2, 0, &store); + ck_assert_int_eq(ret, 0); + ret = wolfPSA_Store_Write(store, (unsigned char *)"abc", 3); + ck_assert_int_eq(ret, 3); + wolfPSA_Store_Close(store); + + /* Read: pre-fix the negative len was replaced by the 3 remaining + * bytes and copied out; post-fix it is rejected. */ + ret = wolfPSA_Store_Open(type, id1, id2, 1, &store); + ck_assert_int_eq(ret, 0); + ret = wolfPSA_Store_Read(store, rd, -32768); + ck_assert_int_eq(ret, -1); + wolfPSA_Store_Close(store); + + /* Write: pre-fix the negative len was clamped to the remaining + * capacity (4088) and read that many bytes from the 16-byte buffer + * below; post-fix it is rejected. */ + ret = wolfPSA_Store_Open(type, id1, id2, 0, &store); + ck_assert_int_eq(ret, 0); + ret = wolfPSA_Store_Write(store, wr, -32768); + ck_assert_int_eq(ret, -1); + wolfPSA_Store_Close(store); +} +END_TEST + Suite *wolfboot_suite(void) { Suite *s = suite_create("wolfBoot-psa-store"); @@ -300,6 +347,7 @@ Suite *wolfboot_suite(void) TCase *tcase_find_bounds = tcase_create("find_bounds"); TCase *tcase_tail = tcase_create("shorter_overwrite_clears_tail"); TCase *tcase_zeroize = tcase_create("cache_commit_zeroizes_cached_sector"); + TCase *tcase_neg_len = tcase_create("rejects_negative_len"); tcase_add_test(tcase_write, test_cross_sector_write_preserves_length); tcase_add_test(tcase_close, test_close_clears_handle_state); @@ -308,6 +356,7 @@ Suite *wolfboot_suite(void) tcase_add_test(tcase_find_bounds, test_find_object_search_stops_at_header_sector); tcase_add_test(tcase_tail, test_shorter_overwrite_clears_tail); tcase_add_test(tcase_zeroize, test_cache_commit_zeroizes_cached_sector); + tcase_add_test(tcase_neg_len, test_store_rejects_negative_len); suite_add_tcase(s, tcase_write); suite_add_tcase(s, tcase_close); suite_add_tcase(s, tcase_delete); @@ -315,6 +364,7 @@ Suite *wolfboot_suite(void) suite_add_tcase(s, tcase_find_bounds); suite_add_tcase(s, tcase_tail); suite_add_tcase(s, tcase_zeroize); + suite_add_tcase(s, tcase_neg_len); return s; } From b9068ce162d6d0ca930705441d47c57cd7e41687 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 14:06:33 +0200 Subject: [PATCH 13/21] F-12105: tpm: keep keystore size as int in load_pubkey keystore_get_size() returns -1 on invalid or oversized OTP slot data; storing it in uint16_t made -1 become 65535, which passed the hdrSz <= 0 check and was fed to the ECC/RSA parser as a 65535-byte read from the keystore buffer. Keep it as int and reject values <= 0 or above KEYSTORE_PUBKEY_SIZE. --- src/tpm.c | 4 ++-- tools/unit-tests/unit-tpm-rsa-exp.c | 28 +++++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/tpm.c b/src/tpm.c index 551485d4ac..3c9fc21e5c 100644 --- a/src/tpm.c +++ b/src/tpm.c @@ -393,7 +393,7 @@ int wolfBoot_load_pubkey(const uint8_t* pubkey_hint, WOLFTPM2_KEY* pubKey, uint32_t key_type; int key_slot = -1; uint8_t *hdr; - uint16_t hdrSz; + int hdrSz; *pAlg = TPM_ALG_NULL; @@ -406,7 +406,7 @@ int wolfBoot_load_pubkey(const uint8_t* pubkey_hint, WOLFTPM2_KEY* pubKey, key_type = keystore_get_key_type(key_slot); hdr = keystore_get_buffer(key_slot); hdrSz = keystore_get_size(key_slot); - if (hdr == NULL || hdrSz <= 0) + if (hdr == NULL || hdrSz <= 0 || hdrSz > KEYSTORE_PUBKEY_SIZE) rc = -1; } /* Parse public key to TPM public key. Note: this loads as temp handle, diff --git a/tools/unit-tests/unit-tpm-rsa-exp.c b/tools/unit-tests/unit-tpm-rsa-exp.c index f5909d55e8..1cf0ce5d69 100644 --- a/tools/unit-tests/unit-tpm-rsa-exp.c +++ b/tools/unit-tests/unit-tpm-rsa-exp.c @@ -30,6 +30,8 @@ static uint8_t test_nv_digest[WOLFBOOT_SHA_DIGEST_SIZE]; static uint32_t captured_exponent; static int forbidden_memcmp_calls; static uint32_t mock_nv_digest_sz; +static int mock_keystore_size; +static int decode_calls; int keyslot_id_by_sha(const uint8_t* pubkey_hint) { @@ -52,7 +54,7 @@ uint8_t *keystore_get_buffer(int id) int keystore_get_size(int id) { ck_assert_int_eq(id, 0); - return (int)sizeof(test_hdr); + return mock_keystore_size; } int wc_RsaPublicKeyDecode_ex(const byte* input, word32* inOutIdx, word32 inSz, @@ -61,6 +63,7 @@ int wc_RsaPublicKeyDecode_ex(const byte* input, word32* inOutIdx, word32 inSz, (void)input; (void)inSz; + decode_calls++; *inOutIdx = 0; *n = test_modulus; *nSz = sizeof(test_modulus); @@ -175,6 +178,8 @@ static void setup(void) captured_exponent = 0; forbidden_memcmp_calls = 0; mock_nv_digest_sz = WOLFBOOT_SHA_DIGEST_SIZE; + mock_keystore_size = (int)sizeof(test_hdr); + decode_calls = 0; } START_TEST(test_wolfBoot_load_pubkey_decodes_der_exponent_bytes) @@ -194,6 +199,26 @@ START_TEST(test_wolfBoot_load_pubkey_decodes_der_exponent_bytes) } END_TEST +/* A failed keystore_get_size() (-1: invalid or oversized OTP slot) must be + * rejected, not narrowed to uint16_t (65535) and fed to the key parser. */ +START_TEST(test_wolfBoot_load_pubkey_rejects_failed_keystore_size) +{ + uint8_t hint[WOLFBOOT_SHA_DIGEST_SIZE] = { 0 }; + WOLFTPM2_KEY key; + TPM_ALG_ID alg = TPM_ALG_NULL; + int rc; + + memset(&key, 0, sizeof(key)); + mock_keystore_size = -1; + + rc = wolfBoot_load_pubkey(hint, &key, &alg); + + ck_assert_int_eq(rc, -1); + ck_assert_uint_eq(decode_calls, 0); + ck_assert_int_eq(alg, TPM_ALG_NULL); +} +END_TEST + START_TEST(test_wolfBoot_check_rot_avoids_memcmp_on_digest_compare) { uint8_t hint[WOLFBOOT_SHA_DIGEST_SIZE]; @@ -245,6 +270,7 @@ static Suite *tpm_suite(void) tc = tcase_create("wolfBoot_load_pubkey"); tcase_add_checked_fixture(tc, setup, NULL); tcase_add_test(tc, test_wolfBoot_load_pubkey_decodes_der_exponent_bytes); + tcase_add_test(tc, test_wolfBoot_load_pubkey_rejects_failed_keystore_size); tcase_add_test(tc, test_wolfBoot_check_rot_avoids_memcmp_on_digest_compare); tcase_add_test(tc, test_wolfBoot_check_rot_rejects_mismatched_digest); tcase_add_test(tc, test_wolfBoot_check_rot_rejects_wrong_digest_size); From d64f15af057bb1558bee06ce0f6c7447b3e99921 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 14:33:29 +0200 Subject: [PATCH 14/21] F-7392: stm32h7: document inherent OTP immutability in set_readonly The H7 OTP memory is one-time programmable, so the keystore and UDS are permanent once written; the H7 has no OTP block-lock register (unlike the H5), so there is no write-protection step to perform. Replace the misleading TODO with the reason the no-op is correct. --- hal/stm32h7.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hal/stm32h7.c b/hal/stm32h7.c index eb47d696b9..4383e4daa7 100644 --- a/hal/stm32h7.c +++ b/hal/stm32h7.c @@ -560,7 +560,11 @@ static void hal_flash_otp_lock(void) int hal_flash_otp_set_readonly(uint32_t flashAddress, uint16_t length) { - /* TODO: set WP on OTP if needed */ + /* The STM32H7 OTP memory is one-time programmable: once the keystore + * and UDS are written, the data is permanent and cannot be overwritten. + * Unlike the STM32H5, the H7 has no OTP block-lock register, so there + * is no explicit write-protection step to perform. The anchor is + * protected by the inherent immutability of the programmed OTP. */ return 0; } From 62918f2e90caf059594f6a62a020f1789e9fcb96 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 14:53:15 +0200 Subject: [PATCH 15/21] F-12107: pkcs11 nsc: zero work buffer before it can be freed If the snap allocation fails but the work allocation succeeds, the initialisation loop never runs and nsc_tmpl_free() would release indeterminate work[].pValue pointers. Zero work[] right after the allocation (before the NULL check) so every path that reaches the free hands out initialised, NULL pValue entries. --- src/pkcs11_callable.c | 6 +++ tools/unit-tests/unit-pkcs11-nsc-zeroize.c | 59 +++++++++++++++++++++- 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/pkcs11_callable.c b/src/pkcs11_callable.c index 323c929a5b..474a0c1a4e 100644 --- a/src/pkcs11_callable.c +++ b/src/pkcs11_callable.c @@ -602,6 +602,12 @@ static CK_RV nsc_tmpl_prepare(CK_ATTRIBUTE_PTR ns, CK_ULONG count, int isOut, NULL, DYNAMIC_TYPE_TMP_BUFFER); t->work = (CK_ATTRIBUTE *)XMALLOC((size_t)(count * sizeof(CK_ATTRIBUTE)), NULL, DYNAMIC_TYPE_TMP_BUFFER); + if (t->work != NULL) { + /* Zero before any path can reach nsc_tmpl_free(): if the snap + * allocation failed, the initialisation loop below never runs, and + * free() would release indeterminate work[].pValue pointers. */ + XMEMSET(t->work, 0, (size_t)(count * sizeof(CK_ATTRIBUTE))); + } if (t->snap == NULL || t->work == NULL) { rv = CKR_HOST_MEMORY; goto fail; diff --git a/tools/unit-tests/unit-pkcs11-nsc-zeroize.c b/tools/unit-tests/unit-pkcs11-nsc-zeroize.c index 04f95b5413..df1f00f2de 100644 --- a/tools/unit-tests/unit-pkcs11-nsc-zeroize.c +++ b/tools/unit-tests/unit-pkcs11-nsc-zeroize.c @@ -42,11 +42,18 @@ */ static uint8_t sec_pool[4096]; static size_t sec_pool_used; +static int malloc_fail_next; +static void *freed_ptrs[64]; +static int freed_count; static void *sec_malloc(size_t n) { void *p; + if (malloc_fail_next) { + malloc_fail_next = 0; + return NULL; + } if (n == 0) n = 1; n = (n + 7U) & ~(size_t)7U; /* keep allocations aligned */ @@ -59,7 +66,7 @@ static void *sec_malloc(size_t n) #define XMALLOC_OVERRIDE #define XMALLOC(n, h, t) sec_malloc((size_t)(n)) -#define XFREE(p, h, t) do { (void)(p); } while (0) +#define XFREE(p, h, t) do { if ((p) != NULL && freed_count < 64) freed_ptrs[freed_count++] = (void *)(p); } while (0) #define XREALLOC(p, n, h, t) NULL #include "user_settings.h" @@ -160,6 +167,9 @@ static void reset_state(void) { memset(sec_pool, 0, sizeof(sec_pool)); sec_pool_used = 0; + malloc_fail_next = 0; + memset(freed_ptrs, 0, sizeof(freed_ptrs)); + freed_count = 0; stub_saw_secret = 0; memset(&ns_mem, 0, sizeof(ns_mem)); } @@ -246,6 +256,52 @@ START_TEST(test_mech_password_zeroized) } END_TEST +/* + * Partial allocation failure: the pool is pre-filled with 0xDE so any block + * the allocator hands out holds non-NULL garbage. If the snapshot + * allocation fails while the work allocation succeeds, the cleanup must not + * pass the indeterminate work[].pValue pointers to XFREE: every released + * pointer must be one this allocator actually handed out. + */ +START_TEST(test_tmpl_partial_alloc_no_garbage_free) +{ + CK_ATTRIBUTE *tmpl = ns_mem.tmpl; + uint8_t *nsKey = ns_mem.bytes + 3 * sizeof(CK_ATTRIBUTE); + CK_OBJECT_HANDLE *nsHandle; + CK_OBJECT_CLASS *nsClass; + CK_KEY_TYPE *nsType; + CK_RV rv; + int i; + + reset_state(); + memset(sec_pool, 0xDE, sizeof(sec_pool)); + malloc_fail_next = 1; /* fail the snap alloc */ + memcpy(nsKey, secret_key, sizeof(secret_key)); + nsClass = (CK_OBJECT_CLASS *)(nsKey + sizeof(secret_key)); + nsType = (CK_KEY_TYPE *)(nsClass + 1); + nsHandle = (CK_OBJECT_HANDLE *)(nsType + 1); + *nsClass = CKO_SECRET_KEY; + *nsType = CKK_AES; + tmpl[0].type = CKA_CLASS; + tmpl[0].pValue = nsClass; + tmpl[0].ulValueLen = sizeof(*nsClass); + tmpl[1].type = CKA_KEY_TYPE; + tmpl[1].pValue = nsType; + tmpl[1].ulValueLen = sizeof(*nsType); + tmpl[2].type = CKA_VALUE; + tmpl[2].pValue = nsKey; + tmpl[2].ulValueLen = sizeof(secret_key); + + rv = C_CreateObject_nsc_call(1, tmpl, 3, nsHandle); + ck_assert_int_eq((int)rv, (int)CKR_HOST_MEMORY); + for (i = 0; i < freed_count; i++) { + uint8_t *p = freed_ptrs[i]; + ck_assert_msg(p >= sec_pool && p < sec_pool + sizeof(sec_pool), + "XFREE got indeterminate pointer %p", (void *)p); + } +} +END_TEST + Suite *pkcs11_nsc_suite(void) { Suite *s = suite_create("pkcs11-nsc-zeroize"); @@ -253,6 +309,7 @@ Suite *pkcs11_nsc_suite(void) tcase_add_test(tc, test_create_object_value_zeroized); tcase_add_test(tc, test_mech_password_zeroized); + tcase_add_test(tc, test_tmpl_partial_alloc_no_garbage_free); suite_add_tcase(s, tc); return s; } From 58e79daec7291d546e8f3822f0b401f60d3ab1d8 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 15:52:19 +0200 Subject: [PATCH 16/21] F-11047: image: drop incorrect boot-partition destination check The ELF scatter destination is the exec region, which sits outside the boot partition that stores the signed ELF. Bounding it to the boot partition rejected every legitimate segment (aurix exec is below boot, sim scatter is above it) and bricked corruption recovery. The paddr is covered by the image signature verified before this restore path and the overflow check keeps it from wrapping, so no destination bound is needed; this also matches the check function, which bounds no destination. --- src/image.c | 17 ++++---- tools/unit-tests/unit-image-elf-scatter.c | 47 ++++------------------- 2 files changed, 15 insertions(+), 49 deletions(-) diff --git a/src/image.c b/src/image.c index fc0a144d9b..e42702b0cf 100644 --- a/src/image.c +++ b/src/image.c @@ -2523,9 +2523,13 @@ int wolfBoot_load_flash_image_elf(int part, unsigned long* entry_out, int ext_fl } /* Validate the segment before writing: the source must stay - * inside the manifest image, the paddr range must fit the - * destination (uintptr_t) width, and the destination must stay - * inside the boot partition. Reject instead of writing. */ + * inside the manifest image and the paddr range must fit the + * destination (uintptr_t) width so the load_addr cast below + * cannot wrap. The scatter destination is the exec region, which + * sits outside the boot partition that stores the signed ELF, so + * it is not bounded here: the program-header paddr values are + * covered by the image signature verified before this restore + * path. Reject instead of writing. */ if (filesz > UINT32_MAX) { wolfBoot_printf("ELF: [STORE] ERROR: segment file_size " "%lu does not fit a 32-bit length\n", @@ -2548,13 +2552,6 @@ int wolfBoot_load_flash_image_elf(int part, unsigned long* entry_out, int ext_fl return -1; } load_addr = (uintptr_t)seg_start; - if (load_addr < (uintptr_t)boot.hdr || - load_addr + filesz > - (uintptr_t)boot.hdr + (uintptr_t)WOLFBOOT_PARTITION_SIZE) { - wolfBoot_printf("ELF: [STORE] ERROR: segment destination " - "outside boot partition\n"); - return -1; - } wolfBoot_printf("ELF: [STORE] Writing loadable segment: " "loadaddr=0x%08lx, offset=0x%08lx, size=%lu\n", diff --git a/tools/unit-tests/unit-image-elf-scatter.c b/tools/unit-tests/unit-image-elf-scatter.c index c53760d1f4..47cc5df44b 100644 --- a/tools/unit-tests/unit-image-elf-scatter.c +++ b/tools/unit-tests/unit-image-elf-scatter.c @@ -572,13 +572,14 @@ END_TEST * * The load function walks the same program header table and copies each * PT_LOAD segment from the manifest (fw_base + offset) to its scattered - * destination (paddr + BASE_OFF). In this harness the boot partition is - * mmap'd at MOCK_ADDRESS_BOOT (== WOLFBOOT_PARTITION_BOOT_ADDRESS) and - * BASE_OFF is 0, so a valid destination is any address inside - * [MOCK_ADDRESS_BOOT, MOCK_ADDRESS_BOOT + WOLFBOOT_PARTITION_SIZE). - * The mock flash layer fails the test on any erase/write outside the - * known partition ranges, so an unvalidated paddr cannot walk off into - * unmapped memory here. */ + * destination (paddr + BASE_OFF). In reality that destination is the exec + * region, which sits outside the boot partition; the harness models it as + * an address inside the mmap'd boot partition (MOCK_ADDRESS_BOOT, BASE_OFF + * 0) so the mock flash layer can service the write. The load function does + * not bound the destination (the paddr is signature-protected); it rejects + * a source that runs past fw_size, a paddr range that overflows the address + * width, and a program header that cannot be read. The mock flash layer + * fails the test on any erase/write outside the known partition ranges. */ #define LOAD_DEST (MOCK_ADDRESS_BOOT + 0x4000) @@ -678,36 +679,6 @@ START_TEST(test_elf_scatter_load_paddr_range_overflow_rejected) } END_TEST -/* A destination outside the boot partition must be rejected before any - * flash access. Pre-fix the mock's out-of-range erase check caught it - * with fail("Invalid address"); on real hardware this is an - * erase/write at an unintended location. */ -START_TEST(test_elf_scatter_load_dest_outside_partition_rejected) -{ - unsigned long entry = 0; - struct seg_spec segs[1]; - int ret; - - map_boot_partition(); - - memset(seg2_flash, 0, sizeof(seg2_flash)); - segs[0].offset = ELF_HDR_SZ; - segs[0].filesz = SEG_SIZE; - segs[0].paddr = (uint64_t)(MOCK_ADDRESS_BOOT + WOLFBOOT_PARTITION_SIZE + - 0x1000); - segs[0].payload = seg2_flash; - segs[0].fillsz = SEG_SIZE; - - build_scattered_image_n(segs, 1, IMG_FW_SIZE); - - ret = wolfBoot_load_flash_image_elf(PART_BOOT, &entry, 0); - - ck_assert_int_eq(ret, -1); - - unmap_boot_partition(); -} -END_TEST - /* A program header that lies past the end of the manifest image cannot * be read; the load must abort instead of consuming the uninitialized * header locals. The PHT sits at fw offset 64 (right after the 64-byte @@ -762,8 +733,6 @@ Suite *elf_scatter_suite(void) test_elf_scatter_load_segment_beyond_fw_size_rejected); tcase_add_test(tc_load, test_elf_scatter_load_paddr_range_overflow_rejected); - tcase_add_test(tc_load, - test_elf_scatter_load_dest_outside_partition_rejected); tcase_add_test(tc_load, test_elf_scatter_load_phdr_read_failure_rejected); tcase_set_timeout(tc_load, 10); suite_add_tcase(s, tc_load); From 01a71c7d39dee88911c51e2d26c0af6efc37271c Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 15:52:27 +0200 Subject: [PATCH 17/21] x86_64_efi: return after LoadImage-failure panic under UNIT_TEST panic() returns under UNIT_TEST, so the LoadImage-failure site fell through to StartImage on a failed load. Add the return to match the zero-size guard; on target panic() never returns, so behavior is unchanged. --- hal/x86_64_efi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hal/x86_64_efi.c b/hal/x86_64_efi.c index c99dcbd693..e7f1fddfe6 100644 --- a/hal/x86_64_efi.c +++ b/hal/x86_64_efi.c @@ -158,6 +158,7 @@ void RAMFUNCTION x86_64_efi_do_boot(const uint32_t *boot_addr) if (status != EFI_SUCCESS) { wolfBoot_printf("can't load kernel image from memory\n"); panic(); + return; /* Never reached on target, where panic() does not return */ } /* Hand the authenticated command line to the loaded image via LoadOptions From b0209d0bf677b567621abe0c297c9b0a9266615d Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 16:39:26 +0200 Subject: [PATCH 18/21] image: use uint64_t for ELF scatter program-header locals paddr/filesz/offset were unsigned long, which is 32-bit on the ELF-scatter targets (aurix-tc375, sim32). ELF64 program headers truncate before the segment guards run, so filesz > UINT32_MAX is never true and an out-of-range paddr wraps in-range. Use uint64_t to match wolfBoot_check_flash_image_elf so the guards see untruncated values. --- src/image.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/image.c b/src/image.c index e42702b0cf..64b8a628a6 100644 --- a/src/image.c +++ b/src/image.c @@ -2482,10 +2482,10 @@ int wolfBoot_load_flash_image_elf(int part, unsigned long* entry_out, int ext_fl /* Walk the program header table and store each loadable segment */ for (i = 0; i < entry_count; ++i) { - unsigned long paddr, filesz, offset; - int is_loadable; - uintptr_t load_addr; - uint64_t seg_start; + uint64_t paddr, filesz, offset; + int is_loadable; + uintptr_t load_addr; + uint64_t seg_start; /* Read the current program header into a local buffer */ if (is_elf32) { @@ -2497,9 +2497,9 @@ int wolfBoot_load_flash_image_elf(int part, unsigned long* entry_out, int ext_fl return -1; } is_loadable = (p32.type == ELF_PT_LOAD); - paddr = (unsigned long)p32.paddr; - offset = (unsigned long)p32.offset; - filesz = (unsigned long)p32.file_size; + paddr = p32.paddr; + offset = p32.offset; + filesz = p32.file_size; ph_size = sizeof(p32); } else { @@ -2511,9 +2511,9 @@ int wolfBoot_load_flash_image_elf(int part, unsigned long* entry_out, int ext_fl return -1; } is_loadable = (p64.type == ELF_PT_LOAD); - paddr = (unsigned long)p64.paddr; - offset = (unsigned long)p64.offset; - filesz = (unsigned long)p64.file_size; + paddr = p64.paddr; + offset = p64.offset; + filesz = p64.file_size; ph_size = sizeof(p64); } /* Skip non-loadable segments */ From faaad372cc67be19ff00da836a1883302d154295 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 16:39:26 +0200 Subject: [PATCH 19/21] Address minor PR 882 review nits Cap the library fw_size clamp at UINT32_MAX (a > 4 GiB file would otherwise truncate to a small value), say 'restore' not 'store' in the scatter-restore error message, and print stdout as well as stderr in the compile-check scripts on failure. --- hal/library.c | 14 +++++++++++--- src/update_flash.c | 2 +- tools/unit-tests/unit-elf-scatter-db-build.py | 1 + .../unit-tests/unit-image-wolfhsm-client-build.py | 1 + tools/unit-tests/unit-x86-fsp-stage1auth-build.py | 1 + 5 files changed, 15 insertions(+), 4 deletions(-) diff --git a/hal/library.c b/hal/library.c index 9a60336cb5..76a465db42 100644 --- a/hal/library.c +++ b/hal/library.c @@ -131,6 +131,7 @@ static const uint8_t test_img[] = { int wolfBoot_start(void) { struct wolfBoot_image os_image; + size_t max_payload; int ret = 0; memset(&os_image, 0, sizeof(os_image)); @@ -141,9 +142,16 @@ int wolfBoot_start(void) } /* The loaded file may be shorter than the firmware size the header - * claims; bound the hash range to the bytes actually loaded. */ - if (os_image.fw_size > (uint32_t)(gImageSize - IMAGE_HEADER_SIZE)) { - os_image.fw_size = (uint32_t)(gImageSize - IMAGE_HEADER_SIZE); + * claims; bound the hash range to the bytes actually loaded. Compute + * the payload in size_t and cap at UINT32_MAX so a > 4 GiB file + * clamps to the maximum firmware size rather than truncating to a + * small value. */ + max_payload = gImageSize - IMAGE_HEADER_SIZE; + if (max_payload > UINT32_MAX) { + max_payload = UINT32_MAX; + } + if (os_image.fw_size > (uint32_t)max_payload) { + os_image.fw_size = (uint32_t)max_payload; } if ((ret = wolfBoot_verify_integrity(&os_image)) < 0) { diff --git a/src/update_flash.c b/src/update_flash.c index c129eff574..58a23f9e86 100644 --- a/src/update_flash.c +++ b/src/update_flash.c @@ -1339,7 +1339,7 @@ static int RAMFUNCTION wolfBoot_update(int fallback_allowed) if (wolfBoot_load_flash_image_elf(PART_BOOT, &entry, PART_IS_EXT(&boot)) < 0) { wolfBoot_printf( - "ELF: [UPDATE] ERROR: could not store scattered image\n"); + "ELF: [UPDATE] ERROR: could not restore scattered image\n"); wolfBoot_panic(); } if (wolfBoot_check_flash_image_elf(PART_BOOT, &entry) < 0) { diff --git a/tools/unit-tests/unit-elf-scatter-db-build.py b/tools/unit-tests/unit-elf-scatter-db-build.py index 576b79b1fb..03a118864e 100644 --- a/tools/unit-tests/unit-elf-scatter-db-build.py +++ b/tools/unit-tests/unit-elf-scatter-db-build.py @@ -36,6 +36,7 @@ def main(): if p.returncode != 0: print("FAIL: DISABLE_BACKUP + ELF scatter + EXT_FLASH " "does not compile:\n") + print(p.stdout[-2000:]) print(p.stderr[-2000:]) return 1 print("PASS: DISABLE_BACKUP + ELF scatter + EXT_FLASH compiles") diff --git a/tools/unit-tests/unit-image-wolfhsm-client-build.py b/tools/unit-tests/unit-image-wolfhsm-client-build.py index 626ec43035..3075336b5a 100755 --- a/tools/unit-tests/unit-image-wolfhsm-client-build.py +++ b/tools/unit-tests/unit-image-wolfhsm-client-build.py @@ -36,6 +36,7 @@ def main(): if p.returncode != 0: print("FAIL: WOLFBOOT_ENABLE_WOLFHSM_CLIENT image.c " "does not compile:\n") + print(p.stdout[-2000:]) print(p.stderr[-2000:]) return 1 print("PASS: WOLFBOOT_ENABLE_WOLFHSM_CLIENT image.c compiles") diff --git a/tools/unit-tests/unit-x86-fsp-stage1auth-build.py b/tools/unit-tests/unit-x86-fsp-stage1auth-build.py index 2b261b7f52..13263263ea 100644 --- a/tools/unit-tests/unit-x86-fsp-stage1auth-build.py +++ b/tools/unit-tests/unit-x86-fsp-stage1auth-build.py @@ -36,6 +36,7 @@ def main(): if p.returncode != 0: print("FAIL: STAGE1_AUTH variant of boot_x86_fsp.c " "does not compile:\n") + print(p.stdout[-2000:]) print(p.stderr[-2000:]) return 1 print("PASS: STAGE1_AUTH variant of boot_x86_fsp.c compiles") From 2dbb8aea0103ee3abc4d7d259de6ee2620db3372 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 20:02:10 +0200 Subject: [PATCH 20/21] image: cast ELF scatter log fields to unsigned long offset/filesz are uint64_t now; cast them for the %08lx/%lu conversions so the varargs read matches on 32-bit targets, matching the check function and the rejection messages above. --- src/image.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/image.c b/src/image.c index 64b8a628a6..24c806d47b 100644 --- a/src/image.c +++ b/src/image.c @@ -2555,7 +2555,8 @@ int wolfBoot_load_flash_image_elf(int part, unsigned long* entry_out, int ext_fl wolfBoot_printf("ELF: [STORE] Writing loadable segment: " "loadaddr=0x%08lx, offset=0x%08lx, size=%lu\n", - (unsigned long)load_addr, offset, filesz); + (unsigned long)load_addr, (unsigned long)offset, + (unsigned long)filesz); if (copy_flash_buffered((uintptr_t)(image + offset), load_addr, filesz, ext_flash, ext_flash) != 0) { wolfBoot_printf("ELF: [STORE] ERROR: could not write " From c2916395a9bb2207fc588e6c89f8cf3b434edae3 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 21:08:36 +0200 Subject: [PATCH 21/21] x86_64_efi: early-return after remaining panic() sites under UNIT_TEST do_boot() already returned after its panic() calls, but GetVolume() and efi_main() did not: under UNIT_TEST panic() returns, so GetVolume() hit a NULL deref / uninitialized return and efi_main() ran wolfBoot_start(). Return NULL / EFI_LOAD_ERROR at each site (unreachable on target). --- hal/x86_64_efi.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/hal/x86_64_efi.c b/hal/x86_64_efi.c index e7f1fddfe6..4937ebd083 100644 --- a/hal/x86_64_efi.c +++ b/hal/x86_64_efi.c @@ -210,19 +210,25 @@ static EFI_FILE_HANDLE GetVolume(EFI_HANDLE image) status = uefi_call_wrapper(BS->HandleProtocol, 3, image, &lipGuid, (void **) &loaded_image); - if (status != EFI_SUCCESS) + if (status != EFI_SUCCESS) { panic(); + return NULL; /* Never reached on target (panic() does not return) */ + } status = uefi_call_wrapper(BS->HandleProtocol, 3, loaded_image->DeviceHandle, &fsGuid, (VOID*)&IOVolume); - if (status != EFI_SUCCESS) + if (status != EFI_SUCCESS) { panic(); + return NULL; /* Never reached on target (panic() does not return) */ + } status = uefi_call_wrapper(IOVolume->OpenVolume, 2, IOVolume, &Volume); - if (status != EFI_SUCCESS) + if (status != EFI_SUCCESS) { panic(); + return NULL; /* Never reached on target (panic() does not return) */ + } return Volume; } @@ -353,6 +359,8 @@ efi_main (EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) if (kernel_addr == 0 && update_addr == 0) { wolfBoot_printf("No image to load\n"); panic(); + return EFI_LOAD_ERROR; /* Never reached on target (panic() does not + * return) */ } wolfBoot_start();