oneshot hash build option

[bigbrett]

Adds build option to enable hashing an entire image in one update call, which can dramatically speed up performance on platforms that can support it (memory mapped flash or wolfHSM client offload with DMA).

Deliberately did not make this incompatible at compile time with EXT_FLASH to prevent interfering with "off-label usage" of EXT_FLASH by some HALs (for example, used for flash read indirection only in certain cases on TC3xx), however the logical incompatibility of the two is clearly stated in the documentation.

[Copilot]

Pull request overview

Adds a build option to enable hashing a full image (and header region) in a single hash update call to improve verification performance on memory-mapped/offload-capable platforms.

Changes:

• Introduces WOLFBOOT_IMG_HASH_ONESHOT build flag (config + options) to toggle one-shot hashing.
• Updates image hashing paths in src/image.c to use a single wc_*Update() call when enabled.
• Documents the option and expands CI workflows to build/run simulator tests with the flag enabled.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tools/config.mk	Adds default value and exposes WOLFBOOT_IMG_HASH_ONESHOT in CONFIG_VARS.
options.mk	Maps WOLFBOOT_IMG_HASH_ONESHOT=1 to -DWOLFBOOT_IMG_HASH_ONESHOT.
src/image.c	Adds one-shot hashing branches for header/image hashing and flash hashing helpers.
docs/compile.md	Documents behavior, constraints, and incompatibility notes for one-shot hashing.
.github/workflows/test-sunnyday-simulator.yml	Adds simulator build+run coverage for oneshot + SHA256/SHA384/SHA3.
.github/workflows/test-elf-scattered.yml	Adds an ELF_SCATTERED simulator build+run with oneshot enabled.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #736

Scan targets checked: wolfboot-bugs, wolfboot-src
Findings: 3

Medium (2)

Missing bounds guard in ONESHOT header hash allows integer wrap on malformed image

File: src/image.c:990,1093,1202
Function: header_sha256, header_sha384, header_sha3_384
Category: Integer overflows

In the ONESHOT paths of header_sha256, header_sha384, and header_sha3_384, the expression (word32)(end_sha - p) is passed directly to the hash update function without first verifying that end_sha >= p. The original block-by-block code was naturally guarded by the while (p < end_sha) loop condition, which would simply not execute if end_sha <= p. In the ONESHOT path, if a malformed image header causes end_sha to be less than p (e.g., a crafted image where the stored SHA TLV appears at an unexpected offset), the pointer subtraction yields a negative ptrdiff_t which wraps to a very large value when cast to word32. This would cause wc_Sha256Update (and the sha384/sha3 variants) to read far beyond the intended buffer, potentially crashing the bootloader or causing a denial-of-service during firmware verification.

#ifdef WOLFBOOT_IMG_HASH_ONESHOT
    wc_Sha256Update(sha256_ctx, p, (word32)(end_sha - p));
