From 0e2a46375018aa3f76088bde09682278d9e5ad31 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 11:53:12 +0200 Subject: [PATCH] F-11047: image: validate ELF scatter segments before flash writes wolfBoot_load_flash_image_elf() ignored read_flash_fwimage failures and used the program-header fields unchecked, so a failed read consumed indeterminate stack data and a malformed (but signed) ELF could drive an out-of-bounds source read and erase/write at an unintended destination. Check the ELF header and program header reads, then validate each PT_LOAD segment before copying: file_size fits a 32-bit length, the source stays inside the manifest image, the paddr range fits the destination address width, and the destination stays inside the boot partition (mirrors the sibling check-function validation). Check the copy result. New unit tests in unit-image-elf-scatter.c cover the load path: a valid restore (positive control) and rejections for source past fw_size, paddr range overflow, destination outside the boot partition, and a program header that cannot be read. The first three fail pre-fix (the mock flash layer also catches the two out-of-range destinations with its own address check); the phdr read-failure case consumed uninitialized stack data pre-fix (valgrind: conditional jump on uninitialised value at the is_loadable check). --- src/image.c | 64 +++++++- tools/unit-tests/unit-image-elf-scatter.c | 189 +++++++++++++++++++++- 2 files changed, 245 insertions(+), 8 deletions(-) diff --git a/src/image.c b/src/image.c index 8914ba99..c3ea6566 100644 --- a/src/image.c +++ b/src/image.c @@ -2438,7 +2438,11 @@ int wolfBoot_load_flash_image_elf(int part, unsigned long* entry_out, int ext_fl /* Get the elf header from the image into a local buffer. We may overread * the buffer depending on architecture */ memset(elfHdrBuf, 0, sizeof(elfHdrBuf)); - read_flash_fwimage(&boot, 0, elfHdrBuf, sizeof(elfHeaderMaxBuf)); + if (read_flash_fwimage(&boot, 0, elfHdrBuf, + sizeof(elfHeaderMaxBuf)) != 0) { + wolfBoot_printf("ELF: [STORE] ERROR: could not read ELF header\n"); + return -1; + } if (elf_open(elfHdrBuf, &is_elf32) != 0) { return -1; } @@ -2472,11 +2476,17 @@ int wolfBoot_load_flash_image_elf(int part, unsigned long* entry_out, int ext_fl unsigned long paddr, filesz, offset; int is_loadable; uintptr_t load_addr; + uint64_t seg_start; /* Read the current program header into a local buffer */ if (is_elf32) { elf32_program_header p32; - read_flash_fwimage(&boot, entry_off, &p32, sizeof(p32)); + if (read_flash_fwimage(&boot, entry_off, &p32, + sizeof(p32)) != 0) { + wolfBoot_printf("ELF: [STORE] ERROR: could not read " + "program header\n"); + return -1; + } is_loadable = (p32.type == ELF_PT_LOAD); paddr = (unsigned long)p32.paddr; offset = (unsigned long)p32.offset; @@ -2485,7 +2495,12 @@ int wolfBoot_load_flash_image_elf(int part, unsigned long* entry_out, int ext_fl } else { elf64_program_header p64; - read_flash_fwimage(&boot, entry_off, &p64, sizeof(p64)); + if (read_flash_fwimage(&boot, entry_off, &p64, + sizeof(p64)) != 0) { + wolfBoot_printf("ELF: [STORE] ERROR: could not read " + "program header\n"); + return -1; + } is_loadable = (p64.type == ELF_PT_LOAD); paddr = (unsigned long)p64.paddr; offset = (unsigned long)p64.offset; @@ -2498,12 +2513,49 @@ int wolfBoot_load_flash_image_elf(int part, unsigned long* entry_out, int ext_fl return -1; } - load_addr = (uintptr_t)(paddr + BASE_OFF); + /* Validate the segment before writing: the source must stay + * inside the manifest image, the paddr range must fit the + * destination (uintptr_t) width, and the destination must stay + * inside the boot partition. Reject instead of writing. */ + if (filesz > UINT32_MAX) { + wolfBoot_printf("ELF: [STORE] ERROR: segment file_size " + "%lu does not fit a 32-bit length\n", + (unsigned long)filesz); + return -1; + } + if (offset > (uint64_t)boot.fw_size || + filesz > (uint64_t)boot.fw_size - offset) { + wolfBoot_printf("ELF: [STORE] ERROR: segment offset %lu + " + "size %lu exceeds image size %u\n", + (unsigned long)offset, + (unsigned long)filesz, boot.fw_size); + return -1; + } + seg_start = paddr + (uint64_t)BASE_OFF; + if (seg_start < paddr || + seg_start > (uint64_t)UINTPTR_MAX - filesz) { + wolfBoot_printf("ELF: [STORE] ERROR: segment paddr range " + "overflows\n"); + return -1; + } + load_addr = (uintptr_t)seg_start; + if (load_addr < (uintptr_t)boot.hdr || + load_addr + filesz > + (uintptr_t)boot.hdr + (uintptr_t)WOLFBOOT_PARTITION_SIZE) { + wolfBoot_printf("ELF: [STORE] ERROR: segment destination " + "outside boot partition\n"); + return -1; + } + wolfBoot_printf("ELF: [STORE] Writing loadable segment: " "loadaddr=0x%08lx, offset=0x%08lx, size=%lu\n", (unsigned long)load_addr, offset, filesz); - copy_flash_buffered((uintptr_t)(image + offset), load_addr, filesz, - ext_flash, ext_flash); + if (copy_flash_buffered((uintptr_t)(image + offset), load_addr, + filesz, ext_flash, ext_flash) != 0) { + wolfBoot_printf("ELF: [STORE] ERROR: could not write " + "loadable segment\n"); + return -1; + } entry_off += ph_size; } diff --git a/tools/unit-tests/unit-image-elf-scatter.c b/tools/unit-tests/unit-image-elf-scatter.c index c5c65103..c53760d1 100644 --- a/tools/unit-tests/unit-image-elf-scatter.c +++ b/tools/unit-tests/unit-image-elf-scatter.c @@ -568,10 +568,184 @@ START_TEST(test_elf_scatter_paddr_beyond_pointer_width_rejected) END_TEST #endif +/* --- wolfBoot_load_flash_image_elf() (the store/restore path) --- + * + * The load function walks the same program header table and copies each + * PT_LOAD segment from the manifest (fw_base + offset) to its scattered + * destination (paddr + BASE_OFF). In this harness the boot partition is + * mmap'd at MOCK_ADDRESS_BOOT (== WOLFBOOT_PARTITION_BOOT_ADDRESS) and + * BASE_OFF is 0, so a valid destination is any address inside + * [MOCK_ADDRESS_BOOT, MOCK_ADDRESS_BOOT + WOLFBOOT_PARTITION_SIZE). + * The mock flash layer fails the test on any erase/write outside the + * known partition ranges, so an unvalidated paddr cannot walk off into + * unmapped memory here. */ + +#define LOAD_DEST (MOCK_ADDRESS_BOOT + 0x4000) + +static void set_load_paddr(uint64_t paddr) +{ + uint8_t *manifest = (uint8_t *)(uintptr_t)MOCK_ADDRESS_BOOT; + elf64_program_header *ph = (elf64_program_header *) + (manifest + IMAGE_HEADER_SIZE + sizeof(elf64_header)); + + ph->paddr = paddr; +} + +START_TEST(test_elf_scatter_load_valid_image_restores) +{ + unsigned long entry = 0; + uint8_t *manifest = (uint8_t *)(uintptr_t)MOCK_ADDRESS_BOOT; + uint8_t *source = manifest + IMAGE_HEADER_SIZE + ELF_HDR_SZ; + uint8_t *dest = (uint8_t *)(uintptr_t)LOAD_DEST; + unsigned int i; + int ret; + + map_boot_partition(); + + build_scattered_image(); + set_load_paddr(LOAD_DEST); + for (i = 0; i < SEG_SIZE; i++) { + source[i] = (uint8_t)(0x30U + i); + } + + ret = wolfBoot_load_flash_image_elf(PART_BOOT, &entry, 0); + + ck_assert_int_eq(ret, 0); + for (i = 0; i < SEG_SIZE; i++) { + ck_assert_uint_eq(dest[i], source[i]); + } + + unmap_boot_partition(); +} +END_TEST + +/* A segment whose file layout (offset + file_size) extends past the + * manifest image must be rejected before any flash write. Pre-fix the + * source read walked past fw_size and the copy still "succeeded" + * (ret 0). */ +START_TEST(test_elf_scatter_load_segment_beyond_fw_size_rejected) +{ + unsigned long entry = 0; + struct seg_spec segs[1]; + int ret; + + map_boot_partition(); + + memset(seg2_flash, 0, sizeof(seg2_flash)); + segs[0].offset = ELF_HDR_SZ; + segs[0].filesz = SEG1_SIZE; /* 0x2000 > the manifest layout slack */ + segs[0].paddr = LOAD_DEST; + segs[0].payload = seg2_flash; + segs[0].fillsz = 0; + + build_scattered_image_n(segs, 1, IMG_FW_SIZE); + + ret = wolfBoot_load_flash_image_elf(PART_BOOT, &entry, 0); + + /* Pre-fix this returned 0: the mock happily erased/wrote the valid + * destination with bytes read past fw_size. */ + ck_assert_int_eq(ret, -1); + + unmap_boot_partition(); +} +END_TEST + +/* A paddr whose segment range overflows the address space must be + * rejected before any flash access. Pre-fix the wrapped load_addr drove + * the mock into its out-of-range erase check (fail("Invalid address")). */ +START_TEST(test_elf_scatter_load_paddr_range_overflow_rejected) +{ + unsigned long entry = 0; + struct seg_spec segs[1]; + int ret; + + map_boot_partition(); + + memset(seg2_flash, 0, sizeof(seg2_flash)); + segs[0].offset = ELF_HDR_SZ; + segs[0].filesz = SEG_SIZE; + segs[0].paddr = UINT64_MAX - 4; /* +SEG_SIZE wraps past UINT64_MAX */ + segs[0].payload = seg2_flash; + segs[0].fillsz = SEG_SIZE; + + build_scattered_image_n(segs, 1, IMG_FW_SIZE); + + ret = wolfBoot_load_flash_image_elf(PART_BOOT, &entry, 0); + + ck_assert_int_eq(ret, -1); + + unmap_boot_partition(); +} +END_TEST + +/* A destination outside the boot partition must be rejected before any + * flash access. Pre-fix the mock's out-of-range erase check caught it + * with fail("Invalid address"); on real hardware this is an + * erase/write at an unintended location. */ +START_TEST(test_elf_scatter_load_dest_outside_partition_rejected) +{ + unsigned long entry = 0; + struct seg_spec segs[1]; + int ret; + + map_boot_partition(); + + memset(seg2_flash, 0, sizeof(seg2_flash)); + segs[0].offset = ELF_HDR_SZ; + segs[0].filesz = SEG_SIZE; + segs[0].paddr = (uint64_t)(MOCK_ADDRESS_BOOT + WOLFBOOT_PARTITION_SIZE + + 0x1000); + segs[0].payload = seg2_flash; + segs[0].fillsz = SEG_SIZE; + + build_scattered_image_n(segs, 1, IMG_FW_SIZE); + + ret = wolfBoot_load_flash_image_elf(PART_BOOT, &entry, 0); + + ck_assert_int_eq(ret, -1); + + unmap_boot_partition(); +} +END_TEST + +/* A program header that lies past the end of the manifest image cannot + * be read; the load must abort instead of consuming the uninitialized + * header locals. The PHT sits at fw offset 64 (right after the 64-byte + * ELF header, the only offset check_scatter_format accepts), so a + * fw_size of 100 makes the 56-byte phdr read (64 + 56) run past + * fw_size. Pre-fix the read failure was ignored and p64 held + * indeterminate stack data (valgrind: conditional jump on uninitialised + * value at the is_loadable check). */ +START_TEST(test_elf_scatter_load_phdr_read_failure_rejected) +{ + unsigned long entry = 0; + struct seg_spec segs[1]; + int ret; + + map_boot_partition(); + + memset(seg2_flash, 0, sizeof(seg2_flash)); + segs[0].offset = ELF_HDR_SZ; + segs[0].filesz = SEG_SIZE; + segs[0].paddr = LOAD_DEST; + segs[0].payload = seg2_flash; + segs[0].fillsz = SEG_SIZE; + + build_scattered_image_n(segs, 1, 100); /* PHT extends past fw_size */ + + ret = wolfBoot_load_flash_image_elf(PART_BOOT, &entry, 0); + + ck_assert_int_eq(ret, -1); + + unmap_boot_partition(); +} +END_TEST + Suite *elf_scatter_suite(void) { - Suite *s = suite_create("ELF flash-scatter image check"); - TCase *tc = tcase_create("wolfBoot_check_flash_image_elf"); + Suite *s = suite_create("ELF flash-scatter image check"); + TCase *tc = tcase_create("wolfBoot_check_flash_image_elf"); + TCase *tc_load = tcase_create("wolfBoot_load_flash_image_elf"); tcase_add_test(tc, test_elf_scatter_valid_image_verifies_ok); tcase_add_test(tc, test_elf_scatter_corrupted_segment_rejected); tcase_add_test(tc, test_elf_scatter_filesz_over_32bit_rejected); @@ -582,6 +756,17 @@ Suite *elf_scatter_suite(void) #endif tcase_set_timeout(tc, 10); suite_add_tcase(s, tc); + + tcase_add_test(tc_load, test_elf_scatter_load_valid_image_restores); + tcase_add_test(tc_load, + test_elf_scatter_load_segment_beyond_fw_size_rejected); + tcase_add_test(tc_load, + test_elf_scatter_load_paddr_range_overflow_rejected); + tcase_add_test(tc_load, + test_elf_scatter_load_dest_outside_partition_rejected); + tcase_add_test(tc_load, test_elf_scatter_load_phdr_read_failure_rejected); + tcase_set_timeout(tc_load, 10); + suite_add_tcase(s, tc_load); return s; }