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.
pull/868/head
Daniele Lacamera 2026-08-21 08:57:46 +02:00
parent 04531cbf2d
commit 72c1ec234a
2 changed files with 41 additions and 3 deletions

View File

@ -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;

View File

@ -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;