#else
    {
        int blksz;
        while (p < end_sha) {

Recommendation: Add a bounds check before the ONESHOT hash call in all three header hash functions: if (end_sha <= p) return -1; (or return 0; to match the no-data behavior of the original loop). For example:

#ifdef WOLFBOOT_IMG_HASH_ONESHOT
    if (end_sha <= p)
        return -1;
    wc_Sha256Update(sha256_ctx, p, (word32)(end_sha - p));
#else

---

ONESHOT update_hash_flash_addr silently ignores src_ext flag with no compile-time guard

File: src/image.c:1803-1806
Function: update_hash_flash_addr
Category: Flash manipulation

When WOLFBOOT_IMG_HASH_ONESHOT is defined, update_hash_flash_addr casts addr to (uint8_t*) and dereferences it for size bytes, while discarding src_ext with (void)src_ext. The src_ext parameter indicates whether the address resides in external (non-memory-mapped) flash. If a user misconfigures the build by enabling both WOLFBOOT_IMG_HASH_ONESHOT=1 and EXT_FLASH=1, this function will attempt to directly dereference an external flash address as a memory pointer, leading to a hard fault, reading from an unintended memory region, or computing an incorrect hash — any of which could compromise the boot verification process. While the documentation warns against this combination, there is no #if defined(WOLFBOOT_IMG_HASH_ONESHOT) && defined(EXT_FLASH) #error ... #endif compile-time guard to enforce it.

#ifdef WOLFBOOT_IMG_HASH_ONESHOT
    (void)src_ext;
    update_hash(ctx, (uint8_t*)addr, size);
    return 0;
#else

Recommendation: Add a compile-time guard in src/image.c (or in options.mk) to prevent the incompatible configuration:

#if defined(WOLFBOOT_IMG_HASH_ONESHOT) && defined(EXT_FLASH)
    #error "WOLFBOOT_IMG_HASH_ONESHOT is incompatible with EXT_FLASH"
#endif

Alternatively, in options.mk:

ifeq ($(WOLFBOOT_IMG_HASH_ONESHOT),1)
  ifeq ($(EXT_FLASH),1)
    $(error WOLFBOOT_IMG_HASH_ONESHOT is incompatible with EXT_FLASH)
  endif
  CFLAGS+=-DWOLFBOOT_IMG_HASH_ONESHOT
endif

---

Low (1)

ONESHOT image hash paths skip NULL check on fw_base

File: src/image.c:1016,1122,1230
Function: image_sha256, image_sha384, image_sha3_384
Category: NULL pointer dereference

In the ONESHOT paths of image_sha256, image_sha384, and image_sha3_384, img->fw_base is passed directly to the hash update function (e.g., wc_Sha256Update(&sha256_ctx, img->fw_base, img->fw_size)). The original block-by-block code retrieved the data pointer via get_sha_block(img, position) and checked if (p == NULL) break;, which provided a safety net if the firmware base was not accessible. The ONESHOT path has no equivalent NULL check on img->fw_base. While img itself is checked in the preceding header_sha* call, img->fw_base could be NULL if the image metadata was initialized but the firmware was not memory-mapped, potentially causing a NULL pointer dereference.

#ifdef WOLFBOOT_IMG_HASH_ONESHOT
    wc_Sha256Update(&sha256_ctx, img->fw_base, img->fw_size);
#else
    {
        uint32_t position = 0;
        uint8_t* p;
        int      blksz;
        do {
            p = get_sha_block(img, position);
            if (p == NULL)
                break;

Recommendation: Add a NULL check on img->fw_base before the ONESHOT hash call in all three image hash functions:

#ifdef WOLFBOOT_IMG_HASH_ONESHOT
    if (img->fw_base == NULL)
        return -1;
    wc_Sha256Update(&sha256_ctx, img->fw_base, img->fw_size);
#else

---

This review was generated automatically by Fenrir. Findings are non-blocking.

[bigbrett]

Addressed Fenrir pointer validation concerns. Wontfix compile-time check against EXT_FLASH for reasons mentioned in PR description

[dgarske]

🐺 Skoll Code Review

Overall recommendation: REQUEST_CHANGES
Findings: 5 total — 5 posted, 0 skipped

Posted findings

• [High] Hash context resource leak on ONESHOT error path in header_sha functions* — src/image.c:987-992
• [High] Hash context resource leak on ONESHOT fw_base NULL check in image_sha functions* — src/image.c:1022-1024
• [Medium] Missing NULL and bounds checks in update_hash_flash_fwimg ONESHOT path — src/image.c:1779-1781
• [Medium] update_hash_flash_addr ONESHOT path silently ignores src_ext flag — src/image.c:1815-1818
• [Low] Behavioral difference between ONESHOT and non-ONESHOT when end_sha <= p — src/image.c:990-1005

Review generated by Skoll via openclaw

[dgarske]

🟠 [High] Hash context resource leak on ONESHOT error path in header_sha functions*
🚫 BLOCK bug

In all three header_sha* functions, the hash context is initialized (e.g. wc_InitSha256(sha256_ctx) at line 987) before the new ONESHOT guard if (end_sha <= p) return -1 at line 991-992. When this error path is taken, the hash context is initialized but never freed. The callers (image_sha256, etc.) return -1 immediately without calling wc_Sha*Free. This is a resource leak introduced by this PR.

The pre-existing error returns at lines 978 and 983 are safe because they occur before the init call. The non-ONESHOT path has no such early return after init — the while (p < end_sha) loop simply doesn't execute when p >= end_sha and returns 0.

This is especially concerning when WOLFBOOT_ENABLE_WOLFHSM_CLIENT is defined (line 984-985), as wc_InitSha256_ex(sha256_ctx, NULL, hsmDevIdHash) allocates HSM session resources that require explicit cleanup. The PR description explicitly mentions wolfHSM as a target use case.

Suggestion:

Suggested change

	wc_InitSha256(sha256_ctx);
	Move the ONESHOT guard before the init call, or add cleanup:
	end_sha = stored_sha - (2 * sizeof(uint16_t));
	#ifdef WOLFBOOT_IMG_HASH_ONESHOT
	if (end_sha <= p)
	return -1;
	#endif
	#ifdef WOLFBOOT_ENABLE_WOLFHSM_CLIENT
	(void)wc_InitSha256_ex(sha256_ctx, NULL, hsmDevIdHash);
	#else
	wc_InitSha256(sha256_ctx);
	#endif
	Alternatively, call wc_Sha256Free before the error return. Apply the same fix to header_sha384 (line 1094-1098) and header_sha3_384 (line 1208-1212).

[dgarske]

🟠 [High] Hash context resource leak on ONESHOT fw_base NULL check in image_sha functions*
🚫 BLOCK bug

In image_sha256 (and the SHA384/SHA3 equivalents), when header_sha256 succeeds (context is initialized and has header data hashed into it), the subsequent ONESHOT guard if (img->fw_base == NULL) return -1 at line 1023-1024 returns without calling wc_Sha256Final or wc_Sha256Free. The same pattern repeats in image_sha384 (line 1133-1134) and image_sha3_384 (line 1245-1246).

This leaks the hash context resources. For HSM-backed crypto, this leaks a hardware session.

Suggestion:

Suggested change

	#ifdef WOLFBOOT_IMG_HASH_ONESHOT
	Add cleanup before the error return:
	#ifdef WOLFBOOT_IMG_HASH_ONESHOT
	if (img->fw_base == NULL) {
	wc_Sha256Free(&sha256_ctx);
	return -1;
	}
	wc_Sha256Update(&sha256_ctx, img->fw_base, img->fw_size);
	Apply the same fix to image_sha384 and image_sha3_384.

[dgarske]

🟡 [Medium] Missing NULL and bounds checks in update_hash_flash_fwimg ONESHOT path
💡 SUGGEST bug

The ONESHOT path in update_hash_flash_fwimg directly dereferences img->fw_base + offset without any validation:

1. No NULL check on img->fw_base (unlike the image_sha* ONESHOT paths which check fw_base == NULL)
2. No bounds check on offset + size vs img->fw_size (the non-ONESHOT path gets this via read_flash_fwimage which checks at line 1716)

The callers in wolfBoot_check_flash_image_elf compute offset/size from parsed ELF headers, so a malformed ELF could cause out-of-bounds reads. The non-ONESHOT path has defense-in-depth with bounds checking in read_flash_fwimage.

Suggestion:

Suggested change

	#ifdef WOLFBOOT_IMG_HASH_ONESHOT
	#ifdef WOLFBOOT_IMG_HASH_ONESHOT
	if (img->fw_base == NULL)
	return -1;
	if ((uint64_t)offset + size > img->fw_size)
	return -1;
	update_hash(ctx, img->fw_base + offset, size);
	return 0;

[dgarske]

🟡 [Medium] update_hash_flash_addr ONESHOT path silently ignores src_ext flag
💡 SUGGEST bug

The ONESHOT path in update_hash_flash_addr casts addr to uint8_t* and passes it directly to update_hash, voiding the src_ext parameter. If WOLFBOOT_IMG_HASH_ONESHOT=1 is accidentally combined with an EXT_FLASH configuration (which the PR description acknowledges is logically incompatible but deliberately not compile-time guarded), this function will attempt to dereference an external flash address as if it were memory-mapped, leading to incorrect reads or a crash.

The caller at line 1988-1989 passes PART_IS_EXT(&boot) as src_ext. A runtime assertion or at least a debug warning would prevent silent misuse.

Suggestion:

Suggested change

	#ifdef WOLFBOOT_IMG_HASH_ONESHOT
	Consider adding a runtime check:
	#ifdef WOLFBOOT_IMG_HASH_ONESHOT
	if (src_ext) {
	wolfBoot_printf("ERROR: ONESHOT hash incompatible with external flash\n");
	return -1;
	}
	update_hash(ctx, (uint8_t*)addr, size);
	return 0;

[dgarske]

🔵 [Low] Behavioral difference between ONESHOT and non-ONESHOT when end_sha <= p
🔧 NIT convention

In the non-ONESHOT path, when end_sha <= p (malformed/empty header region), the while (p < end_sha) loop simply doesn't execute and the function returns 0, proceeding to hash the firmware body. In the ONESHOT path, the same condition causes return -1, failing the entire verification.

While the ONESHOT behavior is arguably more correct (this condition indicates a corrupt header), the inconsistency means the same malformed image would pass verification without ONESHOT but fail with ONESHOT enabled. This could cause confusion during testing or field deployment when switching between the two modes.

Suggestion:

Suggested change

	#ifdef WOLFBOOT_IMG_HASH_ONESHOT
	For consistency, consider adding the same guard to the non-ONESHOT path, or documenting this behavioral difference.