Fenrir fixes 2026 06 11#795
Merged
mattia-moffa merged 23 commits intoJun 11, 2026
Merged
Conversation
…Boot_start update_flash_hwswap.c stored the firmware versions into int locals via (int)wolfBoot_current_firmware_version() / (int)wolfBoot_update_firmware_version(). Under WOLFBOOT_FIXED_PARTITIONS those macros resolve to wolfBoot_get_image_version(), which returns uint32_t. Versions >= 0x80000000 became negative when cast to int, failed the ">= 0" clamp, and left boot_v/update_v at 0. With both partition versions above INT_MAX, max_v collapsed to 0 and the rollback guard "(max_v > 0U) && (active_v < max_v)" was silently skipped, allowing a rolled-back image to be staged. This was a regression introduced in 90670fc; the original code used uint32_t directly. wolfBoot_get_blob_version() returns uint32_t with 0 as its only "invalid" value, so the negative-clamp logic was both unnecessary and harmful. Restore the direct uint32_t reads. Add tools/unit-tests/unit-update-flash-hwswap.c which drives wolfBoot_start with BOOT (v0x80000002, unbootable) and UPDATE (v0x80000001, valid): the boot path falls back to UPDATE and must deny the downgrade. The test fails before the fix (do_boot reached) and passes after.
…wolfBoot_start update_ram.c stored the firmware versions into int locals via (int)wolfBoot_current_firmware_version() / (int)wolfBoot_update_firmware_version(). Under WOLFBOOT_FIXED_PARTITIONS those macros resolve to wolfBoot_get_image_version(), which returns uint32_t. Versions >= 0x80000000 became negative when cast to int, failed the ">= 0" clamp, and left boot_v/update_v at 0. With both partition versions above INT_MAX, max_v collapsed to 0 and the rollback guard "(max_v > 0U) && (active_v < max_v)" was silently skipped, allowing a rolled-back image to be staged after a fallback. Same fundamental pattern as F-4411 (hwswap) and the documented 3736 (update_disk). wolfBoot_get_image_version() returns uint32_t with 0 as its only "invalid" value, so the negative-clamp logic was both unnecessary and harmful. Use the direct uint32_t reads. Add tools/unit-tests/unit-update-ram-noramboot.c, a WOLFBOOT_NO_RAMBOOT build of wolfBoot_start (the configuration that uses wolfBoot_open_image() and therefore actually reaches the rollback guard). The rollback test drives BOOT (v0x80000005, marked oversize so its open is rejected) and UPDATE (v0x80000003, valid): the boot path falls back to UPDATE and must deny the downgrade. The test fails before the fix (UPDATE staged, do_boot reached) and passes after. The existing unit-update-ram target uses the RAMBOOT path, where wolfBoot_ramboot() has its own separate int-cast of the version that rejects all high-version images before the guard is reached; that path cannot exercise this guard and is left unchanged.
The firmware encryption key and nonce were left in the NVM_CACHE_SIZE stack buffer after hal_flash_write returned, until the frame was naturally overwritten. Add ForceZero guarded by the same preprocessor condition that declares the stack-local ENCRYPT_CACHE.
…ters All four PS dispatch arms (PS_SET, PS_GET, PS_GET_INFO, PS_REMOVE) cast in_vec[N].base to a typed pointer but only checked for NULL, not that the caller-supplied .len covers the target type. An NS caller could pass len=0 (or len < sizeof) with a valid non-NULL base, causing the secure side to read past the declared buffer. Mirror the pattern already used in wolfboot_crypto_dispatch (F-3541) and the attestation handler.
…idate_addr Equal-version and newer-update cases were untested, allowing mutations of the comparator or the addr assignment at libwolfboot.c:1446-1448 to survive. Add two tests to unit-update-ram-nofixed.c that directly verify retval and *addr for boot_v==update_v (must prefer primary) and update_v>boot_v (must prefer update partition).
The --custom-tlv TAG LEN VAL path in make_header_ex() passed &CMD.custom_tlv[i].val (a raw uint64_t pointer) directly to header_append_tag(), which does a memcpy. On big-endian build hosts the first LEN bytes of the uint64_t are the high bytes, producing the wrong LE encoding (e.g. 4-byte value 0x12345678 encodes as 00 00 00 00). The fix serialises through header_store_u64_le() before calling header_append_tag(), matching the pattern already used by header_append_tag_u16/u32/u64 for all system TLVs. Add unit-sign-custom-tlv-le.py to verify the LE byte encoding of 4-byte and 8-byte custom TLV values in the signed image header.
Mirror the DISABLE_BACKUP treatment: print a $(warning ...) so the build operator gets an unmistakable signal that anti-rollback enforcement has been disabled.
…rtial page end_address = address + len - 1 (inclusive) combined with p < end_address (strict less-than) skipped the last page when len % FLASH_PAGE_SIZE == 1. Use the exclusive form end_address = address + len, mirroring the stm32h5 fix. Add WOLFBOOT_UNIT_TEST_FLASH_ERASE guards and a new unit test that proves the regression before the fix and passes after.
…rtial page end_address was computed as address + len - 1 (inclusive) but paired with a strict p < end_address guard, so the last page was skipped whenever len was not a multiple of FLASH_PAGE_SIZE. Change end_address to the exclusive form address + len so the loop covers all pages that overlap the requested range. Add unit-flash-erase-l0 regression test (mirrors the unit-flash-erase-wb harness) with guards in hal/stm32l0.c allowing the HAL to be compiled on the host; the unaligned-len test fails before the fix and passes after.
…rtial page end_address was computed as address + len - 1 (inclusive) but the loop used strict less-than, so the final page was skipped when len was not a multiple of FLASH_PAGE_SIZE. Use an exclusive end (address + len) to match the fix applied to stm32h5.c, stm32wb.c, and stm32l0.c. Add WOLFBOOT_UNIT_TEST_FLASH_ERASE guard to hal/stm32g0.c and a new unit test (unit-flash-erase-g0) that proves the regression and confirms the fix.
…rtial page end_address = address + len - 1 (inclusive) with p < end_address (strict) skips the final page when len is not a multiple of FLASH_PAGE_SIZE. Change to exclusive end_address = address + len, matching the fix applied to stm32g0/l0/wb. Add WOLFBOOT_UNIT_TEST_FLASH_ERASE guards so the function can be included in isolation on the host, and add unit-flash-erase-c0 to the test suite.
…rtial page end_address was computed as address + len - 1 (inclusive last byte) but the loop used strict p < end_address, so a non-page-aligned len caused the final page to be skipped. Change to end_address = address + len (exclusive) to match stm32u5.c, the claimed reference implementation. Add WOLFBOOT_UNIT_TEST_FLASH_ERASE guards to hal/stm32u3.c and hal/stm32u3.h (DMB/ISB/DSB, FLASH_NS_CR/SR register macros, and all non-erase functions) so that hal_flash_erase() can be compiled and tested in isolation on the host. Add tools/unit-tests/unit-flash-erase-u3.c with four check-based tests; the regression case (len = PAGE_SIZE + 1) failed before this fix.
Add hal_secret_zeroize calls for the digest buffer and SHA context after memcpy in uds_from_uid, matching the existing pattern applied to the OTP path in hal_uds_derive_key. Also moves/adds hal_secret_zeroize definition before uds_from_uid so it is available at the call site.
…hort policy The guard `policySz <= 0` was dead for unsigned uint16_t, so values 1-3 passed. The subsequent `policySz -= sizeof(pcrMask)` (4) then wrapped to 65533-65535, passing a garbage length to wolfTPM2_VerifyHashTicket. Replace the guard with `policySz < sizeof(pcrMask)` to reject policies too short to hold a pcrMask.
…ot_start Versions with the high bit set (>= 0x80000000) were stored as negative int, causing the `> 0` guards to fail silently and leaving pA_ver_u / pB_ver_u at 0. This broke partition selection and disabled the rollback check for any image with a version number above INT_MAX. Declare pA_ver and pB_ver as uint32_t (matching the return type of wolfBoot_get_blob_version and get_decrypted_blob_version), update the guards to != 0U, and drop the now-redundant (uint32_t) casts. Add a unit test that places 0x80000001 in A and 0x80000002 in B and asserts B is selected; before the fix the test failed with A chosen.
boot_addr is uint32_t*, so (boot_addr + *size) advanced *size * 4 bytes instead of *size bytes, overstating the MEMMAP_DEVICE_PATH range by 4x relative to the SourceSize passed to LoadImage. Cast to uint8_t* first so the addition is byte-accurate and consistent with the SourceSize argument. No unit test: compiling hal/x86_64_efi.c in a host test requires stubbing the complete UEFI SDK type tree (EFI_FILE_IO_INTERFACE, EFI_LOADED_IMAGE, EFI_GUID, CHAR16, LibFileInfo, FreePool, InitializeLib, …) throughout the file — disproportionate scaffolding for a one-line arithmetic fix.
…on assembly Shifting a uint8_t (int-promoted) left by 24 is undefined behaviour per C11 6.5.7p4 when the byte value is >= 0x80. Cast the MSB operand to uint32_t at both sites (CMD_APP_VER line 391, CMD_HDR_VER line 412). Note: the report's endian-mismatch claim is incorrect. CMD_APP_VER senders (app_nrf52.c, app_stm32f4.c, app_stm32wb.c) all transmit version bytes MSB-first; CMD_HDR_VER is sent LSB-first by uart_send_current_version. The two decode expressions are intentionally asymmetric and both reconstruct the correct version value.
…A verify Under WOLFBOOT_ENABLE_WOLFHSM_CLIENT/SERVER, wc_InitRsaKey_ex at line 471 registers state with the wolfHSM subsystem, but four early-return paths in wolfBoot_verify_signature_rsa_common bypassed the wc_FreeRsaKey at line 571: RsaSetKeyId failure on the PUBKEY_ID path, KeyCache failure, RsaSetKeyId failure on the non-PUBKEY_ID path, and KeyEvict failure post-verify. Add wc_FreeRsaKey before each of these returns.
…public device UID Add a $(warning ...) in options.mk matching the existing pattern used for ALLOW_DOWNGRADE and DISABLE_BACKUP, so operators are alerted at build time that UDS (and all derived DICE/PSA secrets) is being rooted in the publicly-readable STM32 factory UID rather than OTP fuses.
WREN and address-phase return codes were silently overwritten by subsequent assignments; a failed WREN would let the write proceed with the WEL bit unset, causing a silently dropped write while returning hal_status_ok. Add early-return checks after each transmit phase in FRAM_Write and guard the subsequent calls in FRAM_Init.
…e_b) Adds test_wb_patch_and_diff_shrinking_update to cover the previously untested case where the target image is smaller than the base image (3077 -> 2048 bytes), exercising distinct wb_diff loop paths when off_b reaches size_b before size_a is exhausted.
…sh_otp_keystore Cover the previously untested mutation points in keystore_num_pubkeys(): bad magic returns 0, item_count > KEYSTORE_MAX_PUBKEYS returns 0, item_count == KEYSTORE_MAX_PUBKEYS is accepted (guards > vs >= mutation), and slot read failure propagates through all keystore_get_* accessors.
In hal/va416x0.c, FRAM_Write now explicitly aborts the split SPI write transaction on command-phase failure before returning, instead of leaving the bus in the half-open state introduced by the early return. I also added a host-side regression test in tools/unit-tests/unit-va416x0-fram.c that injects a command-phase HAL_Spi_Transmit(..., false) failure and verifies a later write still reaches the closing ...true phase. In src/arm_tee_psa_ipc.c, I extracted the protected-storage dispatch path into a small helper so it can be tested directly without changing runtime behavior. The new test in tools/unit- tests/unit-arm-tee-psa-ipc.c covers the short-vector invalid-argument branches for SET, GET, GET_INFO, and REMOVE, plus a full success path across those operations. The unit harness additions are wired up in tools/unit-tests/Makefile and use a tiny local CMSE stub in tools/unit-tests/arm_cmse.h.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #795
Scan targets checked: wolfboot-bugs, wolfboot-src
No new issues found in the changed files. ✅
mattia-moffa
approved these changes
Jun 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix remaining low/info level findings.
809b98d F-3734: add unit tests for magic check and item_count boundary in flash_otp_keystore
405a142 F-3740: add roundtrip test for shrinking firmware delta (size_a > size_b)
333055d F-3967: propagate HAL_Spi_Transmit errors in FRAM_Write and FRAM_Init
e9b70fe F-3969: warn when WOLFBOOT_UDS_UID_FALLBACK_FORTEST=1 weakens UDS to public device UID
f600fe9 F-4968: fix wc_FreeRsaKey missing on early-return paths in wolfHSM RSA verify
524397d F-5675: fix signed-overflow UB from uint8_t<<24 in serve_update version assembly
5cec621 F-3735: fix EndingAddress uint32_t* arithmetic in x86_64_efi_do_boot
2365e45 F-3736: fix pA_ver/pB_ver int/uint32_t mismatch in update_disk wolfBoot_start
ddf05a9 F-3738: fix policySz uint16_t underflow in wolfBoot_unseal_blob for short policy
a10a993 F-3741: zeroize digest and hash in uds_from_uid for stm32l5/stm32h5
ef85a1e F-3962: fix hal_flash_erase loop bound in stm32u3.c skipping final partial page
4b95dc2 F-3963: fix hal_flash_erase loop bound in stm32c0.c skipping final partial page
2f8f0c6 F-3964: fix hal_flash_erase loop bound in stm32g0.c skipping final partial page
15e4b81 F-3965: fix hal_flash_erase loop bound in stm32l0.c skipping final partial page
2f53d0d F-3966: fix hal_flash_erase loop bound in stm32wb.c skipping final partial page
889788d F-3968: emit build-time warning when ALLOW_DOWNGRADE=1 is set
1690522 F-3970: serialise custom TLV integer values through header_store_u64_le
61dbd25 F-3972: add direction-sensitive unit tests for wolfBoot_dualboot_candidate_addr
108a33c F-3973: validate in_vec[N].len before dereferencing typed PS IPC pointers
e81f262 F-3974: zero ENCRYPT_CACHE stack buffer before return in hal_set_key
1016c4d F-4410: fix uint32_t->int cast bypassing anti-rollback in update_ram wolfBoot_start
2ee4108 F-4411: fix uint32_t->int cast bypassing anti-rollback in hwswap wolfBoot_start