From f5401b0e237fe8b372e8c75804ef7cec4bf04e67 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 22:35:46 +0200 Subject: [PATCH 01/27] F-7380: make SAMA5D3 ext_flash_read byte-accurate for partial pages ext_flash_read() ignored the intra-page offset of the start address (a partial read returned bytes from the head of the page), copied sub-page reads in 32-bit words (dropping a sub-word tail), and wrote a full NAND page into the caller's buffer when a multi-page read ended mid-page (overrunning it). The integrity check hashes the image in 64-byte blocks from fw_base + offset, so any read crossing a page mid-block returned the wrong bytes. Walk the read page by page from the exact address: full aligned pages go straight into the caller's buffer; partial first/last pages are staged through the page buffer and copied from the column offset. Proven by unit-sama5d3-ext-read, which extracts the real function and runs it against an emulated device: unaligned starts, 1-3 byte lengths, the 64-byte integrity-hash block pattern, page/block boundaries, and multi-page reads with a short tail. Pre-fix 6 of the 8 checks failed. --- .gitignore | 2 + hal/sama5d3.c | 78 ++++---- tools/unit-tests/Makefile | 15 ++ tools/unit-tests/unit-sama5d3-ext-read.c | 244 +++++++++++++++++++++++ 4 files changed, 296 insertions(+), 43 deletions(-) create mode 100644 tools/unit-tests/unit-sama5d3-ext-read.c diff --git a/.gitignore b/.gitignore index c7e87b7ae1..1ad89d29c8 100644 --- a/.gitignore +++ b/.gitignore @@ -478,12 +478,14 @@ tools/unit-tests/unit-versal-qspi-dma tools/unit-tests/unit-xspi-tfd-index tools/unit-tests/unit-zynq-erase-loop tools/unit-tests/unit-zynq-ext-write +tools/unit-tests/unit-sama5d3-ext-read # sources generated by the unit-test extraction rules tools/unit-tests/aurix_erased_extract.h tools/unit-tests/nvm_cache_scrub_extract.h tools/unit-tests/nxp_ls1028a_host.c tools/unit-tests/nxp_p1021_host.c tools/unit-tests/nxp_t10xx_fixup_extract.h +tools/unit-tests/sama5d3_read_extract.h tools/unit-tests/sdhci_host.c tools/unit-tests/stm32l5_write_extract.h tools/unit-tests/stm32u5_write_extract.h diff --git a/hal/sama5d3.c b/hal/sama5d3.c index e4134c1a47..613173c1d0 100644 --- a/hal/sama5d3.c +++ b/hal/sama5d3.c @@ -601,28 +601,29 @@ static int nand_check_bad_block(uint32_t block) int ext_flash_read(uintptr_t address, uint8_t *data, int len) { uint8_t buffer_page[NAND_FLASH_PAGE_SIZE]; - uint32_t block = div_u(address, nand_flash.block_size); /* The block where the address falls in */ - uint32_t page = div_u(address, nand_flash.page_size); /* The page where the address falls in */ - uint32_t start_page_in_block = mod(page, nand_flash.pages_per_block); /* The start page within this block */ - uint32_t in_block_offset = mod(address, nand_flash.block_size); /* The offset of the address within the block */ - uint32_t remaining = nand_flash.block_size - in_block_offset; /* How many bytes remaining to read in the first block */ - int len_to_read = len; - uint8_t *buffer = data; - uint32_t i; - int copy = 0; + uintptr_t addr = address; + uint8_t *dst = data; + uint32_t in_page = mod(address, nand_flash.page_size); /* The offset of the address within the page */ + int remaining = len; int ret; - if (len < (int)nand_flash.page_size) { - buffer = buffer_page; - copy = 1; - len_to_read = nand_flash.page_size; - } + if (len <= 0) + return 0; + + while (remaining > 0) { + uint32_t block; + uint32_t page; + uint32_t page_in_block; + uint32_t chunk; + + /* Bytes available from the current address to the end of its page */ + chunk = nand_flash.page_size - in_page; + if (chunk > (uint32_t)remaining) + chunk = (uint32_t)remaining; - while (len_to_read > 0) { - uint32_t sz = len_to_read; - uint32_t pages_to_read; - if (sz > remaining) - sz = remaining; + block = div_u(addr, nand_flash.block_size); + page = div_u(addr, nand_flash.page_size); + page_in_block = mod(page, nand_flash.pages_per_block); do { ret = nand_check_bad_block(block); @@ -632,32 +633,23 @@ int ext_flash_read(uintptr_t address, uint8_t *data, int len) } } while (ret < 0); - /* Amount of pages to be read from this block */ - pages_to_read = div_u((sz + nand_flash.page_size - 1), nand_flash.page_size); - - if (pages_to_read * nand_flash.page_size > remaining) - pages_to_read--; - - /* Read (remaining) pages off a block */ - for (i = 0; i < pages_to_read; i++) { - nand_read_page(block, start_page_in_block + i, buffer); - if (sz > nand_flash.page_size) - sz = nand_flash.page_size; - len_to_read -= sz; - buffer += sz; + if ((in_page == 0) && (chunk == nand_flash.page_size)) { + /* Full page at the page head: read straight into the caller's + * buffer. */ + nand_read_page(block, page_in_block, dst); } - /* The block is over, move to the next one */ - block++; - start_page_in_block = 0; - remaining = nand_flash.block_size; - } - if (copy) { - uint32_t *dst = (uint32_t *)data; - uint32_t *src = (uint32_t *)buffer_page; - uint32_t tot_len = (uint32_t)len; - for (i = 0; i < (tot_len >> 2); i++) { - dst[i] = src[i]; + else { + /* Partial first/last page: stage through the page buffer and + * copy from the column offset, so a full page is never written + * past the end of the caller's buffer. */ + nand_read_page(block, page_in_block, buffer_page); + memcpy(dst, buffer_page + in_page, chunk); } + + dst += chunk; + remaining -= (int)chunk; + addr += chunk; + in_page = mod(addr, nand_flash.page_size); } return len; } diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index e94410262c..28f5284039 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -150,6 +150,7 @@ ENABLE_32BIT_TESTS ?= $(HAVE_M32) ifeq ($(ENABLE_32BIT_TESTS),1) TESTS+=unit-linux-loader-e820 TESTS+=unit-linux-loader-syssize +TESTS+=unit-sama5d3-ext-read else $(info Skipping 32-bit x86 linux-loader unit tests: 'gcc -m32' unavailable (set ENABLE_32BIT_TESTS=1 to force)) endif @@ -837,6 +838,20 @@ zynq_write_extract.h: ../../hal/zynq.c unit-zynq-ext-write: unit-zynq-ext-write.c zynq_write_extract.h gcc -o $@ unit-zynq-ext-write.c $(CFLAGS) $(LDFLAGS) +# unit-sama5d3-ext-read runs the real ext_flash_read() from hal/sama5d3.c +# (the intra-page offset of the start address was never applied, sub-page +# reads were copied in 32-bit words, and a multi-page read ending +# mid-page wrote a full page past the caller's buffer). The function and +# the nand_flash geometry struct are extracted verbatim; the test provides +# host div_u()/mod() and emulated nand_read_page()/nand_check_bad_block() +# backed by a byte array. +sama5d3_read_extract.h: ../../hal/sama5d3.c + sed -n '/^struct nand_flash {/,/^} nand_flash = { 0 };/p' $< > $@ + sed -n '/^int ext_flash_read(/,/^}/p' $< >> $@ + +unit-sama5d3-ext-read: unit-sama5d3-ext-read.c sama5d3_read_extract.h + gcc -o $@ unit-sama5d3-ext-read.c $(CFLAGS) $(LDFLAGS) + # unit-versal-qspi-dma drives the real DMA RX path of qspi_transfer() in # hal/versal.c (an unaligned read larger than the 4096-byte # temp buffer copied the full requested length out of the buffer). The diff --git a/tools/unit-tests/unit-sama5d3-ext-read.c b/tools/unit-tests/unit-sama5d3-ext-read.c new file mode 100644 index 0000000000..b571c5fb44 --- /dev/null +++ b/tools/unit-tests/unit-sama5d3-ext-read.c @@ -0,0 +1,244 @@ +/* unit-sama5d3-ext-read.c + * + * Regression test: ext_flash_read() in hal/sama5d3.c never applied the + * intra-page offset of the start address (a partial read returned bytes + * from the head of the page instead of the requested column), copied + * sub-page reads in 32-bit words (dropping a sub-word tail), and wrote + * a full NAND page into the caller's buffer for a multi-page read + * ending mid-page (overrunning the buffer). + * + * The HAL cannot be built on the host, so the Makefile extracts + * ext_flash_read() verbatim along with the nand_flash geometry struct; + * the test provides host div_u()/mod() (the HAL's software-division + * wrappers exist only because the Cortex-A5 has no divider) and + * emulated nand_read_page()/nand_check_bad_block() backed by a + * deterministic byte array. + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfBoot. + * + * wolfBoot is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfBoot is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA + */ + +#include +#include +#include +#include + +/* sama5d3.h constants the extracted function needs. */ +#define NAND_FLASH_PAGE_SIZE 0x800 /* 2KB */ +#define NAND_FLASH_OOB_SIZE 0x40 /* 64B */ +#define MAX_ECC_BYTES 8 +#define wolfBoot_printf(...) do {} while (0) + +/* Emulated NAND: 8 blocks x 16 pages x 2KB = 256KB. */ +#define EMU_BLOCKS 8 +#define EMU_PAGES_PER_BLK 16 +#define EMU_PAGE_SIZE NAND_FLASH_PAGE_SIZE +#define EMU_BLOCK_SIZE (EMU_PAGES_PER_BLK * EMU_PAGE_SIZE) +#define EMU_TOTAL (EMU_BLOCKS * EMU_BLOCK_SIZE) +static uint8_t emu_nand[EMU_TOTAL]; + +/* Host equivalents of the HAL's software-division wrappers. */ +static uint32_t div_u(uint32_t dividend, uint32_t divisor) +{ + return dividend / divisor; +} + +static uint32_t mod(uint32_t dividend, uint32_t divisor) +{ + return dividend % divisor; +} + +/* Emulated NAND primitives: a full page read from column 0, like the + * hardware path; every block is good. */ +static int nand_check_bad_block(uint32_t block) +{ + (void)block; + return 0; +} + +static int nand_read_page(uint32_t block, uint32_t page, uint8_t *data) +{ + uint32_t row = block * EMU_PAGES_PER_BLK + page; + + memcpy(data, emu_nand + row * EMU_PAGE_SIZE, EMU_PAGE_SIZE); + return 0; +} + +/* The real ext_flash_read() and nand_flash struct from hal/sama5d3.c + * (extracted by the Makefile). */ +#include "sama5d3_read_extract.h" + +/* Read buffer plus an adjacent canary region: a multi-page read ending + * mid-page used to write a full page past the requested length. */ +#define SCRATCH_LEN (EMU_PAGE_SIZE * 4) +static uint8_t scratch[SCRATCH_LEN + 64]; +#define CANARY (scratch + SCRATCH_LEN) +#define CANARY_LEN 64 + +static void setup(void) +{ + uint32_t i; + + /* Deterministic pattern: every byte depends on row and column. */ + for (i = 0; i < EMU_TOTAL; i++) + emu_nand[i] = (uint8_t)((i / EMU_PAGE_SIZE) * 7 + i); + + nand_flash.page_size = EMU_PAGE_SIZE; + nand_flash.block_size = EMU_BLOCK_SIZE; + nand_flash.block_count = EMU_BLOCKS; + nand_flash.pages_per_block = EMU_PAGES_PER_BLK; + nand_flash.pages_per_device = EMU_BLOCKS * EMU_PAGES_PER_BLK; + nand_flash.total_size = EMU_TOTAL; +} + +static void teardown(void) +{ +} + +static void fill_expected(uint8_t *dst, uint32_t address, uint32_t len) +{ + uint32_t i; + + for (i = 0; i < len; i++) + dst[i] = emu_nand[address + i]; +} + +/* Run one read and compare against the emulated device byte for byte. */ +static void read_case(uint32_t address, int len) +{ + uint8_t expected[SCRATCH_LEN]; + int i; + int ret; + + ck_assert_int_lt(len, SCRATCH_LEN); + memset(scratch, 0xEE, sizeof(scratch)); + fill_expected(expected, address, (uint32_t)len); + + ret = ext_flash_read(address, scratch, len); + ck_assert_int_eq(ret, len); + if (len > 0) + ck_assert_mem_eq(scratch, expected, (size_t)len); + + /* The canary must be untouched: nothing may be written past len. */ + for (i = 0; i < CANARY_LEN; i++) + ck_assert_uint_eq(CANARY[i], 0xEE); +} + +START_TEST(test_read_zero_length) +{ + memset(scratch, 0xEE, sizeof(scratch)); + ck_assert_int_eq(ext_flash_read(0x4000, scratch, 0), 0); + ck_assert_uint_eq(scratch[0], 0xEE); +} +END_TEST + +START_TEST(test_read_aligned_full_page) +{ + read_case(0, EMU_PAGE_SIZE); + read_case(0x10000, EMU_PAGE_SIZE); +} +END_TEST + +START_TEST(test_read_unaligned_small) +{ + /* Mid-page starts: the head of the page must not be returned. */ + read_case(1, 4); + read_case(0x7FC, 8); + read_case(0x400, 64); +} +END_TEST + +START_TEST(test_read_subword_lengths) +{ + /* 1-3 byte reads copy nothing in a word-count loop. */ + read_case(0x800, 1); + read_case(0x801, 2); + read_case(0x1800, 3); +} +END_TEST + +START_TEST(test_read_sha_block_pattern) +{ + /* The integrity check hashes the image in 64-byte blocks from + * fw_base + offset: every block after the first in each page is + * an unaligned small read. */ + uint32_t offset; + + for (offset = 0; offset < EMU_PAGE_SIZE; offset += 0x40) + read_case(0x800 + offset, 64); +} +END_TEST + +START_TEST(test_read_page_boundaries) +{ + read_case(0, EMU_PAGE_SIZE - 1); + read_case(0, EMU_PAGE_SIZE + 1); + /* Start near the end of a page and cross into the next. */ + read_case(0x7F0, 0x20); + read_case(0x7FF, 0x101); +} +END_TEST + +START_TEST(test_read_multipage_partial_tail) +{ + /* Multi-page reads whose tail is shorter than a page (and not a + * multiple of 4): the tail page must not be written in full. */ + read_case(0x100, 0x903); + read_case(0, 0x1805); + read_case(0x40, 0x1F01); +} +END_TEST + +START_TEST(test_read_cross_block) +{ + read_case(0x7F00, 0x120); + read_case(0x7000, 0x200); + read_case(0x7001, 0x1000); +} +END_TEST + +Suite *sama5d3_ext_read_suite(void) +{ + Suite *s = suite_create("sama5d3-ext-read"); + TCase *tc = tcase_create("sama5d3-ext-read"); + + tcase_add_checked_fixture(tc, setup, teardown); + tcase_add_test(tc, test_read_zero_length); + tcase_add_test(tc, test_read_aligned_full_page); + tcase_add_test(tc, test_read_unaligned_small); + tcase_add_test(tc, test_read_subword_lengths); + tcase_add_test(tc, test_read_sha_block_pattern); + tcase_add_test(tc, test_read_page_boundaries); + tcase_add_test(tc, test_read_multipage_partial_tail); + tcase_add_test(tc, test_read_cross_block); + suite_add_tcase(s, tc); + + return s; +} + +int main(void) +{ + int fails; + Suite *s = sama5d3_ext_read_suite(); + SRunner *sr = srunner_create(s); + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + + return fails; +} From 0a6a5fe36017b327e3d5d84b8b3ee72b30f28205 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 22:41:25 +0200 Subject: [PATCH 02/27] F-7381: fail SAMA5D3 NAND write/erase instead of reporting success ext_flash_write() and ext_flash_erase() discarded their arguments and returned 0, so any update flow that reaches this HAL was told an erase/program succeeded while the NAND contents were left untouched. NAND page program and block erase are not implemented for this target; report failure so callers that branch on the result can detect it instead of continuing with a write that never happened. The read-only boot path is unaffected. --- hal/sama5d3.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/hal/sama5d3.c b/hal/sama5d3.c index 613173c1d0..d572696a89 100644 --- a/hal/sama5d3.c +++ b/hal/sama5d3.c @@ -708,20 +708,23 @@ static void dbgu_init(void) { int ext_flash_write(uintptr_t address, const uint8_t *data, int len) { - /* TODO */ + /* SAMA5D3 NAND page program is not implemented: this HAL only + * supports reading NAND. Report failure instead of pretending the + * data was programmed. */ (void)address; (void)data; (void)len; - return 0; + return -1; } int ext_flash_erase(uintptr_t address, int len) { - /* TODO */ + /* SAMA5D3 NAND block erase is not implemented: see ext_flash_write. */ (void)address; (void)len; - return 0; + + return -1; } /* SAMA5D3 NAND flash does not have an enable pin */ From 431b01560df7140bada0f5eef9167c4af596cbf4 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 22:41:58 +0200 Subject: [PATCH 03/27] F-9759: document that QE/FMan microcode is not authenticated The P1021 QE and T10xx QE/FMan microcode blobs are validated only for structural integrity (header magic, version, size bounds) before they are activated; no cryptographic authentication is applied. State that in Targets.md for both targets, with the deployment-model reasoning: the P1021 microcode region sits inside the update partition, and the T10xx regions require local board flash write access, which also allows replacing the wolfBoot image itself. Same residual claim as F-8000 and F-9761 (T10xx QE and FMan paths); the bounds fixes for all three are already in master (000c05c7, 91c020a2). Product decision: no microcode authentication feature. --- docs/Targets.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/docs/Targets.md b/docs/Targets.md index dcc5892848..76643e9d41 100644 --- a/docs/Targets.md +++ b/docs/Targets.md @@ -5736,6 +5736,14 @@ A first stage loader is required to load the wolfBoot image into DDR for executi | fsl_qe_ucode_1021_10_A.bin | 0x01F00000 | | swap block | 0x02200000 | +The QE microcode programmed at `0x01F00000` is validated only for +structural integrity (header magic, version, and size bounds) before it +is activated; it is not cryptographically authenticated. This is out of +scope for the example configuration: the microcode region sits inside +the update partition, so replacing it requires the same flash write +access that would let an attacker replace the update image itself, +which wolfBoot's image authentication already protects against. + ### Building wolfBoot for NXP P1021 PPC By default wolfBoot will use `powerpc-linux-gnu-` cross-compiler prefix. These tools can be installed with the Debian package `gcc-powerpc-linux-gnu` (`sudo apt install gcc-powerpc-linux-gnu`). @@ -5888,6 +5896,13 @@ Note: On T1040, FMAN and QE firmware share the same 128KB NOR erase sector (0xEFF00000-0xEFF1FFFF). They must be programmed together in a single erase/write operation. +The QE and FMan microcode in these NOR regions is validated only for +structural integrity (header magic, version, and size bounds) before it +is activated; it is not cryptographically authenticated. This is out of +scope for the example configurations: programming these regions requires +local write access to the board's flash, which also allows replacing the +wolfBoot image itself - a threat the signed image flow already covers. + ### Design Both T1024 and T1040 use a two-stage boot. Stage1 runs XIP from NOR flash, From ce5f20c74d1ba5413529a5e2c0e7751784b48dfd Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 22:58:06 +0200 Subject: [PATCH 04/27] F-7970: free the LMS key on the parameter/import error paths wolfBoot_verify_signature_lms() returned without calling wc_LmsKey_Free() when wc_LmsKey_SetParameters() or wc_LmsKey_ImportPubRaw() failed after a successful wc_LmsKey_Init(), leaking the key (and whatever heap state the LMS backend allocated for it). Free the key on both error paths, matching the success path. --- src/image.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/image.c b/src/image.c index f9bf6bfe84..33700fc6cc 100644 --- a/src/image.c +++ b/src/image.c @@ -691,6 +691,7 @@ static void wolfBoot_verify_signature_lms(uint8_t key_slot, wolfBoot_printf("error: wc_LmsKey_SetParameters(%d, %d, %d)" \ " returned %d\n", LMS_LEVELS, LMS_HEIGHT, LMS_WINTERNITZ, ret); + wc_LmsKey_Free(&lms); return; } @@ -703,6 +704,7 @@ static void wolfBoot_verify_signature_lms(uint8_t key_slot, /* Something is wrong with the pub key or LMS parameters. */ wolfBoot_printf("error: wc_LmsKey_ImportPubRaw" \ " returned %d\n", ret); + wc_LmsKey_Free(&lms); return; } From d722c0f2bcfee7c37f309b692fa568338dabca0f Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 22:58:29 +0200 Subject: [PATCH 05/27] F-7971: free the ECC key on the wolfHSM setup error paths In the wolfHSM branch of wolfBoot_verify_signature_ecc(), both failure paths after wc_ecc_init_ex() - the HSM key-ID setup and wc_ecc_import_unsigned() - did a bare return that skipped the wc_ecc_free(&ecc) at the end of the function, leaking the key (and its heap-allocated mp_ints in non-SP-math builds). Free the key before returning on both paths, matching the plain wolfCrypt path. The wc_ecc_import_unsigned() path sits in the server-only, non-cert-chain configuration, which does not compile today (it references pubkey/ point_sz, declared only for the software and client builds) - see the F-7995 note; the free is added there for consistency so the path is correct if that configuration is ever made buildable. --- src/image.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/image.c b/src/image.c index 33700fc6cc..7741f50283 100644 --- a/src/image.c +++ b/src/image.c @@ -355,6 +355,7 @@ static void wolfBoot_verify_signature_ecc(uint8_t key_slot, #endif #endif /* !WOLFBOOT_CERT_CHAIN_VERIFY */ if (ret != 0) { + wc_ecc_free(&ecc); return; } #else @@ -364,6 +365,7 @@ static void wolfBoot_verify_signature_ecc(uint8_t key_slot, ret = wc_ecc_import_unsigned(&ecc, pubkey, pubkey + point_sz, NULL, ECC_KEY_TYPE); if (ret != 0) { + wc_ecc_free(&ecc); return; } From c5ae368021f52bec43abea5838ffc129b5567ae8 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 23:02:15 +0200 Subject: [PATCH 06/27] F-7974: declare hdr_cpy_done as int in both translation units libwolfboot.c defined the external-flash header-cache flag as uint32_t while image.c declared it extern int - the same object with incompatible types in two translation units is undefined behaviour. It is benign on every supported target (both are 32-bit) but a latent portability defect; the flag is written from both files. Make the definition int, matching both declarations. --- src/libwolfboot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libwolfboot.c b/src/libwolfboot.c index 6274550109..ecd7eadfb6 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -1402,7 +1402,7 @@ uint16_t wolfBoot_find_header(uint8_t *haystack, uint16_t type, uint8_t **ptr) #ifdef EXT_FLASH uint8_t hdr_cpy[IMAGE_HEADER_SIZE] XALIGNED(4); -uint32_t hdr_cpy_done = 0; +int hdr_cpy_done = 0; #endif /** From 8aa5221fee83dfb368ef3e9a732ddd100fff46ed Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 23:19:06 +0200 Subject: [PATCH 07/27] F-7056: fix node-found guards and esdhc log label in hal_dts_fixup The fman and esdhc fixup guards tested 'off != !FDT_ERR_NOTFOUND', which is 'off != 0'. fdt_node_offset_by_compatible() returns a negative offset when the node is absent, so the guard passed and the fixups ran with a negative offset (fdt_setprop() rejects it, the fixups fail silently with misleading log lines); conversely a node at struct offset 0 (the root node) matched but was skipped. Every other guard in the function already used '-FDT_ERR_NOTFOUND'. Use the correct guard in both blocks and fix the esdhc status fixup's log label, a copy-paste from the cpu fixup block. unit-t10xx-dts-memac gains four cases driving the real hal_dts_fixup(): a root node compatible with fsl,fman gets the clock fixup (failed before the guard fix), child fman and esdhc nodes get their fixups, and absent nodes are skipped cleanly. --- hal/nxp_t10xx.c | 6 +- tools/unit-tests/unit-t10xx-dts-memac.c | 119 ++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 3 deletions(-) diff --git a/hal/nxp_t10xx.c b/hal/nxp_t10xx.c index be46f95cc0..954e2ff9f4 100644 --- a/hal/nxp_t10xx.c +++ b/hal/nxp_t10xx.c @@ -3522,7 +3522,7 @@ int hal_dts_fixup(void* dts_addr) /* fixup the fman clock */ off = fdt_node_offset_by_compatible(fdt, -1, "fsl,fman"); - if (off != !FDT_ERR_NOTFOUND) { + if (off != -FDT_ERR_NOTFOUND) { fdt_fixup_val(fdt, off, "fman@", "clock-frequency", hal_get_bus_clk()); } @@ -3617,9 +3617,9 @@ int hal_dts_fixup(void* dts_addr) /* fix SDHC */ off = fdt_node_offset_by_compatible(fdt, -1, "fsl,esdhc"); - if (off != !FDT_ERR_NOTFOUND) { + if (off != -FDT_ERR_NOTFOUND) { fdt_fixup_val(fdt, off, "sdhc@", "clock-frequency", hal_get_bus_clk()); - fdt_fixup_str(fdt, off, "cpu", "status", "okay"); + fdt_fixup_str(fdt, off, "sdhc@", "status", "okay"); } #endif /* !BUILD_LOADER_STAGE1 */ diff --git a/tools/unit-tests/unit-t10xx-dts-memac.c b/tools/unit-tests/unit-t10xx-dts-memac.c index 44c0249892..f4db6232cd 100644 --- a/tools/unit-tests/unit-t10xx-dts-memac.c +++ b/tools/unit-tests/unit-t10xx-dts-memac.c @@ -439,6 +439,121 @@ START_TEST(test_memac_mixed_oob_then_valid) } END_TEST +/* Build a DTB with a memory node and one node with the given + * compatible. */ +static void dtb_build_compat_node(struct dtb *d, const char *node, + const char *compat) +{ + uint32_t reg[4] = {cpu_to_fdt32(0), cpu_to_fdt32(0), cpu_to_fdt32(0), + cpu_to_fdt32(0x10000000U)}; + + dtb_init(d); + dtb_begin_node(d, ""); + dtb_begin_node(d, "memory"); + dtb_prop_raw(d, "reg", reg, sizeof(reg)); + dtb_end_node(d); + dtb_begin_node(d, node); + dtb_prop_raw(d, "compatible", compat, strlen(compat) + 1); + dtb_end_node(d); + dtb_end_node(d); /* root */ + dtb_finalize(d); +} + +/* Build a DTB whose root node (struct offset 0) carries the given + * compatible. */ +static void dtb_build_root_compat(struct dtb *d, const char *compat) +{ + dtb_init(d); + dtb_begin_node(d, ""); + dtb_prop_raw(d, "compatible", compat, strlen(compat) + 1); + dtb_end_node(d); + dtb_finalize(d); +} + +/* The fman/esdhc node-found guards used `off != !FDT_ERR_NOTFOUND`, + * which is `off != 0`: a node at struct offset 0 (the root) was + * skipped even when it matched, and the fixup ran with a negative + * offset when no node matched. A root node compatible with fsl,fman + * must get the clock fixup. */ +START_TEST(test_fman_root_node_compatible_fixed) +{ + struct dtb d; + int off, len; + const void *clk; + + dtb_build_root_compat(&d, "fsl,fman"); + ck_assert_int_eq(hal_dts_fixup(d.buf), 0); + + off = fdt_node_offset_by_compatible(d.buf, -1, "fsl,fman"); + ck_assert_int_eq(off, 0); /* the root node */ + clk = fdt_getprop(d.buf, off, "clock-frequency", &len); + ck_assert_ptr_nonnull(clk); + ck_assert_int_eq(len, 4); + ck_assert_uint_eq(fdt32_to_cpu(*(const uint32_t *)clk), 100000000U); +} +END_TEST + +/* A child fman node gets the clock fixup. */ +START_TEST(test_fman_child_node_fixed) +{ + struct dtb d; + int off, len; + const void *clk; + + dtb_build_compat_node(&d, "fman", "fsl,fman"); + ck_assert_int_eq(hal_dts_fixup(d.buf), 0); + + off = fdt_node_offset_by_compatible(d.buf, -1, "fsl,fman"); + ck_assert_int_gt(off, 0); + clk = fdt_getprop(d.buf, off, "clock-frequency", &len); + ck_assert_ptr_nonnull(clk); + ck_assert_int_eq(len, 4); + ck_assert_uint_eq(fdt32_to_cpu(*(const uint32_t *)clk), 100000000U); +} +END_TEST + +/* An esdhc node gets the clock fixup and status=okay. */ +START_TEST(test_esdhc_node_fixed) +{ + struct dtb d; + int off, len; + const void *clk; + const void *status; + + dtb_build_compat_node(&d, "esdhc", "fsl,esdhc"); + ck_assert_int_eq(hal_dts_fixup(d.buf), 0); + + off = fdt_node_offset_by_compatible(d.buf, -1, "fsl,esdhc"); + ck_assert_int_gt(off, 0); + clk = fdt_getprop(d.buf, off, "clock-frequency", &len); + ck_assert_ptr_nonnull(clk); + ck_assert_int_eq(len, 4); + ck_assert_uint_eq(fdt32_to_cpu(*(const uint32_t *)clk), 100000000U); + status = fdt_getprop(d.buf, off, "status", &len); + ck_assert_ptr_nonnull(status); + ck_assert_int_eq(len, 5); + ck_assert_str_eq(status, "okay"); +} +END_TEST + +/* No fman/esdhc nodes: the fixups are skipped cleanly and the fixup + * still succeeds. */ +START_TEST(test_fman_esdhc_absent_skipped) +{ + struct dtb d; + int off; + + dtb_build_compat_node(&d, "ethernet", "fsl,eth"); + ck_assert_int_eq(hal_dts_fixup(d.buf), 0); + + ck_assert_int_eq(fdt_node_offset_by_compatible(d.buf, -1, "fsl,fman"), + -FDT_ERR_NOTFOUND); + off = fdt_node_offset_by_compatible(d.buf, -1, "fsl,eth"); + ck_assert_int_gt(off, 0); + ck_assert_ptr_null(fdt_getprop(d.buf, off, "clock-frequency", NULL)); +} +END_TEST + Suite *t10xx_dts_memac_suite(void) { Suite *s = suite_create("t10xx-dts-memac"); @@ -449,6 +564,10 @@ Suite *t10xx_dts_memac_suite(void) tcase_add_test(tc, test_memac_unmapped_cell_index_skipped); tcase_add_test(tc, test_memac_valid_cell_index_fixed); tcase_add_test(tc, test_memac_mixed_oob_then_valid); + tcase_add_test(tc, test_fman_root_node_compatible_fixed); + tcase_add_test(tc, test_fman_child_node_fixed); + tcase_add_test(tc, test_esdhc_node_fixed); + tcase_add_test(tc, test_fman_esdhc_absent_skipped); suite_add_tcase(s, tc); return s; From 08903c252eb03e65af53d1153608a79297c74960 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 23:30:35 +0200 Subject: [PATCH 08/27] F-7990: bound the compatible string walk to declared lengths fdt_node_offset_by_compatible() compared each entry of a multi-string compatible value with memcmp(compatible, prop, complen+1) before locating the entry's NUL terminator. When the declared property length equals the search length (no room for a NUL), the comparison reads one byte past the property data and accepts the entry when that byte happens to be zero (e.g. the 4-byte alignment padding). Locate each entry's NUL terminator within the declared length first and only compare entries whose length equals the search length: nothing is read past the property, and an unterminated trailing entry can no longer match. unit-fdt gains four cases with a minimal single-node FDT builder: an unterminated exact-length entry does not match (it did before the fix), a terminated exact-length entry matches, multi-string lists still match on later entries, and a longer entry that starts with the search string does not match. --- src/fdt.c | 27 ++++++---- tools/unit-tests/unit-fdt.c | 98 +++++++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 10 deletions(-) diff --git a/src/fdt.c b/src/fdt.c index e949747ad0..8b15465df4 100644 --- a/src/fdt.c +++ b/src/fdt.c @@ -726,21 +726,28 @@ int fdt_node_offset_by_compatible(const void *fdt, int startoffset, int len; const char *prop = (const char*)fdt_getprop(fdt, offset, "compatible", &len); - /* property list may contain multiple null terminated strings */ - while (prop != NULL && len >= complen) { + /* property list may contain multiple null terminated strings. + * Locate each entry's NUL terminator within the declared length + * first, then compare: the entry must be exactly as long as the + * wanted string, so no byte is ever read past the property and + * an unterminated trailing entry can neither match nor be + * misread as one. */ + while (prop != NULL && len > 0) { const char* nextprop; - if (memcmp(compatible, prop, complen+1) == 0) { - return offset; - } + int entrylen; + nextprop = memchr(prop, '\0', len); - if (nextprop != NULL) { - len -= (nextprop - prop) + 1; - prop = nextprop + 1; - } - else { + if (nextprop == NULL) { /* No NUL terminator within the declared length, break. */ break; } + entrylen = (int)(nextprop - prop); + if (entrylen == complen && + memcmp(compatible, prop, complen) == 0) { + return offset; + } + len -= entrylen + 1; + prop = nextprop + 1; } } return offset; diff --git a/tools/unit-tests/unit-fdt.c b/tools/unit-tests/unit-fdt.c index 2f8ba163d5..951ed3696d 100644 --- a/tools/unit-tests/unit-fdt.c +++ b/tools/unit-tests/unit-fdt.c @@ -203,6 +203,100 @@ START_TEST(test_fdt_node_offset_by_compatible_terminates_on_unterminated_prop) } END_TEST +/* Build a minimal FDT in buf (zero-initialized, so padding bytes are + * 0x00): root node with a `compatible` property whose raw value is + * `len` bytes at `val`. */ +static void build_compat_fdt(uint8_t *buf, size_t size, + const uint8_t *val, uint32_t len) +{ + struct fdt_header *hdr; + uint32_t *s; + uint32_t val_aligned = (len + 3u) & ~3u; + uint32_t struct_off = 0x40; + uint32_t strings_off = 0x80; + + memset(buf, 0, size); + + hdr = (struct fdt_header *)buf; + fdt_set_totalsize(hdr, 0x100); + fdt_set_off_dt_struct(hdr, struct_off); + fdt_set_off_dt_strings(hdr, strings_off); + fdt_set_off_mem_rsvmap(hdr, 0x28); + fdt_set_version(hdr, 17); + fdt_set_last_comp_version(hdr, 16); + fdt_set_size_dt_struct(hdr, + 8u + 12u + val_aligned + 4u + 4u); /* node hdr, prop, end, FDT_END */ + fdt_set_size_dt_strings(hdr, sizeof("compatible")); + memcpy(buf + strings_off, "compatible", sizeof("compatible")); + + s = (uint32_t *)(buf + struct_off); + s[0] = fdt32_to_cpu(FDT_BEGIN_NODE); /* root */ + s[1] = 0; /* empty name */ + s[2] = fdt32_to_cpu(FDT_PROP); + s[3] = fdt32_to_cpu(len); + s[4] = fdt32_to_cpu(0); /* nameoff of "compatible" */ + memcpy(buf + struct_off + 8 + 12, val, len); + s[5 + val_aligned / 4u] = fdt32_to_cpu(FDT_END_NODE); + s[6 + val_aligned / 4u] = fdt32_to_cpu(FDT_END); +} + +/* A compatible entry whose declared length equals the search string + * (no room for a NUL) must not match: before the fix the comparison + * read one byte past the declared length (the 4-byte alignment + * padding, zero here) as if it were the terminator and accepted the + * entry. */ +START_TEST(test_fdt_compatible_unterminated_exact_len_no_match) +{ + static uint8_t buf[0x100]; + int off; + + build_compat_fdt(buf, sizeof(buf), (const uint8_t *)"abc", 3); + + off = fdt_node_offset_by_compatible(buf, -1, "abc"); + ck_assert_int_lt(off, 0); +} +END_TEST + +/* A properly terminated entry of exactly the search length matches. */ +START_TEST(test_fdt_compatible_terminated_exact_len_match) +{ + static uint8_t buf[0x100]; + int off; + + build_compat_fdt(buf, sizeof(buf), (const uint8_t *)"abc\0", 4); + + off = fdt_node_offset_by_compatible(buf, -1, "abc"); + ck_assert_int_eq(off, 0); /* the root node */ +} +END_TEST + +/* Multi-string compatible lists keep working: the second entry + * matches. */ +START_TEST(test_fdt_compatible_multi_string_list_match) +{ + static uint8_t buf[0x100]; + int off; + + build_compat_fdt(buf, sizeof(buf), (const uint8_t *)"xy\0abc\0", 8); + + off = fdt_node_offset_by_compatible(buf, -1, "abc"); + ck_assert_int_eq(off, 0); +} +END_TEST + +/* A terminated entry that merely starts with the search string does + * not match (the entry is longer). */ +START_TEST(test_fdt_compatible_prefix_entry_no_match) +{ + static uint8_t buf[0x100]; + int off; + + build_compat_fdt(buf, sizeof(buf), (const uint8_t *)"abcd\0", 5); + + off = fdt_node_offset_by_compatible(buf, -1, "abc"); + ck_assert_int_lt(off, 0); +} + static Suite *fdt_suite(void) { Suite *s = suite_create("fdt"); @@ -215,6 +309,10 @@ static Suite *fdt_suite(void) tcase_add_test(tc, test_fdt_get_string_returns_string_with_valid_offset); tcase_add_test(tc, test_fit_load_image_rejects_oversized_prop_len); tcase_add_test(tc, test_fdt_shrink_rejects_dt_strings_area_overflow); + tcase_add_test(tc, test_fdt_compatible_unterminated_exact_len_no_match); + tcase_add_test(tc, test_fdt_compatible_terminated_exact_len_match); + tcase_add_test(tc, test_fdt_compatible_multi_string_list_match); + tcase_add_test(tc, test_fdt_compatible_prefix_entry_no_match); suite_add_tcase(s, tc); tcase_set_timeout(tc_dos, 5); From 19d8a9e346062aa758392277cd50232f7435a8d6 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 23:59:33 +0200 Subject: [PATCH 09/27] F-9758: validate FDT layout in fdt_check_header, use it in fdt_get_string fdt_get_string() bounded stroffset against size_dt_strings but formed the string-table pointer from off_dt_strings without ever validating either header field against totalsize; a DTB declaring a large off_dt_strings with a small size_dt_strings made every property lookup (fdt_getprop -> fdt_get_string) scan far outside the blob. Validate the structural layout in fdt_check_header() for finalized (FDT_MAGIC) blobs: the reservation map, structure block and string table must sit inside the blob and not overlap, checked in 64-bit so the size fields cannot wrap. fdt_get_string() now requires a valid header before forming the pointer. The SW_MAGIC (in-progress edit) state keeps its existing check, since its layout is different. Test fixtures are adjusted to the validated layout: the two pre-existing fdt_get_string fixtures now set the header fields the lookup relies on, the compatible-test builder sets the magic word and points the reservation map at the canonical empty list right after the header (it pointed into the string table before). --- src/fdt.c | 25 ++++++++++++++ tools/unit-tests/unit-fdt.c | 46 +++++++++++++++++++++++++ tools/unit-tests/unit-t10xx-dts-memac.c | 4 ++- 3 files changed, 74 insertions(+), 1 deletion(-) diff --git a/src/fdt.c b/src/fdt.c index 8b15465df4..c11ef9653b 100644 --- a/src/fdt.c +++ b/src/fdt.c @@ -462,10 +462,24 @@ static int fdt_subnode_offset_namelen(const void *fdt, int offset, int fdt_check_header(const void *fdt) { if (fdt_magic(fdt) == FDT_MAGIC) { + uint32_t off_rsv = fdt_off_mem_rsvmap(fdt); + uint32_t off_struct = fdt_off_dt_struct(fdt); + uint32_t size_struct = fdt_size_dt_struct(fdt); + uint32_t off_strings = fdt_off_dt_strings(fdt); + uint32_t size_strings = fdt_size_dt_strings(fdt); + if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION) return -FDT_ERR_BADVERSION; if (fdt_last_comp_version(fdt) > FDT_LAST_SUPPORTED_VERSION) return -FDT_ERR_BADVERSION; + /* The three structural areas must sit inside the blob and not + * overlap: reservation map, structure block and string table, + * in that order. The additions are made in 64-bit so the size + * fields cannot wrap around the comparison. */ + if (off_rsv > off_struct + || (uint64_t)off_struct + size_struct > off_strings + || (uint64_t)off_strings + size_strings > fdt_totalsize(fdt)) + return -FDT_ERR_BADSTRUCTURE; } else if (fdt_magic(fdt) == FDT_SW_MAGIC) { if (fdt_size_dt_struct(fdt) == 0) @@ -579,6 +593,17 @@ const char* fdt_get_string(const void *fdt, int stroffset, int *lenp) uint32_t strsize = fdt_size_dt_strings(fdt); const char *s; const char *end; + int err; + + /* off_dt_strings/size_dt_strings are attacker-influenceable header + * fields; validate the layout against totalsize before forming the + * string-table pointer. */ + err = fdt_check_header(fdt); + if (err != 0) { + if (lenp) + *lenp = err; + return NULL; + } if ((stroffset < 0) || ((uint32_t)stroffset >= strsize)) { if (lenp) diff --git a/tools/unit-tests/unit-fdt.c b/tools/unit-tests/unit-fdt.c index 951ed3696d..516eca3f6f 100644 --- a/tools/unit-tests/unit-fdt.c +++ b/tools/unit-tests/unit-fdt.c @@ -43,8 +43,12 @@ START_TEST(test_fdt_get_string_rejects_out_of_range_offset) const char *s; memset(&blob, 0, sizeof(blob)); + fdt_set_totalsize(&blob, sizeof(blob.hdr) + sizeof(blob.strings)); fdt_set_off_dt_strings(&blob, sizeof(blob.hdr)); fdt_set_size_dt_strings(&blob, sizeof(blob.strings)); + fdt_set_magic(&blob, FDT_MAGIC); + fdt_set_version(&blob, 17); + fdt_set_last_comp_version(&blob, 16); memcpy(blob.strings, "chosen", sizeof("chosen")); blob.after[0] = 'X'; blob.after[1] = '\0'; @@ -66,8 +70,12 @@ START_TEST(test_fdt_get_string_returns_string_with_valid_offset) const char *s; memset(&blob, 0, sizeof(blob)); + fdt_set_totalsize(&blob, sizeof(blob.hdr) + sizeof(blob.strings)); fdt_set_off_dt_strings(&blob, sizeof(blob.hdr)); fdt_set_size_dt_strings(&blob, sizeof(blob.strings)); + fdt_set_magic(&blob, FDT_MAGIC); + fdt_set_version(&blob, 17); + fdt_set_last_comp_version(&blob, 16); memcpy(blob.strings, "serial\0console\0", 15); s = fdt_get_string(&blob, 7, &len); @@ -218,6 +226,7 @@ static void build_compat_fdt(uint8_t *buf, size_t size, memset(buf, 0, size); hdr = (struct fdt_header *)buf; + hdr->magic = fdt32_to_cpu(FDT_MAGIC); fdt_set_totalsize(hdr, 0x100); fdt_set_off_dt_struct(hdr, struct_off); fdt_set_off_dt_strings(hdr, strings_off); @@ -296,6 +305,41 @@ START_TEST(test_fdt_compatible_prefix_entry_no_match) off = fdt_node_offset_by_compatible(buf, -1, "abc"); ck_assert_int_lt(off, 0); } +END_TEST + +/* A finalized DTB whose string table lies past the declared end of + * the blob must be rejected: before the fix fdt_check_header() + * validated only magic and version, and fdt_get_string() formed the + * string-table pointer from the unvalidated header fields. */ +START_TEST(test_fdt_check_header_rejects_unbounded_string_area) +{ + static uint8_t buf[0x100]; + int len = 0; + const char *s; + + build_compat_fdt(buf, sizeof(buf), (const uint8_t *)"abc\0", 4); + /* push the string table past the declared end of the blob */ + fdt_set_off_dt_strings((struct fdt_header *)buf, 0x100); + + ck_assert_int_eq(fdt_check_header(buf), -FDT_ERR_BADSTRUCTURE); + + s = fdt_get_string(buf, 0, &len); + ck_assert_ptr_null(s); + ck_assert_int_lt(len, 0); +} +END_TEST + +/* The structure block must not overlap the string table. */ +START_TEST(test_fdt_check_header_rejects_overlapping_areas) +{ + static uint8_t buf[0x100]; + + build_compat_fdt(buf, sizeof(buf), (const uint8_t *)"abc\0", 4); + fdt_set_size_dt_struct((struct fdt_header *)buf, 0x100); + + ck_assert_int_eq(fdt_check_header(buf), -FDT_ERR_BADSTRUCTURE); +} +END_TEST static Suite *fdt_suite(void) { @@ -313,6 +357,8 @@ static Suite *fdt_suite(void) tcase_add_test(tc, test_fdt_compatible_terminated_exact_len_match); tcase_add_test(tc, test_fdt_compatible_multi_string_list_match); tcase_add_test(tc, test_fdt_compatible_prefix_entry_no_match); + tcase_add_test(tc, test_fdt_check_header_rejects_unbounded_string_area); + tcase_add_test(tc, test_fdt_check_header_rejects_overlapping_areas); suite_add_tcase(s, tc); tcase_set_timeout(tc_dos, 5); diff --git a/tools/unit-tests/unit-t10xx-dts-memac.c b/tools/unit-tests/unit-t10xx-dts-memac.c index f4db6232cd..cfb86b6e04 100644 --- a/tools/unit-tests/unit-t10xx-dts-memac.c +++ b/tools/unit-tests/unit-t10xx-dts-memac.c @@ -261,7 +261,9 @@ static void dtb_finalize(struct dtb *d) hdr->totalsize = cpu_to_fdt32(0x2000); hdr->off_dt_struct = cpu_to_fdt32(d->struct_off); hdr->off_dt_strings = cpu_to_fdt32(d->strings_off); - hdr->off_mem_rsvmap = cpu_to_fdt32(d->strings_off); /* no reservations */ + hdr->off_mem_rsvmap = cpu_to_fdt32(0x28); + /* the 8 bytes after the 40-byte header are zero: an empty + * reservation list at the canonical spot */ hdr->version = cpu_to_fdt32(17); hdr->last_comp_version = cpu_to_fdt32(16); hdr->boot_cpuid_phys = cpu_to_fdt32(0); From fe1dcb080d61115a144c0f2076a5acd066d2e4b9 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 20 Aug 2026 23:59:53 +0200 Subject: [PATCH 10/27] F-9757: length-bound the FIT name/compression string properties fit_find_images() took the FIT configuration's image names (kernel/fdt/ramdisk/fpga) and the configuration name (default) straight from fdt_getprop() and passed them on to fdt_find_node_offset(), which strlen()s them; fit_load_image_inner() strcmp()'d the compression property after only checking it was non-empty. A property value not NUL-terminated within its declared length makes those calls scan past the property - and past the end of the blob for a property at the tail. Add fit_getprop_string(), which returns the property value only when it is NUL-terminated within its declared length, and use it for the five name properties (a malformed value is rejected and the type-based search still applies). Compare compression within the declared length: the value must be exactly "gzip" or "none"; any other shape fails closed with the existing unsupported-compression path instead of being strcmp()'d past the property. unit-fdt gains a FIT whose configuration kernel property is unterminated (the valid default is still honored, the image name is rejected); unit-fit-gzip gains a truncated compression="none" value, which used to pass the subimage through as raw and now fails closed. Both build variants (gzip enabled/disabled) run it. --- src/fdt.c | 44 +++++++++++++++------ tools/unit-tests/unit-fdt.c | 67 ++++++++++++++++++++++++++++++++ tools/unit-tests/unit-fit-gzip.c | 39 +++++++++++++++++++ 3 files changed, 139 insertions(+), 11 deletions(-) diff --git a/src/fdt.c b/src/fdt.c index c11ef9653b..b388d0d19a 100644 --- a/src/fdt.c +++ b/src/fdt.c @@ -940,6 +940,22 @@ int fdt_fixup_val64(void* fdt, int off, const char* node, const char* name, /* FIT Specific */ + +/* Returns the property value only when it is a NUL-terminated C string + * within its declared length, else NULL: property values are opaque + * byte arrays and the names taken from them are passed to + * fdt_find_node_offset()/strcmp(), which strlen() them. */ +static const char* fit_getprop_string(const void* fdt, int offset, + const char* name) +{ + int len = 0; + const char* val = (const char*)fdt_getprop(fdt, offset, name, &len); + + if (val == NULL || len <= 0 || memchr(val, '\0', len) == NULL) + return NULL; + return val; +} + const char* fit_find_images(void* fdt, const char** pkernel, const char** pflat_dt, const char** pramdisk, const char** pfpga) { @@ -968,19 +984,16 @@ const char* fit_find_images(void* fdt, const char** pkernel, const char** pflat_ if (conf == NULL) #endif { - val = fdt_getprop(fdt, off, "default", &len); - if (val != NULL && len > 0) { - conf = (const char*)val; - } + conf = fit_getprop_string(fdt, off, "default"); } } if (conf != NULL) { off = fdt_find_node_offset(fdt, -1, conf); if (off > 0) { - kernel = fdt_getprop(fdt, off, "kernel", &len); - flat_dt = fdt_getprop(fdt, off, "fdt", &len); - ramdisk = fdt_getprop(fdt, off, "ramdisk", &len); - fpga = fdt_getprop(fdt, off, "fpga", &len); + kernel = fit_getprop_string(fdt, off, "kernel"); + flat_dt = fit_getprop_string(fdt, off, "fdt"); + ramdisk = fit_getprop_string(fdt, off, "ramdisk"); + fpga = fit_getprop_string(fdt, off, "fpga"); } } if (kernel == NULL) { @@ -1219,11 +1232,20 @@ static void* fit_load_image_inner(void* fdt, const char* image, int* lenp, * raw. */ comp = (const char*)fdt_getprop(fdt, off, "compression", &complen); - if (comp != NULL && complen > 0) { - if (strcmp(comp, "gzip") == 0) { + /* Compare within the declared property length: the value + * must be exactly "gzip" or "none" (NUL-terminated). Any + * other shape - including an unterminated value - fails + * closed instead of being strncmp()'d past the property. */ + if (comp != NULL) { + if (complen == 5 && comp[4] == '\0' && + memcmp(comp, "gzip", 4) == 0) { is_gzip = 1; } - else if (strcmp(comp, "none") != 0) { + else if (complen == 5 && comp[4] == '\0' && + memcmp(comp, "none", 4) == 0) { + /* uncompressed */ + } + else { is_unknown_comp = 1; } } diff --git a/tools/unit-tests/unit-fdt.c b/tools/unit-tests/unit-fdt.c index 516eca3f6f..5f05b2417f 100644 --- a/tools/unit-tests/unit-fdt.c +++ b/tools/unit-tests/unit-fdt.c @@ -341,6 +341,72 @@ START_TEST(test_fdt_check_header_rejects_overlapping_areas) } END_TEST +/* FIT whose configuration `kernel` property is not NUL-terminated + * within its declared length: fit_find_images() must still honor the + * valid `default` but reject the malformed image name instead of + * passing it on as a C string. */ +static const uint8_t fit_cfg_unterminated_kernel[] = { + /* header */ + 0xd0, 0x0d, 0xfe, 0xed, /* magic */ + 0x00, 0x00, 0x00, 0xa7, /* totalsize = 167 */ + 0x00, 0x00, 0x00, 0x38, /* off_dt_struct = 56 */ + 0x00, 0x00, 0x00, 0x98, /* off_dt_strings = 152 */ + 0x00, 0x00, 0x00, 0x28, /* off_mem_rsvmap = 40 */ + 0x00, 0x00, 0x00, 0x11, /* version = 17 */ + 0x00, 0x00, 0x00, 0x10, /* last_comp_version = 16 */ + 0x00, 0x00, 0x00, 0x00, /* boot_cpuid_phys */ + 0x00, 0x00, 0x00, 0x0f, /* size_dt_strings = 15 */ + 0x00, 0x00, 0x00, 0x60, /* size_dt_struct = 96 */ + /* mem_rsvmap terminator (offset 40) */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + /* struct block (offset 56) */ + 0x00, 0x00, 0x00, 0x01, /* BEGIN_NODE root */ + 0x00, 0x00, 0x00, 0x00, /* "" */ + 0x00, 0x00, 0x00, 0x01, /* BEGIN_NODE */ + 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x75, 0x72, + 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x73, 0x00, 0x00, /* "configurations\0" */ + 0x00, 0x00, 0x00, 0x03, /* FDT_PROP */ + 0x00, 0x00, 0x00, 0x07, /* len = 7 */ + 0x00, 0x00, 0x00, 0x00, /* nameoff = 0 ("default") */ + 0x63, 0x6f, 0x6e, 0x66, 0x2d, 0x31, 0x00, 0x00, /* "conf-1\0" */ + 0x00, 0x00, 0x00, 0x01, /* BEGIN_NODE */ + 0x63, 0x6f, 0x6e, 0x66, 0x2d, 0x31, 0x00, 0x00, /* "conf-1\0" */ + 0x00, 0x00, 0x00, 0x03, /* FDT_PROP */ + 0x00, 0x00, 0x00, 0x08, /* len = 8 */ + 0x00, 0x00, 0x00, 0x08, /* nameoff = 8 ("kernel") */ + 0x6b, 0x65, 0x72, 0x6e, 0x65, 0x6c, 0x2d, 0x31, /* "kernel-1" -- no NUL */ + 0x00, 0x00, 0x00, 0x02, /* END_NODE conf-1 */ + 0x00, 0x00, 0x00, 0x02, /* END_NODE configurations */ + 0x00, 0x00, 0x00, 0x02, /* END_NODE root */ + 0x00, 0x00, 0x00, 0x09, /* FDT_END */ + /* strings block (offset 152) */ + 0x64, 0x65, 0x66, 0x61, 0x75, 0x6c, 0x74, 0x00, /* "default\0" */ + 0x6b, 0x65, 0x72, 0x6e, 0x65, 0x6c, 0x00, /* "kernel\0" */ +}; + +START_TEST(test_fit_find_images_rejects_unterminated_image_name) +{ + static uint8_t fit_scratch[sizeof(fit_cfg_unterminated_kernel)]; + const char *conf = NULL, *kern = NULL, *fdt = NULL; + const char *rd = NULL, *fpga = NULL; + + memcpy(fit_scratch, fit_cfg_unterminated_kernel, sizeof(fit_scratch)); + + conf = fit_find_images(fit_scratch, &kern, &fdt, &rd, &fpga); + + /* The valid `default` is still honored... */ + ck_assert_str_eq(conf, "conf-1"); + /* ...but the config's `kernel` property is not NUL-terminated + * within its declared length, so it must be rejected rather than + * passed on to fdt_find_node_offset()/strlen(). */ + ck_assert_ptr_null(kern); + ck_assert_ptr_null(fdt); + ck_assert_ptr_null(rd); + ck_assert_ptr_null(fpga); +} +END_TEST + static Suite *fdt_suite(void) { Suite *s = suite_create("fdt"); @@ -359,6 +425,7 @@ static Suite *fdt_suite(void) tcase_add_test(tc, test_fdt_compatible_prefix_entry_no_match); tcase_add_test(tc, test_fdt_check_header_rejects_unbounded_string_area); tcase_add_test(tc, test_fdt_check_header_rejects_overlapping_areas); + tcase_add_test(tc, test_fit_find_images_rejects_unterminated_image_name); suite_add_tcase(s, tc); tcase_set_timeout(tc_dos, 5); diff --git a/tools/unit-tests/unit-fit-gzip.c b/tools/unit-tests/unit-fit-gzip.c index 373a4be81d..7c175fdacc 100644 --- a/tools/unit-tests/unit-fit-gzip.c +++ b/tools/unit-tests/unit-fit-gzip.c @@ -269,6 +269,44 @@ END_TEST #endif /* WOLFBOOT_GZIP */ +START_TEST(test_fit_to_none_unterminated_fails_closed) +{ + /* The `compression` property is truncated to 4 bytes ("none" with + * no NUL): the declared length no longer covers the terminator. + * Before the fix the value was strcmp()'d and the following byte + * (the dropped NUL) terminated it, so the subimage was passed + * through as raw; a malformed value must fail closed. */ + uint8_t buf[64]; + int len = -1; + void *ret; + static uint8_t fit_scratch[sizeof(fit_with_none_comp)]; + uint8_t *p; + unsigned i; + + memcpy(fit_scratch, fit_with_none_comp, sizeof(fit_scratch)); + + /* the "none" value appears once in the blob (not as a C string - + * scan raw bytes); the property length word sits 8 bytes before + * the data */ + p = NULL; + for (i = 0; i + 4 <= sizeof(fit_scratch); i++) { + if (memcmp(fit_scratch + i, "none", 4) == 0) { + p = fit_scratch + i; + break; + } + } + ck_assert_ptr_nonnull(p); + p[-8] = 0; + p[-7] = 0; + p[-6] = 0; + p[-5] = 4; + + ret = fit_load_image_to(fit_scratch, "kernel-1", + buf, (uint32_t)sizeof(buf), &len); + ck_assert_ptr_null(ret); +} +END_TEST + START_TEST(test_fit_to_lzma_unknown_returns_null) { /* Independent of WOLFBOOT_GZIP - any unknown compression scheme is @@ -320,6 +358,7 @@ static Suite *fit_gzip_suite(void) tcase_add_test(tc, test_fit_to_gzip_disabled_returns_null); #endif tcase_add_test(tc, test_fit_to_lzma_unknown_returns_null); + tcase_add_test(tc, test_fit_to_none_unterminated_fails_closed); tcase_add_test(tc, test_fit_to_none_oversized_rejected); suite_add_tcase(s, tc); From d554bbb98b127af168d452d709fec3b244f1f792 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 00:13:03 +0200 Subject: [PATCH 11/27] F-7066: bound the FIT DTS relocation copy to the staging size The FIT boot path relocated the flat-dt sub-image with a copy whose length came from the FIT-declared data property length, never bounded against the WOLFBOOT_LOAD_DTS_ADDRESS staging region - unlike the sibling DTB paths, which all validate the parsed size against WOLFBOOT_DTS_MIN_SIZE/WOLFBOOT_DTS_MAX_SIZE first. The length was also harvested through a (int*)&dts_size cast of a uint32_t. Relocate the parsed DTB size instead: validate it against the same MIN/MAX bounds as the other DTB sources and copy that many bytes. An out-of-range DTB is rejected (dts_addr stays NULL and the existing fallback chain applies) rather than partially or oversize copied. Applied to both call sites of the pattern: update_ram.c (memcpy) and update_disk.c (wolfBoot_fit_memcpy), which also gains the DTS bounds macros it was missing. unit-update-disk-fit (drives the real update_disk.c wolfBoot_start) gains two cases: a parsed size above WOLFBOOT_DTS_MAX_SIZE and one below WOLFBOOT_DTS_MIN_SIZE are both rejected without a copy, while the existing success/failure-copy cases keep passing. The staging stand-in is grown so the pre-fix unbounded copy is observable as a copy instead of a crash. --- src/update_disk.c | 24 ++++++++-- src/update_ram.c | 13 ++++-- tools/unit-tests/unit-update-disk-fit.c | 58 ++++++++++++++++++++++--- 3 files changed, 83 insertions(+), 12 deletions(-) diff --git a/src/update_disk.c b/src/update_disk.c index edc5799f49..e371925a15 100644 --- a/src/update_disk.c +++ b/src/update_disk.c @@ -249,6 +249,17 @@ static void disk_decrypted_header_clear(uint8_t *hdr) extern int wolfBoot_get_dts_size(void *dts_addr); +#if defined(MMU) || defined(WOLFBOOT_FDT) +/* Bounds for the attacker-influenced fdt_totalsize before relocating a DTB. + * MIN is the FDT v17 header size (also enforced by the signer): fdt_check_header + * validates magic/version but not totalsize, so a crafted header with a tiny + * totalsize must be rejected rather than loaded/forwarded as a partial tree. */ +#ifndef WOLFBOOT_DTS_MAX_SIZE +#define WOLFBOOT_DTS_MAX_SIZE (1024U * 1024U) +#endif +#define WOLFBOOT_DTS_MIN_SIZE (40U) +#endif + #if defined(WOLFBOOT_NO_LOAD_ADDRESS) || !defined(WOLFBOOT_LOAD_ADDRESS) /* from the linker, where wolfBoot ends */ extern uint8_t _end_wb[]; @@ -632,10 +643,17 @@ void RAMFUNCTION wolfBoot_start(void) } #endif if (flat_dt != NULL) { - uint8_t *dts_ptr = fit_load_image(fit, flat_dt, (int*)&dts_size); - if (dts_ptr != NULL && wolfBoot_get_dts_size(dts_ptr) >= 0) { - /* relocate to load DTS address */ + uint8_t *dts_ptr = fit_load_image(fit, flat_dt, NULL); + int parsed = (dts_ptr != NULL) + ? wolfBoot_get_dts_size(dts_ptr) : -1; + if (dts_ptr != NULL && + parsed >= (int)WOLFBOOT_DTS_MIN_SIZE && + (uint32_t)parsed <= WOLFBOOT_DTS_MAX_SIZE) { + /* Relocate to the load DTS address. The copy length is + * the parsed DTB size (bounded by the staging region), + * not the FIT-declared property length. */ dts_addr = (uint8_t*)WOLFBOOT_LOAD_DTS_ADDRESS; + dts_size = (uint32_t)parsed; wolfBoot_printf("Loading DTS: %p -> %p (%d bytes)\n", dts_ptr, dts_addr, dts_size); if (wolfBoot_fit_memcpy(dts_addr, dts_ptr, dts_size) != 0) { diff --git a/src/update_ram.c b/src/update_ram.c index 0dac446536..b632b16d16 100644 --- a/src/update_ram.c +++ b/src/update_ram.c @@ -655,10 +655,17 @@ void RAMFUNCTION wolfBoot_start(void) } #endif if (flat_dt != NULL) { - uint8_t *dts_ptr = fit_load_image(fit, flat_dt, (int*)&dts_size); - if (dts_ptr != NULL && wolfBoot_get_dts_size(dts_ptr) >= 0) { - /* relocate to load DTS address */ + uint8_t *dts_ptr = fit_load_image(fit, flat_dt, NULL); + int parsed = (dts_ptr != NULL) + ? wolfBoot_get_dts_size(dts_ptr) : -1; + if (dts_ptr != NULL && + parsed >= (int)WOLFBOOT_DTS_MIN_SIZE && + (uint32_t)parsed <= WOLFBOOT_DTS_MAX_SIZE) { + /* Relocate to the load DTS address. The copy length is + * the parsed DTB size (bounded by the staging region), + * not the FIT-declared property length. */ dts_addr = (uint8_t*)WOLFBOOT_LOAD_DTS_ADDRESS; + dts_size = (uint32_t)parsed; wolfBoot_printf("Loading DTS: %p -> %p (%d bytes)\n", dts_ptr, dts_addr, dts_size); memcpy(dts_addr, dts_ptr, dts_size); diff --git a/tools/unit-tests/unit-update-disk-fit.c b/tools/unit-tests/unit-update-disk-fit.c index 5799ada798..0b401d8da5 100644 --- a/tools/unit-tests/unit-update-disk-fit.c +++ b/tools/unit-tests/unit-update-disk-fit.c @@ -54,17 +54,26 @@ #include #define TEST_PAYLOAD_SIZE 64 -#define TEST_DTS_SIZE 32 +/* A plausible DTB size: at least WOLFBOOT_DTS_MIN_SIZE (the 40-byte + * FDT v17 header). */ +#define TEST_DTS_SIZE 48 +/* Staging-region stand-in. The oversized test relies on it being big + * enough that the pre-fix unbounded copy lands fully inside it, so the + * regression is observable as "a copy happened" rather than a crash. */ +#define TEST_DTS_STAGE_SIZE (2U * 1024U * 1024U) static uint8_t load_buffer[TEST_PAYLOAD_SIZE]; #define WOLFBOOT_LOAD_ADDRESS ((uintptr_t)load_buffer) -static uint8_t dts_buffer[TEST_DTS_SIZE]; +static uint8_t dts_buffer[TEST_DTS_STAGE_SIZE]; #define WOLFBOOT_LOAD_DTS_ADDRESS ((uintptr_t)dts_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 uint8_t fit_dts_image[TEST_DTS_SIZE]; +static uint8_t fit_dts_image[TEST_DTS_STAGE_SIZE]; +/* Parsed DTB size the wolfBoot_get_dts_size() stub reports, and (pre + * fix) the FIT-declared length the fit_load_image() stub returns. */ +static int mock_dts_size; static int mock_do_boot_called; static int mock_fit_memcpy_ret; static int mock_fit_memcpy_called; @@ -107,6 +116,7 @@ static void reset_mocks(void) build_image(part_a_image, 1, 0xA1); build_image(part_b_image, 2, 0xB2); memset(fit_dts_image, 0xDD, sizeof(fit_dts_image)); + mock_dts_size = TEST_DTS_SIZE; mock_do_boot_called = 0; mock_fit_memcpy_ret = 0; mock_fit_memcpy_called = 0; @@ -217,11 +227,12 @@ int wolfBoot_verify_authenticity(struct wolfBoot_image* img) } /* The loaded payload is treated as a FIT container, and the sub-image - * returned by fit_load_image() is a valid flat device tree. */ + * returned by fit_load_image() is a flat device tree whose parsed + * size is mock_dts_size. */ int wolfBoot_get_dts_size(void *dts_addr) { (void)dts_addr; - return TEST_DTS_SIZE; + return mock_dts_size; } /* Only reached through the fdt_version()/fdt_totalsize() trace macros here. */ @@ -251,7 +262,7 @@ void* fit_load_image(void* fdt, const char* image, int* lenp) (void)fdt; (void)image; if (lenp != NULL) - *lenp = TEST_DTS_SIZE; + *lenp = mock_dts_size; return fit_dts_image; } @@ -331,12 +342,47 @@ START_TEST(test_update_disk_fit_dts_copy_success_boots) } END_TEST +/* A parsed DTB larger than the staging bound (WOLFBOOT_DTS_MAX_SIZE) + * must be rejected rather than copied: before the fix the copy length + * came from the FIT-declared property length, unbounded against the + * staging region. */ +START_TEST(test_update_disk_fit_dts_oversized_rejected) +{ + reset_mocks(); + mock_dts_size = (1024 * 1024) + 4; /* > WOLFBOOT_DTS_MAX_SIZE */ + + wolfBoot_start(); + + ck_assert_int_eq(wolfBoot_panicked, 0); + ck_assert_int_eq(mock_do_boot_called, 1); + /* nothing may have been copied into the staging region */ + ck_assert_uint_eq(dts_buffer[0], 0); +} +END_TEST + +/* A parsed DTB smaller than the FDT header size is a partial tree and + * must be rejected. */ +START_TEST(test_update_disk_fit_dts_below_min_rejected) +{ + reset_mocks(); + mock_dts_size = 32; /* < WOLFBOOT_DTS_MIN_SIZE (40) */ + + wolfBoot_start(); + + ck_assert_int_eq(wolfBoot_panicked, 0); + ck_assert_int_eq(mock_do_boot_called, 1); + ck_assert_uint_eq(dts_buffer[0], 0); +} +END_TEST + Suite *wolfboot_suite(void) { Suite *s = suite_create("wolfBoot"); TCase *tc = tcase_create("update-disk-fit"); tcase_add_test(tc, test_update_disk_fit_dts_copy_failure_zeroizes_key_material); + tcase_add_test(tc, test_update_disk_fit_dts_oversized_rejected); + tcase_add_test(tc, test_update_disk_fit_dts_below_min_rejected); tcase_add_test(tc, test_update_disk_fit_dts_copy_success_boots); suite_add_tcase(s, tc); From f08c62a00035c70490ca70b1d2f5b930711c3378 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 01:12:03 +0200 Subject: [PATCH 12/27] F-9756: validate PT_LOAD segments before the scatter flash hash walk wolfBoot_check_flash_image_elf() fed every PT_LOAD entry's paddr/BASE_OFF straight into update_hash_flash_addr() with the 64-bit file_size truncated to the uint32_t the reader consumes, and never bounded an intermediate segment's file layout against the manifest. The read loop then memcpy's from (or drives the flash driver at) whatever address the image declares - an unauthenticated partition (e.g. WOLFBOOT_SKIP_BOOT_VERIFY builds) or a corrupt one could walk the hash over unmapped memory. Validate each loadable segment before hashing and fail the check instead of continuing: - file_size must fit the uint32_t hash length, - offset + file_size must stay inside the manifest image (overflow-safe comparison; previously only the last segment was checked, after the loop), - paddr + BASE_OFF + file_size must not overflow the address space. The mismatch log no longer prints the first 8 digest bytes. Note: a full flash-region bound for paddr needs a configured scatter-region size; no such knob exists in the target configuration today (scattered segments are deliberately placed outside the boot/update/swap partitions), so the region check is left as a follow-up. unit-image-elf-scatter gains three cases with a multi-segment fixture: a 2^32 file_size and a segment layout extending past fw_size (both verified OK pre-fix because the stored digest matched the truncated/out-of-layout walk) are now rejected with -1, and a paddr whose range overflows the address space is rejected before any flash read (pre-fix: read at 0xfffffffffffffffb, segfault on host). --- src/image.c | 38 +++- tools/unit-tests/unit-image-elf-scatter.c | 208 ++++++++++++++++++++++ 2 files changed, 237 insertions(+), 9 deletions(-) diff --git a/src/image.c b/src/image.c index 7741f50283..d8a648b71d 100644 --- a/src/image.c +++ b/src/image.c @@ -2242,7 +2242,35 @@ int wolfBoot_check_flash_image_elf(uint8_t part, unsigned long* entry_out) /* Handle loadable segments */ if (type == ELF_PT_LOAD) { - uintptr_t load_addr = (uintptr_t)(paddr + BASE_OFF); + uint64_t seg_start; + uintptr_t load_addr; + + /* Validate the segment before hashing: the flash-address hash + * reader consumes a uint32_t length, the file layout must stay + * inside the manifest image, and the paddr range must not + * overflow. Reject instead of continuing. */ + if (filesz > UINT32_MAX) { + wolfBoot_printf("ELF: [CHECK] 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: [CHECK] 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_MAX - filesz) { + wolfBoot_printf("ELF: [CHECK] ERROR: segment paddr range " + "overflows\n"); + return -1; + } + + load_addr = (uintptr_t)seg_start; /* Feed the loadable parts to the hash function */ wolfBoot_printf("ELF: [CHECK] Hashing loadable segment: " "paddr = 0x%08lx, loadaddr = 0x%08lx, " @@ -2315,14 +2343,6 @@ int wolfBoot_check_flash_image_elf(uint8_t part, unsigned long* entry_out) if (wolfBoot_hardened_CT_compare(exp_digest, calc_digest, WOLFBOOT_SHA_DIGEST_SIZE) != 0) { wolfBoot_printf("ELF: [CHECK] SHA verification FAILED\n"); - wolfBoot_printf( - "ELF: [CHECK] Expected %02x%02x%02x%02x%02x%02x%02x%02x\n", - exp_digest[0], exp_digest[1], exp_digest[2], exp_digest[3], - exp_digest[4], exp_digest[5], exp_digest[6], exp_digest[7]); - wolfBoot_printf( - "ELF: [CHECK] Calculated %02x%02x%02x%02x%02x%02x%02x%02x\n", - calc_digest[0], calc_digest[1], calc_digest[2], calc_digest[3], - calc_digest[4], calc_digest[5], calc_digest[6], calc_digest[7]); return -2; } wolfBoot_printf("ELF: [CHECK] Verification successful\n"); diff --git a/tools/unit-tests/unit-image-elf-scatter.c b/tools/unit-tests/unit-image-elf-scatter.c index ad3d1c23dc..060defcf4d 100644 --- a/tools/unit-tests/unit-image-elf-scatter.c +++ b/tools/unit-tests/unit-image-elf-scatter.c @@ -258,6 +258,74 @@ static void patch_expected_digest(const uint8_t *digest) memcpy(manifest + 12, digest, WOLFBOOT_SHA_DIGEST_SIZE); } +/* --- Multi-segment fixtures for the PT_LOAD bounds-rejection tests --- + * + * The manifest holds image header + ELF header + N program headers + * (tightly packed). Each segment's flash-resident payload lives in its + * own static array referenced by ph.paddr. */ +#define SEG1_SIZE 0x2000U +#define SEG2_SIZE 64U + +static uint8_t seg1_flash[SEG1_SIZE]; +static uint8_t seg2_flash[SEG2_SIZE]; + +struct seg_spec { + uint64_t offset; + uint64_t filesz; + uint64_t paddr; + uint8_t *payload; /* pattern-filled for fillsz bytes */ + uint32_t fillsz; +}; + +static void build_scattered_image_n(const struct seg_spec *segs, unsigned n, + uint32_t fw_size) +{ + uint8_t *manifest = (uint8_t *)(uintptr_t)MOCK_ADDRESS_BOOT; + uint32_t magic = WOLFBOOT_MAGIC; + size_t pht_sz = sizeof(elf64_header) + n * sizeof(elf64_program_header); + elf64_header *eh; + elf64_program_header *ph; + unsigned i, j; + + memset(manifest, 0, IMAGE_HEADER_SIZE + pht_sz); + + /* manifest header with a zeroed HDR_HASH TLV (patched by caller) */ + memcpy(manifest + 0, &magic, sizeof(magic)); + memcpy(manifest + 4, &fw_size, sizeof(fw_size)); + write_le16(manifest + 8, HDR_HASH); + write_le16(manifest + 10, WOLFBOOT_SHA_DIGEST_SIZE); + + eh = (elf64_header *)(manifest + IMAGE_HEADER_SIZE); + memcpy(eh->ident, ELF_IDENT_STR, 4); + eh->ident[ELF_CLASS_OFF] = ELF_CLASS_64; + eh->ident[5] = ELF_ENDIAN_LITTLE; + eh->type = ELF_HET_EXEC; + eh->machine = 0; + eh->version = 1; + eh->entry = 0x2000; + eh->ph_offset = sizeof(elf64_header); + eh->flags = 0; + eh->header_size = sizeof(elf64_header); + eh->ph_entry_size = sizeof(elf64_program_header); + eh->ph_entry_count = n; + + ph = (elf64_program_header *)((uint8_t *)eh + sizeof(elf64_header)); + for (i = 0; i < n; i++) { + memset(&ph[i], 0, sizeof(ph[i])); + ph[i].type = ELF_PT_LOAD; + ph[i].offset = segs[i].offset; + ph[i].paddr = segs[i].paddr; + ph[i].file_size = segs[i].filesz; + ph[i].mem_size = segs[i].filesz; + ph[i].align = 1; + for (j = 0; j < segs[i].fillsz; j++) { + segs[i].payload[j] = (uint8_t)(0x60U + i + j); + } + } +} + +#define ELF_HDR_SZ_2 (sizeof(elf64_header) + 2 * sizeof(elf64_program_header)) + static void map_boot_partition(void) { int ret = mmap_file("/tmp/wolfboot-unit-elf-scatter-boot.bin", @@ -330,12 +398,152 @@ START_TEST(test_elf_scatter_corrupted_segment_rejected) } END_TEST +/* A segment whose 64-bit file_size does not fit the uint32_t length the + * flash hash reader consumes must be rejected outright. Pre-fix, the + * size was silently truncated (2^32 -> 0 bytes hashed) and an image + * whose stored digest matched the truncated walk verified OK. */ +START_TEST(test_elf_scatter_filesz_over_32bit_rejected) +{ + uint8_t expected_digest[WOLFBOOT_SHA_DIGEST_SIZE]; + unsigned long entry = 0; + uint32_t fw_size = (uint32_t)ELF_HDR_SZ_2 + SEG2_SIZE; + struct seg_spec segs[2]; + struct wolfBoot_image boot; + wolfBoot_hash_t ctx; + int ret; + + map_boot_partition(); + + memset(seg1_flash, 0, sizeof(seg1_flash)); + memset(seg2_flash, 0, sizeof(seg2_flash)); + segs[0].offset = ELF_HDR_SZ_2 + 0x1000U; /* layout gap, never hashed */ + segs[0].filesz = 0x100000000ULL; /* 2^32: truncates to 0 */ + segs[0].paddr = (uint64_t)(uintptr_t)seg2_flash; /* 0 bytes read */ + segs[0].payload = seg2_flash; + segs[0].fillsz = 0; + segs[1].offset = ELF_HDR_SZ_2; + segs[1].filesz = SEG2_SIZE; + segs[1].paddr = (uint64_t)(uintptr_t)seg2_flash; + segs[1].payload = seg2_flash; + segs[1].fillsz = SEG2_SIZE; + + build_scattered_image_n(segs, 2, fw_size); + + /* Replay the pre-fix walk: the oversized segment contributes zero + * hashed bytes, only seg2 does. */ + ck_assert_int_eq(wolfBoot_open_image(&boot, PART_BOOT), 0); + ck_assert_int_eq(header_hash(&ctx, &boot), 0); + ck_assert_int_eq(update_hash_flash_fwimg(&ctx, &boot, 0, (uint32_t)ELF_HDR_SZ_2), 0); + ck_assert_int_eq( + update_hash_flash_addr(&ctx, (uintptr_t)seg2_flash, SEG2_SIZE, + PART_IS_EXT(&boot)), + 0); + ck_assert_int_eq(final_hash(&ctx, expected_digest), 0); + patch_expected_digest(expected_digest); + + ret = wolfBoot_check_flash_image_elf(PART_BOOT, &entry); + + /* Pre-fix this verified OK (ret 0) with the truncated size. */ + ck_assert_int_eq(ret, -1); + + unmap_boot_partition(); +} +END_TEST + +/* A segment whose file layout (offset + file_size) extends past the + * manifest image must be rejected. Pre-fix, intermediate segments were + * never bounds-checked (only the last one, after the loop) and an image + * whose stored digest matched the out-of-layout walk verified OK. */ +START_TEST(test_elf_scatter_segment_beyond_fw_size_rejected) +{ + uint8_t expected_digest[WOLFBOOT_SHA_DIGEST_SIZE]; + unsigned long entry = 0; + uint32_t fw_size = (uint32_t)ELF_HDR_SZ_2 + SEG2_SIZE; + struct seg_spec segs[2]; + struct wolfBoot_image boot; + wolfBoot_hash_t ctx; + int ret; + + map_boot_partition(); + + memset(seg1_flash, 0, sizeof(seg1_flash)); + memset(seg2_flash, 0, sizeof(seg2_flash)); + segs[0].offset = ELF_HDR_SZ_2; + segs[0].filesz = SEG1_SIZE; /* 0x2000 > the 64-byte layout slack */ + segs[0].paddr = (uint64_t)(uintptr_t)seg1_flash; + segs[0].payload = seg1_flash; + segs[0].fillsz = SEG1_SIZE; + segs[1].offset = ELF_HDR_SZ_2; + segs[1].filesz = SEG2_SIZE; + segs[1].paddr = (uint64_t)(uintptr_t)seg2_flash; + segs[1].payload = seg2_flash; + segs[1].fillsz = SEG2_SIZE; + + build_scattered_image_n(segs, 2, fw_size); + + /* Replay the pre-fix walk: both segments are hashed in full at their + * paddr locations despite seg0's layout extending past fw_size. */ + ck_assert_int_eq(wolfBoot_open_image(&boot, PART_BOOT), 0); + ck_assert_int_eq(header_hash(&ctx, &boot), 0); + ck_assert_int_eq(update_hash_flash_fwimg(&ctx, &boot, 0, (uint32_t)ELF_HDR_SZ_2), 0); + ck_assert_int_eq( + update_hash_flash_addr(&ctx, (uintptr_t)seg1_flash, SEG1_SIZE, + PART_IS_EXT(&boot)), + 0); + ck_assert_int_eq( + update_hash_flash_addr(&ctx, (uintptr_t)seg2_flash, SEG2_SIZE, + PART_IS_EXT(&boot)), + 0); + ck_assert_int_eq(final_hash(&ctx, expected_digest), 0); + patch_expected_digest(expected_digest); + + ret = wolfBoot_check_flash_image_elf(PART_BOOT, &entry); + + /* Pre-fix this verified OK (ret 0). */ + 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 read. Pre-fix this walked off into + * unmapped memory (segfault here; bus fault/hang on target). */ +START_TEST(test_elf_scatter_paddr_range_overflow_rejected) +{ + unsigned long entry = 0; + uint32_t fw_size = IMG_FW_SIZE; + 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, fw_size); + + ret = wolfBoot_check_flash_image_elf(PART_BOOT, &entry); + + 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"); 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); + tcase_add_test(tc, test_elf_scatter_segment_beyond_fw_size_rejected); + tcase_add_test(tc, test_elf_scatter_paddr_range_overflow_rejected); tcase_set_timeout(tc, 10); suite_add_tcase(s, tc); return s; From 8d39a5f9a460264b6103325e715f11693a762766 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 01:21:44 +0200 Subject: [PATCH 13/27] F-9721: make ext_flash_encrypt_write doc match its signature The Doxygen comment documented a 'forcedEnc' parameter that does not exist (the function takes address, data, len) and named AES for a routine whose encryption step is the configured cipher - ChaCha20, AES-CTR, or a PKCS#11-backed cipher, per build configuration. --- src/libwolfboot.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libwolfboot.c b/src/libwolfboot.c index ecd7eadfb6..33db33bf24 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -2621,13 +2621,13 @@ typedef char wolfBoot_encrypt_stage_size_check[ /** * @brief Write encrypted data to an external flash. * - * This function encrypts the provided data using the AES encryption algorithm - * and writes it to the external flash. + * This function encrypts the provided data using the configured external-flash + * encryption cipher (ChaCha20, AES-CTR, or a PKCS#11-backed cipher, per build + * configuration) and writes it to the external flash. * * @param address The address in the external flash to write the data to. * @param data Pointer to the data buffer to be written. * @param len The length of the data to be written. - * @param forcedEnc force writing encryption, used during final swap * * @return int 0 if successful, -1 on failure. */ From 58390cbf27738c2a36214dc322aed6e12285f3a6 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 01:23:39 +0200 Subject: [PATCH 14/27] F-7071: scrub the staging buffer on the swap resume early-return wolfBoot_swap_and_final_erase reads the staging-sector trailer into tmpBuffer (which also stages the firmware key/nonce under EXT_ENCRYPTED) and scrubs it on every exit except the resume early-return, which returned -1 with the buffer still holding the bytes just read from flash. Add the zeroize there so all four exits share the same invariant. --- src/update_flash.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/update_flash.c b/src/update_flash.c index ed9c4185a6..0068b53445 100644 --- a/src/update_flash.c +++ b/src/update_flash.c @@ -526,6 +526,8 @@ static int RAMFUNCTION wolfBoot_swap_and_final_erase(int resume) if ((resume == 1) && (swapDone == 0) && (updateState != IMG_STATE_FINAL_FLAGS) ) { + /* Keep the invariant that every exit scrubs the staging buffer */ + wolfBoot_zeroize(tmpBuffer, sizeof(tmpBuffer)); return -1; } From f3098cbf6df122ecaac691a60d299c48cd9f63d8 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 01:30:43 +0200 Subject: [PATCH 15/27] F-9766: scrub the nonce copies in the IV-derivation helpers wolfBoot_crypto_set_iv() copies the firmware encryption nonce onto the stack (local_nonce) for the AES/PKCS#11 backends and aes_set_iv() derives iv_buf from it, and both returned without scrubbing the copies. The rest of the codebase pairs key scrubs with nonce scrubs (e.g. update_disk.c); these helpers are called once per encrypted block, leaving a nonce copy on the stack at the end of every encrypted I/O sequence, including the one preceding do_boot(). ForceZero both buffers after the derived IV is consumed. The PKCS#11 set_iv helper is intentionally left as-is: it writes the counter into the persistent pkcs11_params CTR state, which the token updates in-place and which must survive the call. --- src/libwolfboot.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libwolfboot.c b/src/libwolfboot.c index 33db33bf24..d054e8a745 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -1950,6 +1950,7 @@ void RAMFUNCTION wolfBoot_crypto_set_iv(const uint8_t *nonce, uint32_t iv_counte uint8_t local_nonce[ENCRYPT_NONCE_SIZE]; XMEMCPY(local_nonce, nonce, ENCRYPT_NONCE_SIZE); crypto_set_iv(local_nonce, iv_counter + encrypt_iv_offset); + ForceZero(local_nonce, sizeof(local_nonce)); #else (void)nonce; (void)iv_counter; @@ -2328,6 +2329,7 @@ void aes_set_iv(uint8_t *nonce, uint32_t iv_ctr) #endif wc_AesSetIV(&aes_enc, (byte *)iv_buf); wc_AesSetIV(&aes_dec, (byte *)iv_buf); + ForceZero(iv_buf, sizeof(iv_buf)); } #elif defined(ENCRYPT_PKCS11) From 495b80b9b026cc71e8136da50738fd66c42dd65b Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 01:49:53 +0200 Subject: [PATCH 16/27] F-7966: propagate the CTR counter carry without branching on the nonce aes_set_iv() and pkcs11_crypto_set_iv() added the block counter to the nonce-derived counter block and propagated the carry with a loop that only runs - and whose trip count depends - on the nonce words: the overflow branch reveals that the high word was within one count of wrapping, and the inner loop's exit point reveals how many of the low words are 0xFFFFFFFF. crypto_set_iv() runs once per encrypted block, so a timing attacker gets one measurement per block. Replace both with an unconditional branch-free four-word carry (standard carry-out flags, three iterations regardless of content). Arithmetic is identical: verified old-vs-new expression equality over 5M random counter/nonce inputs plus the full-carry, zero-counter and max-counter boundary cases; unit-aes128/unit-aes256 encrypted roundtrips pass with the new code, and the ENCRYPT_PKCS11 CKM_AES_CTR path compiles clean. --- src/libwolfboot.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/libwolfboot.c b/src/libwolfboot.c index d054e8a745..9116f84a8e 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -2314,12 +2314,18 @@ void aes_set_iv(uint8_t *nonce, uint32_t iv_ctr) iv_buf[i] = wb_reverse_word32(iv_buf[i]); } #endif - iv_buf[3] += iv_ctr; - if (iv_buf[3] < iv_ctr) { /* overflow */ + /* Add the block counter with an unconditional, branch-free carry: + * a conditional carry loop's trip count would depend on the nonce + * content and leak it through timing. */ + { + uint32_t carry; + uint32_t old = iv_buf[3]; + iv_buf[3] = old + iv_ctr; + carry = (uint32_t)(iv_buf[3] < old); for (i = 2; i >= 0; i--) { - iv_buf[i]++; - if (iv_buf[i] != 0) - break; + uint32_t prev = iv_buf[i]; + iv_buf[i] = prev + carry; + carry = (uint32_t)(iv_buf[i] < prev); } } #ifndef BIG_ENDIAN_ORDER @@ -2474,12 +2480,16 @@ void pkcs11_crypto_set_iv(uint8_t *nonce, uint32_t iv_ctr) cb_words[i] = wb_reverse_word32(cb_words[i]); } #endif - cb_words[3] += iv_ctr; - if (cb_words[3] < iv_ctr) { /* overflow */ + /* Unconditional, branch-free carry (see aes_set_iv) */ + { + uint32_t carry; + uint32_t old = cb_words[3]; + cb_words[3] = old + iv_ctr; + carry = (uint32_t)(cb_words[3] < old); for (i = 2; i >= 0; i--) { - cb_words[i]++; - if (cb_words[i] != 0) - break; + uint32_t prev = cb_words[i]; + cb_words[i] = prev + carry; + carry = (uint32_t)(cb_words[i] < prev); } } #ifndef BIG_ENDIAN_ORDER From e9ee83796fc9c581408ba86dc5660e4651a64929 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 02:01:15 +0200 Subject: [PATCH 17/27] F-7396: scrub the decrypted-header cache after field extraction Under EXT_ENCRYPTED + MMU, decrypt_header() decrypts the firmware manifest into the file-scope dec_hdr buffer, which the blob-field lookups and wolfBoot_ram_decrypt() consume - but never clear, so a plaintext manifest of an image whose confidentiality is the point of EXT_ENCRYPTED sat in .bss through do_boot(). The disk-boot twin (update_disk.c) wipes its equivalent on every exit. Add dec_hdr_clear() and invoke it once the field of interest has been extracted: in wolfBoot_get_blob_version/type/diffbase_version (the tails now extract into a local and return it, identical values in all builds) and in wolfBoot_ram_decrypt right after the length field is taken - the only field read from the manifest, the copy loop that follows uses its own block buffer. --- src/libwolfboot.c | 49 +++++++++++++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/src/libwolfboot.c b/src/libwolfboot.c index 9116f84a8e..0a1988e305 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -1521,6 +1521,15 @@ static int decrypt_header(uint8_t *src) return 0; } +/* Scrub the decrypted manifest once the fields of interest have been + * extracted, so no plaintext header survives in .bss through the boot + * handoff (same treatment as disk_decrypted_header_clear in + * update_disk.c). */ +static void dec_hdr_clear(void) +{ + ForceZero(dec_hdr, IMAGE_HEADER_SIZE); +} + #endif /** * @brief Get blob version. @@ -1539,6 +1548,7 @@ uint32_t wolfBoot_get_blob_version(uint8_t *blob) uint32_t *volatile version_field = NULL; uint32_t *magic = NULL; uint8_t *img_bin = blob; + uint32_t version = 0; if (blob == NULL) return 0; #if defined(EXT_ENCRYPTED) && defined(MMU) @@ -1551,11 +1561,12 @@ uint32_t wolfBoot_get_blob_version(uint8_t *blob) if (*magic != WOLFBOOT_MAGIC) return 0; if (wolfBoot_find_header(img_bin + IMAGE_HEADER_OFFSET, HDR_VERSION, - (void *)&version_field) != sizeof(uint32_t)) - return 0; - if (version_field) - return im2n(*version_field); - return 0; + (void *)&version_field) == sizeof(uint32_t) && version_field) + version = im2n(*version_field); +#if defined(EXT_ENCRYPTED) && defined(MMU) + dec_hdr_clear(); +#endif + return version; } /** @@ -1574,6 +1585,7 @@ uint16_t wolfBoot_get_blob_type(uint8_t *blob) uint16_t *volatile type_field = NULL; uint32_t *magic = NULL; uint8_t *img_bin = blob; + uint16_t type = 0; #if defined(EXT_ENCRYPTED) && defined(MMU) if (wolfBoot_initialize_encryption() < 0) return 0; @@ -1584,12 +1596,12 @@ uint16_t wolfBoot_get_blob_type(uint8_t *blob) if (*magic != WOLFBOOT_MAGIC) return 0; if (wolfBoot_find_header(img_bin + IMAGE_HEADER_OFFSET, HDR_IMG_TYPE, - (void *)&type_field) != sizeof(uint16_t)) - return 0; - if (type_field) - return im2ns(*type_field); - - return 0; + (void *)&type_field) == sizeof(uint16_t) && type_field) + type = im2ns(*type_field); +#if defined(EXT_ENCRYPTED) && defined(MMU) + dec_hdr_clear(); +#endif + return type; } /** @@ -1611,6 +1623,7 @@ uint32_t wolfBoot_get_blob_diffbase_version(uint8_t *blob) uint32_t *volatile delta_base = NULL; uint32_t *magic = NULL; uint8_t *img_bin = blob; + uint32_t delta_base_ver = 0; #if defined(EXT_ENCRYPTED) && defined(MMU) if (wolfBoot_initialize_encryption() < 0) return 0; @@ -1621,11 +1634,12 @@ uint32_t wolfBoot_get_blob_diffbase_version(uint8_t *blob) if (*magic != WOLFBOOT_MAGIC) return 0; if (wolfBoot_find_header(img_bin + IMAGE_HEADER_OFFSET, HDR_IMG_DELTA_BASE, - (void *)&delta_base) != sizeof(uint32_t)) - return 0; - if (delta_base) - return im2n(*delta_base); - return 0; + (void *)&delta_base) == sizeof(uint32_t) && delta_base) + delta_base_ver = im2n(*delta_base); +#if defined(EXT_ENCRYPTED) && defined(MMU) + dec_hdr_clear(); +#endif + return delta_base_ver; } @@ -2912,6 +2926,9 @@ int wolfBoot_ram_decrypt(uint8_t *src, uint8_t *dst) * unaligned cast, then convert to native byte order. */ XMEMCPY(&len, dec_hdr + sizeof(uint32_t), sizeof(len)); len = im2n(len); + /* The length is the only field taken from the decrypted manifest; + * scrub it before it can outlive this function in .bss. */ + dec_hdr_clear(); #if !defined(WOLFBOOT_FIXED_PARTITIONS) && !defined(WOLFBOOT_RAMBOOT_MAX_SIZE) # error "WOLFBOOT_FIXED_PARTITIONS or WOLFBOOT_RAMBOOT_MAX_SIZE required to bound the RAM load" From 00bc8b58236d6c4a6ca96b429f74edaa4c1b1f4e Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 02:35:22 +0200 Subject: [PATCH 18/27] F-6762: compare the TLV field budget in a 32-bit domain wolfBoot_find_header() and the sign tool's re-parser checked each field's 4+len against (uint16_t)(header_size - IMAGE_HEADER_OFFSET). For any header of 64 KiB or more the cast wraps (0x10000 -> 0), so the guard rejects every field and an image the tool signs cannot be parsed by the bootloader - a pack/parse roundtrip break, fail-safe but fatal for large TLVs (post-quantum signatures, big cert chains). Compare in the uint32_t domain in both walkers. No shipped config reaches this size yet (largest example is 12288), so this pins the roundtrip for future large-header configs. unit-parser-large-header (new) builds the walker with IMAGE_HEADER_SIZE = 0x10008 - exactly the wrap boundary - and asserts a 300-byte TLV and a 4-byte version field are located (both were rejected pre-fix, proven against the pre-fix walker in a scratch build). --- src/libwolfboot.c | 7 +- tools/keytools/sign.c | 7 +- tools/unit-tests/Makefile | 7 +- tools/unit-tests/unit-parser-large-header.c | 186 ++++++++++++++++++++ 4 files changed, 202 insertions(+), 5 deletions(-) create mode 100644 tools/unit-tests/unit-parser-large-header.c diff --git a/src/libwolfboot.c b/src/libwolfboot.c index 0a1988e305..b6cf25f87b 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -1374,8 +1374,11 @@ uint16_t wolfBoot_find_header(uint8_t *haystack, uint16_t type, uint8_t **ptr) } len = p[2] | (p[3] << 8); - /* check len */ - if ((4U + len) > (uint16_t)(IMAGE_HEADER_SIZE - IMAGE_HEADER_OFFSET)) { + /* check len (compare in a 32-bit domain: a uint16_t cast of the + * header budget wraps for headers >= 64 KiB and rejects every + * field) */ + if ((uint32_t)(4U + len) > + (uint32_t)(IMAGE_HEADER_SIZE - IMAGE_HEADER_OFFSET)) { unit_dbg("This field is too large (bigger than the space available " "in the current header)\n"); unit_dbg("%u %u %u\n", (unsigned int)len, diff --git a/tools/keytools/sign.c b/tools/keytools/sign.c index 01cc242753..103fdd34cc 100644 --- a/tools/keytools/sign.c +++ b/tools/keytools/sign.c @@ -458,8 +458,11 @@ static uint16_t sign_tool_find_header(uint8_t *haystack, uint16_t type, uint8_t } len = p[2] | (p[3] << 8); - /* check len */ - if ((4 + len) > (uint16_t)(CMD.header_sz - IMAGE_HEADER_OFFSET)) { + /* check len (compare in a 32-bit domain: a uint16_t cast of the + * header budget wraps for headers >= 64 KiB and rejects every + * field) */ + if ((uint32_t)(4 + len) > + (uint32_t)(CMD.header_sz - IMAGE_HEADER_OFFSET)) { fprintf(stderr, "This field too large to fit into header " "(%d > %d)\n", (int)(4 + len), (int)(CMD.header_sz - IMAGE_HEADER_OFFSET)); diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 28f5284039..a408501f34 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -53,7 +53,8 @@ endif -TESTS:=unit-parser unit-fdt unit-extflash unit-string unit-spi-flash unit-aes128 \ +TESTS:=unit-parser unit-parser-large-header unit-fdt unit-extflash unit-string \ + unit-spi-flash unit-aes128 \ unit-uart-flash \ unit-aes256 unit-chacha20 unit-pci unit-mock-state unit-sectorflags \ unit-max-space \ @@ -195,6 +196,7 @@ unit-aes128:CFLAGS+=-DEXT_ENCRYPTED -DENCRYPT_WITH_AES128 unit-aes256:CFLAGS+=-DEXT_ENCRYPTED -DENCRYPT_WITH_AES256 unit-chacha20:CFLAGS+=-DEXT_ENCRYPTED -DENCRYPT_WITH_CHACHA unit-parser:CFLAGS+=-DNVM_FLASH_WRITEONCE +unit-parser-large-header:CFLAGS+=-DNVM_FLASH_WRITEONCE unit-fdt:CFLAGS+=-DWOLFBOOT_FDT unit-nvm:CFLAGS+=-DNVM_FLASH_WRITEONCE -DMOCK_PARTITIONS unit-nvm-flagshome:CFLAGS+=-DNVM_FLASH_WRITEONCE -DMOCK_PARTITIONS -DFLAGS_HOME @@ -303,6 +305,9 @@ unit-extflash.o: FORCE unit-parser: ../../include/target.h unit-parser.c gcc -o $@ $^ $(CFLAGS) $(LDFLAGS) +unit-parser-large-header: ../../include/target.h unit-parser-large-header.c + gcc -o $@ $^ $(CFLAGS) $(LDFLAGS) + unit-fdt: ../../include/target.h unit-fdt.c ../../src/fdt.c gcc -o $@ $^ $(CFLAGS) -ffunction-sections -fdata-sections $(LDFLAGS) \ -Wl,--gc-sections diff --git a/tools/unit-tests/unit-parser-large-header.c b/tools/unit-tests/unit-parser-large-header.c new file mode 100644 index 0000000000..7c98b8a4fa --- /dev/null +++ b/tools/unit-tests/unit-parser-large-header.c @@ -0,0 +1,186 @@ +/* unit-parser-large-header.c + * + * Unit test for wolfBoot_find_header() with a manifest header at or above + * the 64 KiB boundary: the per-field budget check must compare in a + * 32-bit domain, since a uint16_t cast of (IMAGE_HEADER_SIZE - + * IMAGE_HEADER_OFFSET) wraps for headers >= 64 KiB and rejects every + * field, breaking the pack/parse roundtrip for large signed TLVs. + * + * + * 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, see . + */ + +/* Must also define DEBUG_WOLFSSL in user_settings.h */ +#define WOLFBOOT_HASH_SHA256 +/* Exactly at the wrap boundary: the payload budget is 0x10000, which a + * uint16_t cast of the pre-fix check turned into 0 (rejecting everything). */ +#define IMAGE_HEADER_SIZE 0x10008 +#define WC_RSA_BLINDING +#define ECC_TIMING_RESISTANT +#include +/* Consume the unit target header up front so its include guard keeps + * libwolfboot.c's later include from running. image.h sanity-checks + * WOLFBOOT_SECTOR_SIZE > IMAGE_HEADER_SIZE, which the shared unit sector + * (0x400) cannot satisfy for a 0x10008 header; override it here. The + * parser under test does no flash I/O, so the sector size is otherwise + * unused. */ +#include "target.h" +#undef WOLFBOOT_SECTOR_SIZE +#define WOLFBOOT_SECTOR_SIZE 0x20000 +#include "libwolfboot.c" +#include +static int locked = 0; + +/* Mocks */ +void hal_init(void) +{ +} +int hal_flash_write(haladdr_t address, const uint8_t *data, int len) +{ + (void)address; + (void)data; + (void)len; + return 0; +} +int hal_flash_erase(haladdr_t address, int len) +{ + (void)address; + (void)len; + return 0; +} +void hal_flash_unlock(void) +{ + ck_assert_msg(locked, "Double unlock detected\n"); + locked--; +} +void hal_flash_lock(void) +{ + ck_assert_msg(!locked, "Double lock detected\n"); + locked++; +} + +void hal_prepare_boot(void) +{ +} +/* End Mocks */ + +/* A 300-byte TLV payload: comfortably inside the 0x10000 budget, but far + * beyond the 244 bytes the wrapped uint16 budget would have allowed for a + * slightly smaller oversized header. */ +#define BIG_TLV_LEN 300 + +static uint8_t big_hdr[IMAGE_HEADER_SIZE]; + +static void build_big_header(void) +{ + uint32_t magic = WOLFBOOT_MAGIC; + uint32_t fw_size = 0x100; + int i; + + memset(big_hdr, 0xFF, sizeof(big_hdr)); + memcpy(big_hdr + 0, &magic, sizeof(magic)); + memcpy(big_hdr + 4, &fw_size, sizeof(fw_size)); + + /* Single TLV at IMAGE_HEADER_OFFSET: HDR_VERSION with a 300-byte + * payload, followed by the end-of-options zero word. */ + big_hdr[8] = (uint8_t)(HDR_VERSION & 0xFF); + big_hdr[9] = (uint8_t)(HDR_VERSION >> 8); + big_hdr[10] = (uint8_t)(BIG_TLV_LEN & 0xFF); + big_hdr[11] = (uint8_t)(BIG_TLV_LEN >> 8); + for (i = 0; i < BIG_TLV_LEN; i++) + big_hdr[12 + i] = (uint8_t)(0xA0 + i); + big_hdr[12 + BIG_TLV_LEN] = 0x00; + big_hdr[13 + BIG_TLV_LEN] = 0x00; +} + +START_TEST (test_parser_large_header_finds_big_tlv) +{ + uint8_t *p; + int i; + + build_big_header(); + + /* The version field must be located despite the header being at the + * uint16 budget wrap boundary. */ + ck_assert_msg(wolfBoot_find_header(big_hdr + 8, HDR_VERSION, &p) == + BIG_TLV_LEN, + "Parser error: cannot locate version field in a >= 64 KiB header"); + + for (i = 0; i < BIG_TLV_LEN; i++) + ck_assert_msg(p[i] == (uint8_t)(0xA0 + i), + "Parser error: version payload does not match"); + + /* A non-existing field must still report not-found. */ + ck_assert_msg(wolfBoot_find_header(big_hdr + 8, HDR_SHA3_384, &p) == 0, + "Parser error: found a non-existing field"); +} +END_TEST + +START_TEST (test_parser_large_header_blobs) +{ + uint32_t ver; + + build_big_header(); + + /* The blob accessor must read through the large header as well. */ + ver = wolfBoot_get_blob_version(big_hdr); + ck_assert_uint_eq(ver, 0); /* 300-byte version is not a uint32: no field */ + + /* Rebuild with a 4-byte version payload: the accessor must return it. */ + big_hdr[10] = 4; + big_hdr[11] = 0; + big_hdr[12] = 0x0d; + big_hdr[13] = 0x0c; + big_hdr[14] = 0x0b; + big_hdr[15] = 0x0a; + ver = wolfBoot_get_blob_version(big_hdr); + ck_assert_uint_eq(ver, 0x0a0b0c0d); +} +END_TEST + +Suite *wolfboot_suite(void) +{ + + /* Suite initialization */ + Suite *s = suite_create("wolfBoot"); + + /* Test cases */ + TCase *parser_big = tcase_create("Parser large header"); + + /* Test function <-> Test case */ + tcase_add_test(parser_big, test_parser_large_header_finds_big_tlv); + tcase_add_test(parser_big, test_parser_large_header_blobs); + + /* Set parameters + add to suite */ + tcase_set_timeout(parser_big, 20); + suite_add_tcase(s, parser_big); + + 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 d64bf6715182e9c16f27764dd3a063078ac94d2c Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 02:40:12 +0200 Subject: [PATCH 19/27] F-9755: assert wolfBoot_success erases the firmware encryption key The EXT_ENCRYPTED wolfBoot_erase_encrypt_key() call at the tail of wolfBoot_success() - the only point in the normal update lifecycle that wipes the temporary firmware-decryption key/nonce from the boot-partition trailer - was never asserted: the default unit-update-flash build preprocessed it away (no EXT_ENCRYPTED), and the encrypted target (unit-update-flash-enc) ran only its fallback-only subset, so deleting the call would have survived the full suite. Give the CUSTOM_ENCRYPT_KEY mock a call counter, add test_boot_success_erases_encrypt_key asserting exactly one erase after confirmation, and register it in the UNIT_TEST_FALLBACK_ONLY branch so it runs under unit-update-flash-enc. --- tools/unit-tests/unit-update-flash.c | 32 ++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tools/unit-tests/unit-update-flash.c b/tools/unit-tests/unit-update-flash.c index 19e58805df..df11259000 100644 --- a/tools/unit-tests/unit-update-flash.c +++ b/tools/unit-tests/unit-update-flash.c @@ -133,6 +133,7 @@ int unit_test_wb_patch(WB_PATCH_CTX *ctx, uint8_t *dst, uint32_t len) static int mock_get_encrypt_key_ret = 0; static int mock_set_encrypt_key_ret = 0; static int mock_set_encrypt_key_calls = 0; +static int mock_erase_encrypt_key_calls = 0; int wolfBoot_get_encrypt_key(uint8_t *k, uint8_t *nonce) { @@ -158,6 +159,7 @@ int wolfBoot_set_encrypt_key(const uint8_t *key, const uint8_t *nonce) int wolfBoot_erase_encrypt_key(void) { + mock_erase_encrypt_key_calls++; return 0; } #endif @@ -183,6 +185,32 @@ START_TEST (test_boot_success_sets_state) END_TEST #endif +#ifdef CUSTOM_ENCRYPT_KEY +/* wolfBoot_success() must erase the temporary firmware-decryption key + * from the partition trailer as part of confirming an update. The mock + * records the call; without the erase the plaintext key would stay + * resident in flash. */ +START_TEST (test_boot_success_erases_encrypt_key) +{ + uint8_t state = 0; + + reset_mock_stats(); + prepare_flash(); + hal_flash_unlock(); + wolfBoot_set_partition_state(PART_BOOT, IMG_STATE_TESTING); + hal_flash_lock(); + + wolfBoot_success(); + + ck_assert_int_eq(wolfBoot_get_partition_state(PART_BOOT, &state), 0); + ck_assert_uint_eq(state, IMG_STATE_SUCCESS); + ck_assert_int_eq(mock_erase_encrypt_key_calls, 1); + + cleanup_flash(); +} +END_TEST +#endif + Suite *wolfboot_suite(void); int wolfBoot_staged_ok = 0; @@ -234,6 +262,7 @@ static void reset_mock_stats(void) mock_get_encrypt_key_ret = 0; mock_set_encrypt_key_ret = 0; mock_set_encrypt_key_calls = 0; + mock_erase_encrypt_key_calls = 0; #endif #ifndef ARCH_SIM wolfBoot_panicked = 0; @@ -1703,6 +1732,9 @@ Suite *wolfboot_suite(void) tcase_add_test(fallback_verify, test_fallback_image_verification_rejects_corruption); tcase_add_test(fallback_verify, test_final_swap_propagates_encrypt_key_read_failure); tcase_add_test(fallback_verify, test_final_swap_propagates_encrypt_key_persist_failure); +#ifdef CUSTOM_ENCRYPT_KEY + tcase_add_test(fallback_verify, test_boot_success_erases_encrypt_key); +#endif suite_add_tcase(s, fallback_verify); tcase_add_test(encrypt_write_bounds, test_encrypt_write_keeps_trailing_partial_block); From 5caac27eb9a4c1422d1d7dab71062924912b6e48 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 03:31:41 +0200 Subject: [PATCH 20/27] F-9750: decrypt the stored block before the encrypted RMW patch Both partial-block read-modify-write paths in ext_flash_encrypt_write read the stored block (ciphertext), spliced the new plaintext in, and re-encrypted the whole block. The untouched bytes were therefore XOR'd with the keystream a second time: stored ciphertext came back as plaintext in flash, and the next read of those bytes returned raw ciphertext instead of the original data. Any encrypted update whose first or last block partially overlaps a block with previous content - e.g. a retry over a previously written update image - silently corrupted the neighbouring bytes. Decrypt the stored block before splicing (into the scratch buffer, so no backend has to handle in-place decrypt) and re-encrypt the merged plaintext. Erased (0xFF) bytes round-trip unchanged because the decrypt/encrypt pair is the identity on the stored value. The re-encryption re-syncs the stream to the block index first: the decrypt step consumes keystream, and on the ChaCha/PKCS#11 backends encrypt and decrypt share a single stream state, while on the AES backends the decrypt context had not advanced with the full-block writes. The tail path also syncs the decrypt context, which on the AES backends sits at the first block's index after the aligned writes. Fallback-IV offset handling mirrors ext_flash_decrypt_read. New unit-extflash tests (run under the plain, AES-128, AES-256 and ChaCha20 variants): a mid-block patch must leave the untouched bytes of a previously written block intact, a trailing partial block must leave the rest of a previously written block intact, and a stream written in small unaligned chunks must round-trip byte for byte. All three fail on the pre-fix code with every cipher. --- src/libwolfboot.c | 27 ++++++++ tools/unit-tests/unit-extflash.c | 114 +++++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+) diff --git a/src/libwolfboot.c b/src/libwolfboot.c index b6cf25f87b..1020da6f70 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -2720,7 +2720,19 @@ int RAMFUNCTION ext_flash_encrypt_write(uintptr_t address, const uint8_t *data, != ENCRYPT_BLOCK_SIZE) { return -1; } + /* The stored block is ciphertext: decrypt it so the untouched bytes + * can be patched as plaintext and re-encrypted. Re-encrypting the + * ciphertext as-is would double-XOR the untouched bytes and store + * them in plaintext. */ + crypto_decrypt(enc_block, block, ENCRYPT_BLOCK_SIZE); + XMEMCPY(block, enc_block, ENCRYPT_BLOCK_SIZE); XMEMCPY(block + row_offset, data, step); + /* The decrypt above consumed keystream on backends that share one + * stream state between encrypt and decrypt; re-sync the stream to + * this block before re-encrypting. */ + if (fallback_iv_forced) + encrypt_iv_offset = FALLBACK_IV_OFFSET; + wolfBoot_crypto_set_iv(encrypt_iv_nonce, iv_counter); crypto_encrypt(enc_block, block, ENCRYPT_BLOCK_SIZE); ret = ext_flash_write(row_address, enc_block, ENCRYPT_BLOCK_SIZE); if (ret < 0) @@ -2758,11 +2770,26 @@ int RAMFUNCTION ext_flash_encrypt_write(uintptr_t address, const uint8_t *data, * the same way the unaligned head above is handled. */ step = sz & (ENCRYPT_BLOCK_SIZE - 1); if (step > 0) { + /* "address" is block-aligned here; index of the block being patched */ + uint32_t tail_iv_counter = (address - WOLFBOOT_PARTITION_UPDATE_ADDRESS) / + ENCRYPT_BLOCK_SIZE; if (ext_flash_read(address, block, ENCRYPT_BLOCK_SIZE) != ENCRYPT_BLOCK_SIZE) { return -1; } + /* Sync the decrypt context to this block (on backends with separate + * encrypt/decrypt contexts it did not advance with the full-block + * writes above), then decrypt the stored ciphertext so the untouched + * tail bytes survive the re-encryption. */ + if (fallback_iv_forced) + encrypt_iv_offset = FALLBACK_IV_OFFSET; + wolfBoot_crypto_set_iv(encrypt_iv_nonce, tail_iv_counter); + crypto_decrypt(enc_block, block, ENCRYPT_BLOCK_SIZE); + XMEMCPY(block, enc_block, ENCRYPT_BLOCK_SIZE); XMEMCPY(block, data, step); + if (fallback_iv_forced) + encrypt_iv_offset = FALLBACK_IV_OFFSET; + wolfBoot_crypto_set_iv(encrypt_iv_nonce, tail_iv_counter); crypto_encrypt(enc_block, block, ENCRYPT_BLOCK_SIZE); ret = ext_flash_write(address, enc_block, ENCRYPT_BLOCK_SIZE); } diff --git a/tools/unit-tests/unit-extflash.c b/tools/unit-tests/unit-extflash.c index 3fdf22d20b..dbcd46b895 100644 --- a/tools/unit-tests/unit-extflash.c +++ b/tools/unit-tests/unit-extflash.c @@ -373,6 +373,111 @@ START_TEST(test_ext_enc_flash_oversized_write) { } END_TEST +/* A mid-block patch must not destroy the untouched bytes of a block that + * already holds encrypted content: the read-modify-write has to decrypt the + * stored block, splice the patch in, and re-encrypt. Re-encrypting the + * ciphertext as-is double-XORs the untouched bytes and stores plaintext, + * which then reads back as raw ciphertext. */ +START_TEST(test_ext_enc_flash_rmw_head_preserves_neighbors) { + uint32_t address = 0x1000; + uint8_t block_a[TEST_BLOCK_SIZE]; + uint8_t block_p[TEST_BLOCK_SIZE]; + uint8_t expect[TEST_BLOCK_SIZE]; + uint8_t data[TEST_BLOCK_SIZE]; + const int off = 4, len = 8; + int i, rres, wres; + + for (i = 0; i < TEST_BLOCK_SIZE; i++) { + block_a[i] = (uint8_t)(0xA0 + i); + block_p[i] = (uint8_t)(0xB0 + i); + } + + /* Prime the block with known content */ + wres = ext_flash_check_write(address, block_a, TEST_BLOCK_SIZE); + ck_assert_int_eq(wres, 0); + + /* Patch an unaligned mid-block subrange */ + wres = ext_flash_check_write(address + off, block_p, len); + ck_assert_int_eq(wres, 0); + + memcpy(expect, block_a, TEST_BLOCK_SIZE); + memcpy(expect + off, block_p, len); + rres = ext_flash_check_read(address, data, TEST_BLOCK_SIZE); + ck_assert_int_eq(rres, TEST_BLOCK_SIZE); + ck_assert_mem_eq(data, expect, TEST_BLOCK_SIZE); +} +END_TEST + +/* A write whose trailing partial block lands on a block that already holds + * encrypted content: the untouched tail of that block must survive. */ +START_TEST(test_ext_enc_flash_rmw_tail_preserves_neighbors) { + uint32_t address = 0x1000; + uint8_t block_b[TEST_BLOCK_SIZE]; + uint8_t payload[2 * TEST_BLOCK_SIZE]; + uint8_t expect[2 * TEST_BLOCK_SIZE]; + uint8_t data[2 * TEST_BLOCK_SIZE]; + const int tail = 8; + int i, rres, wres; + + for (i = 0; i < TEST_BLOCK_SIZE; i++) { + block_b[i] = (uint8_t)(0xC0 + i); + payload[i] = (uint8_t)(0xD0 + i); + payload[TEST_BLOCK_SIZE + i] = (uint8_t)(0xE0 + i); + } + + /* Two primed blocks */ + wres = ext_flash_check_write(address, payload, TEST_BLOCK_SIZE); + ck_assert_int_eq(wres, 0); + wres = ext_flash_check_write(address + TEST_BLOCK_SIZE, block_b, + TEST_BLOCK_SIZE); + ck_assert_int_eq(wres, 0); + + /* Aligned write of one full block plus a partial tail into block two */ + wres = ext_flash_check_write(address, payload, TEST_BLOCK_SIZE + tail); + ck_assert_int_eq(wres, 0); + + /* Block one is fully replaced; block two holds the spliced tail and the + * original bytes beyond it. */ + memcpy(expect, payload, TEST_BLOCK_SIZE + tail); + memcpy(expect + TEST_BLOCK_SIZE + tail, block_b + tail, + TEST_BLOCK_SIZE - tail); + rres = ext_flash_check_read(address, data, 2 * TEST_BLOCK_SIZE); + ck_assert_int_eq(rres, 2 * TEST_BLOCK_SIZE); + ck_assert_mem_eq(data, expect, 2 * TEST_BLOCK_SIZE); +} +END_TEST + +/* A stream written in small, unaligned chunks must round-trip: every chunk + * boundary exercises the partial-block read-modify-write against content the + * previous chunks already encrypted. */ +START_TEST(test_ext_enc_flash_chunked_stream_roundtrip) { + uint32_t address = 0x1000; + const uint32_t total = 3 * TEST_BLOCK_SIZE + 7; + static uint8_t payload[3 * TEST_BLOCK_SIZE + 7]; + static uint8_t data[3 * TEST_BLOCK_SIZE + 7]; + uint32_t written = 0; + int i, rres, wres; + + for (i = 0; i < (int)total; i++) + payload[i] = (uint8_t)(i * 7 + 3); + + while (written < total) { + uint32_t chunk = total - written; + if (chunk > 13) + chunk = 13; + wres = ext_flash_check_write(address + written, payload + written, + chunk); + ck_assert_int_eq(wres, 0); + written += chunk; + } + + memset(data, 0xA5, sizeof(data)); + rres = ext_flash_check_read(address, data, total); + ck_assert_int_eq(rres, (int)total); + ck_assert_mem_eq(data, payload, total); +} +END_TEST + Suite *wolfboot_suite(void) { @@ -386,6 +491,7 @@ Suite *wolfboot_suite(void) TCase *ext_enc_flash_short_read = tcase_create("External encrypted flash short unaligned read"); TCase *ext_enc_flash_short_write = tcase_create("External encrypted flash short unaligned write"); TCase *ext_enc_flash_oversized_write = tcase_create("External encrypted flash oversized write"); + TCase *ext_enc_flash_rmw = tcase_create("External encrypted flash RMW neighbour preservation"); /* Set parameters + add to suite */ tcase_add_test(ext_flash_operations, test_ext_flash_operations); @@ -396,17 +502,25 @@ Suite *wolfboot_suite(void) test_ext_enc_flash_short_unaligned_write); tcase_add_test(ext_enc_flash_oversized_write, test_ext_enc_flash_oversized_write); + tcase_add_test(ext_enc_flash_rmw, + test_ext_enc_flash_rmw_head_preserves_neighbors); + tcase_add_test(ext_enc_flash_rmw, + test_ext_enc_flash_rmw_tail_preserves_neighbors); + tcase_add_test(ext_enc_flash_rmw, + test_ext_enc_flash_chunked_stream_roundtrip); tcase_set_timeout(ext_flash_operations, 20); tcase_set_timeout(ext_enc_flash_operations, 20); tcase_set_timeout(ext_enc_flash_short_read, 20); tcase_set_timeout(ext_enc_flash_short_write, 20); tcase_set_timeout(ext_enc_flash_oversized_write, 20); + tcase_set_timeout(ext_enc_flash_rmw, 20); suite_add_tcase(s, ext_flash_operations); suite_add_tcase(s, ext_enc_flash_operations); suite_add_tcase(s, ext_enc_flash_short_read); suite_add_tcase(s, ext_enc_flash_short_write); suite_add_tcase(s, ext_enc_flash_oversized_write); + suite_add_tcase(s, ext_enc_flash_rmw); return s; } From 04531cbf2d1c05a19dd142316d801433042ffae3 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 05:53:46 +0200 Subject: [PATCH 21/27] F-9751: Add positive E2E encrypted-update test, fix what it exposes Add unit-update-flash-enc-full, the full end-to-end suite (forward updates, rollback, empty boot, diffbase) against the encrypted swap, plus a byte-for-byte fallback-IV roundtrip test. The suite exposes two product defects: - ext_flash_encrypt_write() partial-block re-syncs re-anchored the keystream at the standard-IV position once the one-shot fallback IV offset had been consumed by the initial set_iv, corrupting the tail of fallback-IV images. Capture the IV offset in effect at entry and re-apply it on every re-sync. - wolfBoot_final_swap() called wolfBoot_set_encrypt_key() with the internal flash unlocked, but the backend expects the flash locked (it manages the unlock/lock around the key write itself) and ends with the flash locked. Lock before the call and drop the now redundant lock on the failure path. Test plumbing for the encrypted target: update-partition writes in the tests now go through the encryption-aware writer, as the update tool does; the hand-rolled TLV headers use the sign tool's dense layout (padding gaps are ciphertext in encrypted builds); and the testing-flag sites anchor on the state trailer, which sits ahead of the key/nonce region in encrypted builds. Verified: unit-update-flash-enc-full 35/35, unit-update-flash-enc 8/8, unit-extflash + AES128/256/ChaCha20 variants 8/8 each, full unit suite green, stm32wb + AES256 cross-build green. --- src/libwolfboot.c | 14 ++- src/update_flash.c | 8 +- tools/unit-tests/Makefile | 16 ++- tools/unit-tests/unit-update-flash.c | 164 ++++++++++++++++++++++----- 4 files changed, 164 insertions(+), 38 deletions(-) diff --git a/src/libwolfboot.c b/src/libwolfboot.c index 1020da6f70..75811b316c 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -2669,6 +2669,10 @@ int RAMFUNCTION ext_flash_encrypt_write(uintptr_t address, const uint8_t *data, int sz = len, i, step, ret; uint8_t part; uint32_t iv_counter = 0; + /* The one-shot IV offset (fallback IV) is consumed by the set_iv below; + * the partial-block re-syncs further down must re-apply the same offset + * or they would re-anchor the stream at the standard-IV position. */ + uint32_t iv_offset_at_entry = 0; #if defined(EXT_ENCRYPTED) && !defined(WOLFBOOT_SMALL_STACK) && \ !defined(NVM_FLASH_WRITEONCE) uint8_t ENCRYPT_CACHE[NVM_CACHE_SIZE] XALIGNED_STACK(32); @@ -2701,6 +2705,7 @@ int RAMFUNCTION ext_flash_encrypt_write(uintptr_t address, const uint8_t *data, } if (wolfBoot_initialize_encryption() < 0) return -1; + iv_offset_at_entry = encrypt_iv_offset; wolfBoot_crypto_set_iv(encrypt_iv_nonce, iv_counter); break; case PART_SWAP: @@ -2730,8 +2735,7 @@ int RAMFUNCTION ext_flash_encrypt_write(uintptr_t address, const uint8_t *data, /* The decrypt above consumed keystream on backends that share one * stream state between encrypt and decrypt; re-sync the stream to * this block before re-encrypting. */ - if (fallback_iv_forced) - encrypt_iv_offset = FALLBACK_IV_OFFSET; + encrypt_iv_offset = iv_offset_at_entry; wolfBoot_crypto_set_iv(encrypt_iv_nonce, iv_counter); crypto_encrypt(enc_block, block, ENCRYPT_BLOCK_SIZE); ret = ext_flash_write(row_address, enc_block, ENCRYPT_BLOCK_SIZE); @@ -2781,14 +2785,12 @@ int RAMFUNCTION ext_flash_encrypt_write(uintptr_t address, const uint8_t *data, * encrypt/decrypt contexts it did not advance with the full-block * writes above), then decrypt the stored ciphertext so the untouched * tail bytes survive the re-encryption. */ - if (fallback_iv_forced) - encrypt_iv_offset = FALLBACK_IV_OFFSET; + encrypt_iv_offset = iv_offset_at_entry; wolfBoot_crypto_set_iv(encrypt_iv_nonce, tail_iv_counter); crypto_decrypt(enc_block, block, ENCRYPT_BLOCK_SIZE); XMEMCPY(block, enc_block, ENCRYPT_BLOCK_SIZE); XMEMCPY(block, data, step); - if (fallback_iv_forced) - encrypt_iv_offset = FALLBACK_IV_OFFSET; + encrypt_iv_offset = iv_offset_at_entry; wolfBoot_crypto_set_iv(encrypt_iv_nonce, tail_iv_counter); crypto_encrypt(enc_block, block, ENCRYPT_BLOCK_SIZE); ret = ext_flash_write(address, enc_block, ENCRYPT_BLOCK_SIZE); diff --git a/src/update_flash.c b/src/update_flash.c index 0068b53445..c2a27e48c8 100644 --- a/src/update_flash.c +++ b/src/update_flash.c @@ -571,18 +571,20 @@ static int RAMFUNCTION wolfBoot_swap_and_final_erase(int resume) wb_flash_erase(boot, WOLFBOOT_PARTITION_SIZE - eraseLen, eraseLen); #ifdef EXT_ENCRYPTED - /* Initialize encryption with the saved key */ + /* Initialize encryption with the saved key. The default backend + * manages the internal flash lock itself around the key write (it ends + * with the flash locked), so call it with the flash locked and re-unlock + * afterwards for the remaining writes. */ + hal_flash_lock(); ret = wolfBoot_set_encrypt_key((uint8_t*)tmpBuffer, (uint8_t*)&tmpBuffer[ENCRYPT_KEY_SIZE / sizeof(uint32_t)]); if (ret != 0) { #ifdef EXT_FLASH ext_flash_lock(); #endif - hal_flash_lock(); wolfBoot_zeroize(tmpBuffer, sizeof(tmpBuffer)); return ret; } - /* wolfBoot_set_encrypt_key calls hal_flash_unlock, need to unlock again */ hal_flash_unlock(); #endif /* Restore the original contents of the staging sector (with the magic trailer if encrypted) */ diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index a408501f34..14b86a6477 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -62,7 +62,7 @@ TESTS:=unit-parser unit-parser-large-header unit-fdt unit-extflash unit-string \ unit-enc-nvm-flagshome unit-delta unit-gzip unit-update-flash unit-update-flash-delta \ unit-update-flash-hook \ unit-update-flash-self-update \ - unit-update-flash-enc 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-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-image-nopart unit-image-sha384 unit-image-sha3-384 unit-image-dts \ unit-image-dts-sha384 unit-image-dts-sha3-384 unit-store-sbrk \ @@ -712,6 +712,20 @@ unit-update-flash-enc: ../../include/target.h unit-update-flash.c $(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/chacha.c \ $(CFLAGS) $(LDFLAGS) +# Same encrypted target without UNIT_TEST_FALLBACK_ONLY, so the full +# end-to-end suite (forward updates, rollback, ...) runs against the +# encrypted swap and its IV derivation. +unit-update-flash-enc-full: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 \ + -DCUSTOM_ENCRYPT_KEY \ + -DWOLFBOOT_ORIGIN=MOCK_ADDRESS_BOOT -DBOOTLOADER_PARTITION_SIZE=WOLFBOOT_PARTITION_SIZE +unit-update-flash-enc-full: ../../include/target.h unit-update-flash.c + gcc -o $@ unit-update-flash.c ../../src/image.c \ + $(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/sha256.c \ + $(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/chacha.c \ + $(CFLAGS) $(LDFLAGS) + unit-update-ram: ../../include/target.h unit-update-ram.c gcc -o $@ unit-update-ram.c ../../src/image.c $(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/sha256.c $(CFLAGS) $(LDFLAGS) diff --git a/tools/unit-tests/unit-update-flash.c b/tools/unit-tests/unit-update-flash.c index df11259000..dad19d97cd 100644 --- a/tools/unit-tests/unit-update-flash.c +++ b/tools/unit-tests/unit-update-flash.c @@ -85,12 +85,21 @@ static uint16_t host_to_img_u16(uint16_t val) #endif } +/* The update partition is written by the update tool through the + * encryption-aware writer: in EXT_ENCRYPTED builds a raw write would leave + * plaintext in flash that the bootloader then decrypts. The swap partition + * is written raw, since its content is already encrypted when staged. */ +static int update_part_write(uintptr_t addr, const void *buf, int len) +{ + return ext_flash_check_write(addr, buf, len); +} + static void ext_flash_write_le16(uintptr_t addr, uint16_t val) { uint8_t le[2]; le[0] = (uint8_t)(val & 0xFFu); le[1] = (uint8_t)((val >> 8) & 0xFFu); - ext_flash_write(addr, le, sizeof(le)); + update_part_write(addr, le, sizeof(le)); } static void ext_flash_write_le32(uintptr_t addr, uint32_t val) @@ -100,7 +109,7 @@ static void ext_flash_write_le32(uintptr_t addr, uint32_t val) le[1] = (uint8_t)((val >> 8) & 0xFFu); le[2] = (uint8_t)((val >> 16) & 0xFFu); le[3] = (uint8_t)((val >> 24) & 0xFFu); - ext_flash_write(addr, le, sizeof(le)); + update_part_write(addr, le, sizeof(le)); } #ifdef DELTA_UPDATES @@ -328,6 +337,11 @@ static void cleanup_flash(void) #define DIGEST_TLV_OFF_IN_HDR 28 +#ifdef EXT_ENCRYPTED +static int add_payload_encrypted(uint8_t part, uint32_t version, uint32_t size, + int use_fallback_iv); +#endif + static int add_payload(uint8_t part, uint32_t version, uint32_t size) { return add_payload_type(part, version, size, @@ -337,6 +351,15 @@ static int add_payload(uint8_t part, uint32_t version, uint32_t size) static int add_payload_type(uint8_t part, uint32_t version, uint32_t size, uint16_t img_type) { +#ifdef EXT_ENCRYPTED + /* The update partition holds ciphertext in EXT_ENCRYPTED builds (the + * update tool writes through the encryption-aware writer); build the + * plaintext image and encrypt it in. add_payload_encrypted builds the + * AUTH_NONE|APP image type, which is what every PART_UPDATE caller + * compiled in the encrypted targets uses. */ + if (part == PART_UPDATE) + return add_payload_encrypted(part, version, size, 0); +#endif uint32_t word; uint32_t magic = WOLFBOOT_MAGIC; uint32_t size_img = host_to_img_u32(size); @@ -474,7 +497,7 @@ START_TEST (test_self_update_newversion_invalid_integrity_denied) HDR_IMG_TYPE_WOLFBOOT | HDR_IMG_TYPE_AUTH); memset(bad_digest, 0xBA, sizeof(bad_digest)); ext_flash_unlock(); - ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + DIGEST_TLV_OFF_IN_HDR + 4, + update_part_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + DIGEST_TLV_OFF_IN_HDR + 4, bad_digest, sizeof(bad_digest)); wolfBoot_set_partition_state(PART_UPDATE, IMG_STATE_UPDATING); ext_flash_lock(); @@ -788,6 +811,59 @@ START_TEST (test_fallback_image_verification_rejects_corruption) } END_TEST +/* Positive counterpart of the corruption test above: an image written with + * the fallback IV must read back byte-for-byte through the product's + * decryption path with the fallback IV forced, the way the update flow does + * when the standard-IV open fails. */ +START_TEST (test_fallback_iv_image_roundtrips) +{ + uint32_t size = TEST_SIZE_SMALL; + uint32_t total = size + IMAGE_HEADER_SIZE; + uint8_t *plain = malloc(total); + uint8_t *readback = malloc(total); + struct wolfBoot_image img; + int prev, ret; + uint32_t i; + + reset_mock_stats(); + prepare_flash(); + ck_assert(plain != NULL && readback != NULL); + + /* build_image_buffer is deterministic (srandom(part)), so this is the + * same plaintext add_payload_encrypted writes with the fallback IV */ + ret = build_image_buffer(PART_UPDATE, 2, size, plain, total); + ck_assert_int_eq(ret, 0); + ret = add_payload_encrypted(PART_UPDATE, 2, size, 1); + ck_assert_int_eq(ret, 0); + + /* The update flow verifies and reads a fallback image with the fallback + * IV forced for the whole operation (persistent flag, re-applied by the + * decrypt path on every block). */ + prev = wolfBoot_force_fallback_iv(1); + ret = wolfBoot_open_image(&img, PART_UPDATE); + ck_assert_int_eq(ret, 0); + ck_assert_uint_eq(img.fw_size, size); + for (i = 0; i < total; i += WOLFBOOT_SECTOR_SIZE) { + uint32_t chunk = total - i; + if (chunk > WOLFBOOT_SECTOR_SIZE) + chunk = WOLFBOOT_SECTOR_SIZE; + ret = ext_flash_check_read( + (uintptr_t)WOLFBOOT_PARTITION_UPDATE_ADDRESS + i, + readback + i, (int)chunk); + ck_assert_int_eq(ret, (int)chunk); + } + wolfBoot_enable_fallback_iv(prev); + + i = 0; + while (i < total && readback[i] == plain[i]) + i++; + ck_assert_mem_eq(readback, plain, total); + free(plain); + free(readback); + cleanup_flash(); +} +END_TEST + START_TEST (test_final_swap_propagates_encrypt_key_persist_failure) { int ret; @@ -990,7 +1066,7 @@ START_TEST (test_invalid_update_type) { add_payload(PART_BOOT, 1, TEST_SIZE_SMALL); add_payload(PART_UPDATE, 2, TEST_SIZE_SMALL); ext_flash_unlock(); - ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 20, (void *)&word16, 2); + update_part_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 20, (void *)&word16, 2); ext_flash_lock(); wolfBoot_update_trigger(); wolfBoot_start(); @@ -1007,7 +1083,7 @@ START_TEST (test_invalid_update_auth_type) { add_payload(PART_BOOT, 1, TEST_SIZE_SMALL); add_payload(PART_UPDATE, 2, TEST_SIZE_SMALL); ext_flash_unlock(); - ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 20, (void *)&word16, 2); + update_part_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 20, (void *)&word16, 2); ext_flash_lock(); wolfBoot_update_trigger(); wolfBoot_start(); @@ -1025,7 +1101,7 @@ START_TEST (test_update_toolarge) { add_payload(PART_UPDATE, 2, TEST_SIZE_LARGE); /* Change the size in the header to be larger than the actual size */ ext_flash_unlock(); - ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 4, (void *)&very_large, 4); + update_part_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 4, (void *)&very_large, 4); ext_flash_lock(); wolfBoot_update_trigger(); @@ -1095,7 +1171,7 @@ START_TEST (test_invalid_sha) { memset(bad_digest, 0xBA, SHA256_DIGEST_SIZE); ext_flash_unlock(); - ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + DIGEST_TLV_OFF_IN_HDR + 4, bad_digest, SHA256_DIGEST_SIZE); + update_part_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + DIGEST_TLV_OFF_IN_HDR + 4, bad_digest, SHA256_DIGEST_SIZE); ext_flash_lock(); wolfBoot_update_trigger(); wolfBoot_start(); @@ -1114,8 +1190,9 @@ START_TEST (test_emergency_rollback) { add_payload(PART_UPDATE, 1, TEST_SIZE_SMALL); /* Set the testing flag in the last five bytes of the BOOT partition */ hal_flash_unlock(); - hal_flash_write(WOLFBOOT_PARTITION_BOOT_ADDRESS + WOLFBOOT_PARTITION_SIZE - 5, - testing_flags, 5); + /* PART_BOOT_ENDFLAGS, not the partition end: in EXT_ENCRYPTED builds + * the key/nonce trailer sits between the state trailer and the end. */ + hal_flash_write(PART_BOOT_ENDFLAGS - 5, testing_flags, 5); hal_flash_lock(); wolfBoot_start(); @@ -1136,8 +1213,9 @@ START_TEST (test_emergency_rollback_equal_versions) { add_payload(PART_UPDATE, 1, TEST_SIZE_SMALL); /* Set the testing flag in the last five bytes of the BOOT partition */ hal_flash_unlock(); - hal_flash_write(WOLFBOOT_PARTITION_BOOT_ADDRESS + WOLFBOOT_PARTITION_SIZE - 5, - testing_flags, 5); + /* PART_BOOT_ENDFLAGS, not the partition end: in EXT_ENCRYPTED builds + * the key/nonce trailer sits between the state trailer and the end. */ + hal_flash_write(PART_BOOT_ENDFLAGS - 5, testing_flags, 5); hal_flash_lock(); wolfBoot_start(); @@ -1158,13 +1236,14 @@ START_TEST (test_emergency_rollback_failure_due_to_bad_update) { add_payload(PART_UPDATE, 1, TEST_SIZE_SMALL); /* Set the testing flag in the last five bytes of the BOOT partition */ hal_flash_unlock(); - hal_flash_write(WOLFBOOT_PARTITION_BOOT_ADDRESS + WOLFBOOT_PARTITION_SIZE - 5, - testing_flags, 5); + /* PART_BOOT_ENDFLAGS, not the partition end: in EXT_ENCRYPTED builds + * the key/nonce trailer sits between the state trailer and the end. */ + hal_flash_write(PART_BOOT_ENDFLAGS - 5, testing_flags, 5); hal_flash_lock(); /* Corrupt the update */ ext_flash_unlock(); - ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS, wrong_update_magic, 4); + update_part_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS, wrong_update_magic, 4); ext_flash_lock(); wolfBoot_start(); @@ -1192,7 +1271,7 @@ START_TEST (test_empty_boot_but_update_sha_corrupted_denied) { add_payload(PART_UPDATE, 5, TEST_SIZE_SMALL); memset(bad_digest, 0xBA, SHA256_DIGEST_SIZE); ext_flash_unlock(); - ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + DIGEST_TLV_OFF_IN_HDR + 4, bad_digest, SHA256_DIGEST_SIZE); + update_part_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + DIGEST_TLV_OFF_IN_HDR + 4, bad_digest, SHA256_DIGEST_SIZE); ext_flash_lock(); wolfBoot_start(); /* We expect to panic */ @@ -1229,33 +1308,38 @@ START_TEST (test_diffbase_version_reads) prepare_flash(); ext_flash_unlock(); - ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS, + update_part_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS, (const uint8_t *)&magic, sizeof(magic)); version_le = host_to_img_u32(version); - ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 4, + update_part_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 4, (const uint8_t *)&version_le, sizeof(version_le)); word = (4u << 16) | HDR_VERSION; word_le = word; - ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 8, + update_part_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 8, (const uint8_t *)&word_le, sizeof(word_le)); - ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 12, + update_part_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 12, (const uint8_t *)&version_le, sizeof(version_le)); word = (2u << 16) | HDR_IMG_TYPE; word_le = word; - ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 16, + update_part_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 16, (const uint8_t *)&word_le, sizeof(word_le)); img_type_le = host_to_img_u16(img_type); - ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 20, + update_part_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 20, (const uint8_t *)&img_type_le, sizeof(img_type_le)); + /* The TLVs follow the sign tool's dense layout: the delta-base TLV + * starts right after the image-type TLV (offset 22). A gap here would + * be padding that only reads as 0xFF in plaintext builds; in encrypted + * builds the gap is ciphertext and the header walker would skip past + * the next TLV. */ word = (4u << 16) | HDR_IMG_DELTA_BASE; word_le = word; - ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 24, + update_part_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 22, (const uint8_t *)&word_le, sizeof(word_le)); delta_base_le = host_to_img_u32(delta_base); - ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 28, + update_part_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 26, (const uint8_t *)&delta_base_le, sizeof(delta_base_le)); ext_flash_lock(); @@ -1299,7 +1383,7 @@ START_TEST (test_diffbase_version_reads_from_little_endian_bytes) prepare_flash(); ext_flash_unlock(); - ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS, + update_part_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS, (const uint8_t *)&magic, sizeof(magic)); ext_flash_write_le32(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 4, TEST_SIZE_SMALL); @@ -1312,14 +1396,26 @@ START_TEST (test_diffbase_version_reads_from_little_endian_bytes) ext_flash_write_le16(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 20, img_type); tag = (4u << 16) | HDR_IMG_DELTA_BASE; - ext_flash_write_le32(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 24, tag); - ext_flash_write_le32(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 28, delta_base); + /* Dense TLV layout, as in the sign tool: no padding gap before the + * delta-base TLV (see test_diffbase_version_reads). */ + ext_flash_write_le32(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 22, tag); + ext_flash_write_le32(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 26, delta_base); ext_flash_lock(); ck_assert_uint_eq(wolfBoot_get_image_version(PART_UPDATE), version); ck_assert_uint_eq(wolfBoot_get_diffbase_version(PART_UPDATE), delta_base); - ck_assert_uint_eq(wolfBoot_get_blob_diffbase_version( - (uint8_t *)(uintptr_t)WOLFBOOT_PARTITION_UPDATE_ADDRESS), delta_base); + + /* The blob accessor expects a plaintext header; in EXT_ENCRYPTED + * builds the partition itself holds ciphertext, so hand it the + * decrypted copy. */ + { + uint8_t hdr[IMAGE_HEADER_SIZE]; + ext_flash_unlock(); + ext_flash_check_read((uintptr_t)WOLFBOOT_PARTITION_UPDATE_ADDRESS, hdr, + IMAGE_HEADER_SIZE); + ext_flash_lock(); + ck_assert_uint_eq(wolfBoot_get_blob_diffbase_version(hdr), delta_base); + } cleanup_flash(); } @@ -1730,6 +1826,7 @@ Suite *wolfboot_suite(void) #ifdef UNIT_TEST_FALLBACK_ONLY #ifdef EXT_ENCRYPTED tcase_add_test(fallback_verify, test_fallback_image_verification_rejects_corruption); + tcase_add_test(fallback_verify, test_fallback_iv_image_roundtrips); tcase_add_test(fallback_verify, test_final_swap_propagates_encrypt_key_read_failure); tcase_add_test(fallback_verify, test_final_swap_propagates_encrypt_key_persist_failure); #ifdef CUSTOM_ENCRYPT_KEY @@ -1796,8 +1893,19 @@ Suite *wolfboot_suite(void) #endif #ifdef EXT_ENCRYPTED tcase_add_test(fallback_verify, test_fallback_image_verification_rejects_corruption); + tcase_add_test(fallback_verify, test_fallback_iv_image_roundtrips); tcase_add_test(fallback_verify, test_final_swap_propagates_encrypt_key_read_failure); tcase_add_test(fallback_verify, test_final_swap_propagates_encrypt_key_persist_failure); +#ifdef CUSTOM_ENCRYPT_KEY + tcase_add_test(fallback_verify, test_boot_success_erases_encrypt_key); +#endif + tcase_add_test(encrypt_write_bounds, + test_encrypt_write_keeps_trailing_partial_block); + tcase_add_test(encrypt_write_bounds, + test_encrypt_write_zero_length_leaves_flash_untouched); + tcase_add_test(encrypt_write_bounds, + test_encrypt_write_reports_head_block_write_failure); + suite_add_tcase(s, encrypt_write_bounds); #endif suite_add_tcase(s, empty_panic); From 72c1ec234ae2ecc9d403644b7f4cbea78bb05846 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 08:57:46 +0200 Subject: [PATCH 22/27] bound the ELF paddr range check by the destination pointer width The F-9756 validation rejected seg_start > UINT64_MAX - filesz, but the very next line truncates: load_addr = (uintptr_t)seg_start. On 32-bit targets a paddr that fits in 64 bits but not in the 32-bit address space (e.g. 0x1_0000_0000) passed every check and the flash hash walk read the wrapped (possibly unmapped) address - the same fault class the check was written to prevent. Bound the range by UINTPTR_MAX, the width the cast actually uses, and pin the 32-bit-only case with a guard test. Skoll review finding 1, 2026-08-21 wolfboot review. --- src/image.c | 8 +++-- tools/unit-tests/unit-image-elf-scatter.c | 36 +++++++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/image.c b/src/image.c index d8a648b71d..b76648ff51 100644 --- a/src/image.c +++ b/src/image.c @@ -2247,8 +2247,9 @@ int wolfBoot_check_flash_image_elf(uint8_t part, unsigned long* entry_out) /* Validate the segment before hashing: the flash-address hash * reader consumes a uint32_t length, the file layout must stay - * inside the manifest image, and the paddr range must not - * overflow. Reject instead of continuing. */ + * inside the manifest image, and the paddr range must fit the + * destination (uintptr_t) address width so the load_addr cast + * below cannot wrap. Reject instead of continuing. */ if (filesz > UINT32_MAX) { wolfBoot_printf("ELF: [CHECK] ERROR: segment file_size " "%lu does not fit a 32-bit length\n", @@ -2264,7 +2265,8 @@ int wolfBoot_check_flash_image_elf(uint8_t part, unsigned long* entry_out) return -1; } seg_start = paddr + (uint64_t)BASE_OFF; - if (seg_start < paddr || seg_start > UINT64_MAX - filesz) { + if (seg_start < paddr || + seg_start > (uint64_t)UINTPTR_MAX - filesz) { wolfBoot_printf("ELF: [CHECK] ERROR: segment paddr range " "overflows\n"); return -1; diff --git a/tools/unit-tests/unit-image-elf-scatter.c b/tools/unit-tests/unit-image-elf-scatter.c index 060defcf4d..c5c6510362 100644 --- a/tools/unit-tests/unit-image-elf-scatter.c +++ b/tools/unit-tests/unit-image-elf-scatter.c @@ -535,6 +535,39 @@ START_TEST(test_elf_scatter_paddr_range_overflow_rejected) } END_TEST +#if UINTPTR_MAX < UINT64_MAX +/* A paddr that fits in 64 bits but not in the destination (uintptr_t) + * width must be rejected: the load_addr cast after the check would + * silently wrap and the hash walk would read the wrapped address. + * 32-bit builds only: on 64-bit builds UINTPTR_MAX == UINT64_MAX and + * the case above already covers it. */ +START_TEST(test_elf_scatter_paddr_beyond_pointer_width_rejected) +{ + unsigned long entry = 0; + uint32_t fw_size = IMG_FW_SIZE; + 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 = 1ULL << 32; /* fits uint64_t, exceeds 32-bit width */ + segs[0].payload = seg2_flash; + segs[0].fillsz = SEG_SIZE; + + build_scattered_image_n(segs, 1, fw_size); + + ret = wolfBoot_check_flash_image_elf(PART_BOOT, &entry); + + ck_assert_int_eq(ret, -1); + + unmap_boot_partition(); +} +END_TEST +#endif + Suite *elf_scatter_suite(void) { Suite *s = suite_create("ELF flash-scatter image check"); @@ -544,6 +577,9 @@ Suite *elf_scatter_suite(void) tcase_add_test(tc, test_elf_scatter_filesz_over_32bit_rejected); tcase_add_test(tc, test_elf_scatter_segment_beyond_fw_size_rejected); tcase_add_test(tc, test_elf_scatter_paddr_range_overflow_rejected); +#if UINTPTR_MAX < UINT64_MAX + tcase_add_test(tc, test_elf_scatter_paddr_beyond_pointer_width_rejected); +#endif tcase_set_timeout(tc, 10); suite_add_tcase(s, tc); return s; From 1dcd481a9fd2151625849ff870e3dac13bfa587d Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 09:03:18 +0200 Subject: [PATCH 23/27] add unaligned fallback-IV RMW test for ext_flash_encrypt_write The F-9750 E2E roundtrip writes sector-aligned chunks, so it never enters the head/tail read-modify-write paths where the iv_offset_at_entry re-syncs live, and the RMW neighbour tests only run with the standard IV. The combination the fix protects - a fallback-IV write whose head and tail partial blocks land on already-encrypted blocks - was untested: a dropped re-sync offset re-anchors exactly one block at the standard-IV position and no current test would catch it. New test primes two blocks under the fallback IV, patches them with one unaligned write (head RMW + tail RMW, block-size independent so it runs on the 16-byte AES and 64-byte ChaCha builds), and reads back with the fallback IV forced the way the update flow does. Verified to fail when the head RMW re-sync offset restore is removed. Skoll review finding 2, 2026-08-21 wolfboot review. --- tools/unit-tests/unit-extflash.c | 61 ++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/tools/unit-tests/unit-extflash.c b/tools/unit-tests/unit-extflash.c index dbcd46b895..aae92db6f0 100644 --- a/tools/unit-tests/unit-extflash.c +++ b/tools/unit-tests/unit-extflash.c @@ -447,6 +447,63 @@ START_TEST(test_ext_enc_flash_rmw_tail_preserves_neighbors) { } END_TEST +/* A fallback-IV write whose head and tail partial blocks land on blocks + * that already hold encrypted content: the RMW re-syncs must re-apply the + * one-shot IV offset (wolfBoot_crypto_set_iv consumes it), or the partial + * blocks are re-anchored at the standard-IV position and only those blocks + * read back wrong under the forced fallback-IV read. */ +#ifdef EXT_ENCRYPTED +START_TEST(test_ext_enc_flash_rmw_fallback_iv_preserves_neighbors) { + uint32_t address = 0x1000; + uint8_t block_a[TEST_BLOCK_SIZE]; + uint8_t block_b[TEST_BLOCK_SIZE]; + uint8_t payload[TEST_BLOCK_SIZE + 7]; + uint8_t expect[2 * TEST_BLOCK_SIZE]; + uint8_t data[2 * TEST_BLOCK_SIZE]; + const int off = 4; + const int head_len = TEST_BLOCK_SIZE - off; + const int tail_len = 7 + off; /* len - head_len, block-size independent */ + const int len = TEST_BLOCK_SIZE + 7; + int prevf, i, rres, wres; + + for (i = 0; i < TEST_BLOCK_SIZE; i++) { + block_a[i] = (uint8_t)(0xF0 + i); + block_b[i] = (uint8_t)(0xF8 + i); + } + for (i = 0; i < (int)sizeof(payload); i++) + payload[i] = (uint8_t)(0x0F + i); + + /* Prime both blocks under the fallback IV (aligned full-block writes; + * the offset is one-shot, so re-enable it before every write). */ + wolfBoot_enable_fallback_iv(1); + wres = ext_flash_check_write(address, block_a, TEST_BLOCK_SIZE); + ck_assert_int_eq(wres, 0); + wolfBoot_enable_fallback_iv(1); + wres = ext_flash_check_write(address + TEST_BLOCK_SIZE, block_b, + TEST_BLOCK_SIZE); + ck_assert_int_eq(wres, 0); + + /* One unaligned write: head RMW in block zero, tail RMW in block one */ + wolfBoot_enable_fallback_iv(1); + wres = ext_flash_check_write(address + off, payload, len); + ck_assert_int_eq(wres, 0); + + memcpy(expect, block_a, off); + memcpy(expect + off, payload, head_len); + memcpy(expect + TEST_BLOCK_SIZE, payload + head_len, tail_len); + memcpy(expect + TEST_BLOCK_SIZE + tail_len, block_b + tail_len, + TEST_BLOCK_SIZE - tail_len); + + /* The update flow reads a fallback image with the fallback IV forced */ + prevf = wolfBoot_force_fallback_iv(1); + rres = ext_flash_check_read(address, data, 2 * TEST_BLOCK_SIZE); + wolfBoot_force_fallback_iv(prevf); + ck_assert_int_eq(rres, 2 * TEST_BLOCK_SIZE); + ck_assert_mem_eq(data, expect, 2 * TEST_BLOCK_SIZE); +} +END_TEST +#endif /* EXT_ENCRYPTED */ + /* A stream written in small, unaligned chunks must round-trip: every chunk * boundary exercises the partial-block read-modify-write against content the * previous chunks already encrypted. */ @@ -506,6 +563,10 @@ Suite *wolfboot_suite(void) test_ext_enc_flash_rmw_head_preserves_neighbors); tcase_add_test(ext_enc_flash_rmw, test_ext_enc_flash_rmw_tail_preserves_neighbors); +#ifdef EXT_ENCRYPTED + tcase_add_test(ext_enc_flash_rmw, + test_ext_enc_flash_rmw_fallback_iv_preserves_neighbors); +#endif tcase_add_test(ext_enc_flash_rmw, test_ext_enc_flash_chunked_stream_roundtrip); From 9b1a48554ab43cb093809536881c3d2b6884fff1 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 09:08:26 +0200 Subject: [PATCH 24/27] share the DTS size bounds in fdt.h and state the window assumption The WOLFBOOT_DTS_MAX_SIZE/WOLFBOOT_DTS_MIN_SIZE pair was defined separately in update_disk.c (F-7066) and update_ram.c (pre-existing), so the two copies could drift. Move it to include/fdt.h, the FDT dialect header both translation units already pull in via image.h; the hal override (nxp_ppc.h, included before fdt.h in boot_ppc.c) keeps its precedence. Also replace the 'bounded by the staging region' comment, which claimed more than the code guarantees: the copy is clamped to WOLFBOOT_DTS_MAX_SIZE, so the staging window at WOLFBOOT_LOAD_DTS_ADDRESS must be at least that large (or the bound overridden for the target), and the header comment now says so. Skoll review finding 3, 2026-08-21 wolfboot review. --- include/fdt.h | 12 ++++++++++++ src/update_disk.c | 17 ++++------------- src/update_ram.c | 17 ++++------------- 3 files changed, 20 insertions(+), 26 deletions(-) diff --git a/include/fdt.h b/include/fdt.h index 6d982737c0..2f60d3965b 100644 --- a/include/fdt.h +++ b/include/fdt.h @@ -73,6 +73,18 @@ struct fdt_property { #define FDT_ALIGN(x, a) (((x) + (a) - 1) & ~((a) - 1)) #define FDT_TAGALIGN(x) (FDT_ALIGN((x), FDT_TAGSIZE)) +/* Bounds for the attacker-influenced fdt_totalsize before relocating or + * forwarding a DTB. MIN is the FDT v17 header size (also enforced by the + * signer): fdt_check_header validates magic/version but not totalsize, so a + * crafted header with a tiny totalsize must be rejected rather than + * loaded/forwarded as a partial tree. The MAX default assumes a staging + * window at WOLFBOOT_LOAD_DTS_ADDRESS of at least 1 MiB; targets with a + * smaller window must override WOLFBOOT_DTS_MAX_SIZE (see hal/nxp_ppc.h). */ +#ifndef WOLFBOOT_DTS_MAX_SIZE +#define WOLFBOOT_DTS_MAX_SIZE (1024U * 1024U) +#endif +#define WOLFBOOT_DTS_MIN_SIZE (40U) + #define FDT_FIRST_SUPPORTED_VERSION 0x10 #define FDT_LAST_SUPPORTED_VERSION 0x11 diff --git a/src/update_disk.c b/src/update_disk.c index e371925a15..4ee5bad6a0 100644 --- a/src/update_disk.c +++ b/src/update_disk.c @@ -249,17 +249,6 @@ static void disk_decrypted_header_clear(uint8_t *hdr) extern int wolfBoot_get_dts_size(void *dts_addr); -#if defined(MMU) || defined(WOLFBOOT_FDT) -/* Bounds for the attacker-influenced fdt_totalsize before relocating a DTB. - * MIN is the FDT v17 header size (also enforced by the signer): fdt_check_header - * validates magic/version but not totalsize, so a crafted header with a tiny - * totalsize must be rejected rather than loaded/forwarded as a partial tree. */ -#ifndef WOLFBOOT_DTS_MAX_SIZE -#define WOLFBOOT_DTS_MAX_SIZE (1024U * 1024U) -#endif -#define WOLFBOOT_DTS_MIN_SIZE (40U) -#endif - #if defined(WOLFBOOT_NO_LOAD_ADDRESS) || !defined(WOLFBOOT_LOAD_ADDRESS) /* from the linker, where wolfBoot ends */ extern uint8_t _end_wb[]; @@ -650,8 +639,10 @@ void RAMFUNCTION wolfBoot_start(void) parsed >= (int)WOLFBOOT_DTS_MIN_SIZE && (uint32_t)parsed <= WOLFBOOT_DTS_MAX_SIZE) { /* Relocate to the load DTS address. The copy length is - * the parsed DTB size (bounded by the staging region), - * not the FIT-declared property length. */ + * the parsed DTB size, clamped to WOLFBOOT_DTS_MAX_SIZE, + * not the FIT-declared property length. The staging window + * at WOLFBOOT_LOAD_DTS_ADDRESS must be at least that large + * (or the bound must be overridden for the target). */ dts_addr = (uint8_t*)WOLFBOOT_LOAD_DTS_ADDRESS; dts_size = (uint32_t)parsed; wolfBoot_printf("Loading DTS: %p -> %p (%d bytes)\n", diff --git a/src/update_ram.c b/src/update_ram.c index b632b16d16..8b98cbd40b 100644 --- a/src/update_ram.c +++ b/src/update_ram.c @@ -51,17 +51,6 @@ extern void hal_flash_dualbank_swap(void); extern uint32_t kernel_load_addr; extern uint32_t dts_load_addr; -#if defined(MMU) || defined(WOLFBOOT_FDT) -/* Bounds for the attacker-influenced fdt_totalsize before relocating a DTB. - * MIN is the FDT v17 header size (also enforced by the signer): fdt_check_header - * validates magic/version but not totalsize, so a crafted header with a tiny - * totalsize must be rejected rather than loaded/forwarded as a partial tree. */ -#ifndef WOLFBOOT_DTS_MAX_SIZE -#define WOLFBOOT_DTS_MAX_SIZE (1024U * 1024U) -#endif -#define WOLFBOOT_DTS_MIN_SIZE (40U) -#endif - #if defined(__WOLFBOOT) && defined(WOLFBOOT_LOAD_ADDRESS) extern uint8_t _end[]; /* linker symbol: end of wolfBoot BSS */ #endif @@ -662,8 +651,10 @@ void RAMFUNCTION wolfBoot_start(void) parsed >= (int)WOLFBOOT_DTS_MIN_SIZE && (uint32_t)parsed <= WOLFBOOT_DTS_MAX_SIZE) { /* Relocate to the load DTS address. The copy length is - * the parsed DTB size (bounded by the staging region), - * not the FIT-declared property length. */ + * the parsed DTB size, clamped to WOLFBOOT_DTS_MAX_SIZE, + * not the FIT-declared property length. The staging window + * at WOLFBOOT_LOAD_DTS_ADDRESS must be at least that large + * (or the bound must be overridden for the target). */ dts_addr = (uint8_t*)WOLFBOOT_LOAD_DTS_ADDRESS; dts_size = (uint32_t)parsed; wolfBoot_printf("Loading DTS: %p -> %p (%d bytes)\n", From ca6b60a1e6fbf56996e1f40f6b83d8360503604c Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 09:11:44 +0200 Subject: [PATCH 25/27] scrub the RMW scratch buffers in ext_flash_encrypt_write Since F-9750 the head and tail read-modify-write paths decrypt the stored neighbour block into block/enc_block before splicing the caller's bytes, so the two stack buffers transiently hold plaintext the caller never supplied, and several exits returned without scrubbing them (stale head plaintext also outlived into the tail path). Funnel every exit after the partition switch through a single cleanup that ForceZero()s both buffers, matching the zeroization posture of the rest of the campaign (F-7396 header cache, F-7966/F-7971 keys, aes_set_iv IV). Defense-in-depth: the buffers are stack-local, but this is the most long-lived plaintext in the write path. Skoll review finding 4, 2026-08-21 wolfboot review. --- src/libwolfboot.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/libwolfboot.c b/src/libwolfboot.c index 75811b316c..c211be6027 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -2723,7 +2723,8 @@ int RAMFUNCTION ext_flash_encrypt_write(uintptr_t address, const uint8_t *data, step = len; if (ext_flash_read(row_address, block, ENCRYPT_BLOCK_SIZE) != ENCRYPT_BLOCK_SIZE) { - return -1; + ret = -1; + goto exit; } /* The stored block is ciphertext: decrypt it so the untouched bytes * can be patched as plaintext and re-encrypted. Re-encrypting the @@ -2740,10 +2741,10 @@ int RAMFUNCTION ext_flash_encrypt_write(uintptr_t address, const uint8_t *data, crypto_encrypt(enc_block, block, ENCRYPT_BLOCK_SIZE); ret = ext_flash_write(row_address, enc_block, ENCRYPT_BLOCK_SIZE); if (ret < 0) - return ret; + goto exit; /* The request fits entirely within this block: nothing left to do */ if (step == len) - return ret; + goto exit; address += step; data += step; sz = len - step; @@ -2763,7 +2764,7 @@ int RAMFUNCTION ext_flash_encrypt_write(uintptr_t address, const uint8_t *data, } ret = ext_flash_write(address, ENCRYPT_CACHE, chunk); if (ret < 0) - return ret; + goto exit; address += chunk; data += chunk; step -= chunk; @@ -2779,7 +2780,8 @@ int RAMFUNCTION ext_flash_encrypt_write(uintptr_t address, const uint8_t *data, ENCRYPT_BLOCK_SIZE; if (ext_flash_read(address, block, ENCRYPT_BLOCK_SIZE) != ENCRYPT_BLOCK_SIZE) { - return -1; + ret = -1; + goto exit; } /* Sync the decrypt context to this block (on backends with separate * encrypt/decrypt contexts it did not advance with the full-block @@ -2796,6 +2798,12 @@ int RAMFUNCTION ext_flash_encrypt_write(uintptr_t address, const uint8_t *data, ret = ext_flash_write(address, enc_block, ENCRYPT_BLOCK_SIZE); } +exit: + /* The head/tail RMW paths above decrypted the stored neighbour blocks + * into block/enc_block; scrub the plaintext (and any stale copies) + * on every exit so it does not outlive the write on the stack. */ + ForceZero(block, sizeof(block)); + ForceZero(enc_block, sizeof(enc_block)); return ret; } From 4769c723e9096cc4ca343ec12bf1e42340640ce3 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 09:12:05 +0200 Subject: [PATCH 26/27] name the full 32-bit gated set in the skip message unit-sama5d3-ext-read joined the ENABLE_32BIT_TESTS gate but the info line still only named the linux-loader tests, which misleads anyone debugging a skipped suite. Skoll review finding 5, 2026-08-21 wolfboot review. --- tools/unit-tests/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 14b86a6477..c4cbfc6cc5 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -153,7 +153,7 @@ TESTS+=unit-linux-loader-e820 TESTS+=unit-linux-loader-syssize TESTS+=unit-sama5d3-ext-read else -$(info Skipping 32-bit x86 linux-loader unit tests: 'gcc -m32' unavailable (set ENABLE_32BIT_TESTS=1 to force)) +$(info Skipping 32-bit x86 unit tests (linux-loader, sama5d3-ext-read): 'gcc -m32' unavailable (set ENABLE_32BIT_TESTS=1 to force)) endif include unit-sign-encrypted-output.mkfrag From 5cd23c7ceabeba71bdc0b2c6bc6c6422b1338466 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 11:13:53 +0200 Subject: [PATCH 27/27] raise footprint limits for fenrir-fixes-2026-08-21 (max +16B) 12 of the 22 test-size-all configs grew 4B (ECC384 NO_ASM 16B), all within the 32B-per-config ratchet. Re-measured in the CI footprint container (ghcr.io/wolfssl/wolfboot-ci-arm:v1.0) with the exact CI sequence (stm32f407-discovery config, keytools, per-signature rebuilds) and ratcheted each grown limit to the measured size; test-size-all passes 22/22 with the new limits. RSAPSS2048/3072/4096 (asm) shrank 4B; their limits are left as-is. --- tools/test.mk | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tools/test.mk b/tools/test.mk index ff05573205..4080c3f4f4 100644 --- a/tools/test.mk +++ b/tools/test.mk @@ -1228,31 +1228,31 @@ test-all: clean test-size-all: - make test-size SIGN=NONE LIMIT=5112 NO_ARM_ASM=1 + make test-size SIGN=NONE LIMIT=5116 NO_ARM_ASM=1 make keysclean - make test-size SIGN=ED25519 LIMIT=12224 NO_ARM_ASM=1 + make test-size SIGN=ED25519 LIMIT=12228 NO_ARM_ASM=1 make keysclean - make test-size SIGN=ECC256 LIMIT=18920 NO_ARM_ASM=1 + make test-size SIGN=ECC256 LIMIT=18924 NO_ARM_ASM=1 make clean - make test-size SIGN=ECC256 NO_ASM=1 LIMIT=13952 NO_ARM_ASM=1 + make test-size SIGN=ECC256 NO_ASM=1 LIMIT=13956 NO_ARM_ASM=1 make keysclean make test-size SIGN=RSA2048 LIMIT=11808 NO_ARM_ASM=1 make clean - make test-size SIGN=RSA2048 NO_ASM=1 LIMIT=12368 NO_ARM_ASM=1 + make test-size SIGN=RSA2048 NO_ASM=1 LIMIT=12372 NO_ARM_ASM=1 make keysclean make test-size SIGN=RSA4096 LIMIT=12108 NO_ARM_ASM=1 make clean - make test-size SIGN=RSA4096 NO_ASM=1 LIMIT=12648 NO_ARM_ASM=1 + make test-size SIGN=RSA4096 NO_ASM=1 LIMIT=12652 NO_ARM_ASM=1 make keysclean - make test-size SIGN=ECC384 LIMIT=19604 NO_ARM_ASM=1 + make test-size SIGN=ECC384 LIMIT=19608 NO_ARM_ASM=1 make clean - make test-size SIGN=ECC384 NO_ASM=1 LIMIT=15300 NO_ARM_ASM=1 + make test-size SIGN=ECC384 NO_ASM=1 LIMIT=15316 NO_ARM_ASM=1 make keysclean - make test-size SIGN=ED448 LIMIT=14252 NO_ARM_ASM=1 + make test-size SIGN=ED448 LIMIT=14256 NO_ARM_ASM=1 make keysclean make test-size SIGN=RSA3072 LIMIT=11948 NO_ARM_ASM=1 make clean - make test-size SIGN=RSA3072 NO_ASM=1 LIMIT=12476 NO_ARM_ASM=1 + make test-size SIGN=RSA3072 NO_ASM=1 LIMIT=12480 NO_ARM_ASM=1 make keysclean make test-size SIGN=RSAPSS2048 LIMIT=13744 NO_ARM_ASM=1 make clean @@ -1268,12 +1268,12 @@ test-size-all: make keysclean make test-size SIGN=LMS LMS_LEVELS=2 LMS_HEIGHT=5 LMS_WINTERNITZ=8 \ WOLFBOOT_SMALL_STACK=0 IMAGE_SIGNATURE_SIZE=2644 \ - IMAGE_HEADER_SIZE?=5288 LIMIT=8116 NO_ARM_ASM=1 + IMAGE_HEADER_SIZE?=5288 LIMIT=8120 NO_ARM_ASM=1 make keysclean make test-size SIGN=XMSS XMSS_PARAMS='XMSS-SHA2_10_256' \ IMAGE_SIGNATURE_SIZE=2500 IMAGE_HEADER_SIZE?=4096 \ LIMIT=8768 NO_ARM_ASM=1 make keysclean make clean - make test-size SIGN=ML_DSA ML_DSA_LEVEL=2 LIMIT=19578 \ + make test-size SIGN=ML_DSA ML_DSA_LEVEL=2 LIMIT=19582 \ IMAGE_SIGNATURE_SIZE=2420 IMAGE_HEADER_SIZE?=8192