Fenrir fixes#762
Merged
dgarske merged 25 commits intowolfSSL:masterfrom Apr 29, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Batch of bootloader hardening fixes and regression tests across flash/disk update paths, crypto/key handling, and GPT parsing.
Changes:
- Tighten failure handling and post-hook verification in flash update/boot paths (delta-update cleanup, final sanity check after boot hook, UART flash read timeout error).
- Harden sensitive comparisons and buffer handling (volatile accumulators for constant-time checks, key buffer zeroization, bounded TPM fallback copies, unaligned-safe TLV parsing).
- Add/extend unit tests for new guards and edge cases (disk A/B blank-partition boot, GPT partition-array CRC validation, QSPI WE failure abort, delta match length cap, multiboot header underflow, XMSS param fallback).
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/xmss/xmss_common.h | Zeroizes XMSS key readback buffer before free during write verification. |
| tools/lms/lms_common.h | Zeroizes LMS key verify buffer on multiple failure/success paths. |
| tools/keytools/keygen.c | Fixes XMSS parameter fallback logic when env var is unset. |
| src/x86/ahci.c | Hardens local constant comparison by using volatile accumulator. |
| src/update_flash.c | Ensures delta-update early errors go through cleanup; adds final sanity check after boot hook. |
| src/update_disk.c | Makes decrypted blob TLV parsing alignment-safe and padding-aware. |
| src/uart_flash.c | Returns error on UART flash read timeout instead of success-like 0. |
| src/tpm.c | Bounds “Unknown” fallback writes to caller-provided buffers. |
| src/qspi_flash.c | Aborts page-write loop immediately after write-enable failure. |
| src/multiboot.c | Guards mb2 header dump against header_length underflow and returns early. |
| src/libwolfboot.c | Hardens encrypted-key checks with volatile accumulators. |
| src/gpt.c | Exposes incremental CRC32 API and uses it for GPT header CRC validation. |
| src/disk.c | Validates GPT partition-entry array CRC before parsing entries. |
| src/delta.c | Caps match extension at UINT16_MAX to bound encoded match length. |
| src/arm_tee_psa_ipc.c | Zeroizes ARM TEE PS buffers on reuse and deletion. |
| include/gpt.h | Adds public CRC32 context + init/update/final API declarations. |
| hal/library.c | Fixes malloc-failure path to still close the opened image file. |
| tools/unit-tests/unit-update-flash.c | Adds hook-mutation panic test; adds inverse-delta version-gate test coverage and refactors setup. |
| tools/unit-tests/unit-update-disk.c | Adds tests for booting from A/B when the other partition is blank. |
| tools/unit-tests/unit-uart-flash.c | New unit test validating UART flash read timeout returns error. |
| tools/unit-tests/unit-tpm-api-names.c | New unit tests ensuring TPM name helper fallbacks are bounded. |
| tools/unit-tests/unit-sectorflags.c | Adds unit coverage ensuring redundant sector-flag writes are avoided. |
| tools/unit-tests/unit-qspi-flash.c | Adds unit test ensuring mid-loop WE failure stops further page programs. |
| tools/unit-tests/unit-multiboot.c | Adds test ensuring mb2_dump_header rejects underflow header_length without dumping tags. |
| tools/unit-tests/unit-keygen-xmss-params.c | New unit tests validating XMSS param env/default selection logic. |
| tools/unit-tests/unit-disk.c | Adds GPT partition-array CRC mismatch rejection test and single-sector partition parsing test. |
| tools/unit-tests/unit-delta.c | Adds test ensuring self-match length is capped to 16-bit maximum. |
| tools/unit-tests/Makefile | Adds new unit-test binaries and build targets (UART flash, hook build, TPM API names, XMSS params). |
Comments suppressed due to low confidence (1)
tools/xmss/xmss_common.h:103
- In
xmss_write_key, the verification file handle (file = fopen(filename, "rb+")) is not closed on the early-return error paths (e.g.,n_read != n_writeandn_cmp != 0). This leaks a FILE descriptor and can also leave the stream unflushed on some platforms.
Consider using a single cleanup path (e.g., goto out) that always fclose(file) when non-NULL, and also zeroizes/frees buff as needed before returning.
if (n_read != n_write) {
fprintf(stderr, "error: read %d, expected %d: %d\n",
(int)n_read, (int)n_write, ferror(file));
wc_ForceZero(buff, privSz);
free(buff);
return WC_XMSS_RC_WRITE_FAIL;
}
n_cmp = XMEMCMP(buff, priv, n_write);
wc_ForceZero(buff, privSz);
free(buff);
buff = NULL;
if (n_cmp != 0) {
fprintf(stderr, "error: write data was corrupted: %d\n", n_cmp);
return WC_XMSS_RC_WRITE_FAIL;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #762
Scan targets checked: wolfboot-bugs, wolfboot-src
No new issues found in the changed files. ✅
dgarske
requested changes
Apr 29, 2026
dgarske
approved these changes
Apr 29, 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
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.
F/3039 - Return UART flash read error on timeout (
eaa6af2b)F/3298 - Fix delta update cleanup on early errors (
f86ffd8f)F/3038 - Handle library image malloc failure (
b0f1bf9b)F/3040 - Fix multiboot debug header length guard (
bcd75b0a)F/3041 - Harden local secret comparisons (
b6e6dc02)F/3042 - Harden encrypted key comparisons (
1e6dd16c)F/3043 - Add single-partition disk boot tests (
74c0d29c)F/3044 - Cap backward delta match length (
6e8e20ce)F/3045 - Validate GPT partition array CRC (
dfc76560)F/3047 - Zeroize ARM TEE PS buffers on reuse (
8eddbf35)F/3299 - Abort QSPI writes after WE failure (
98d1e772)F/3300 - Bound TPM name fallback copies (
54b2ba34)F/3301 - Fix decrypted blob TLV parsing (
26cae311)F/3302 - Add final sanity check after boot hook (
cc6f52ed)F/3303 - Add inverse delta version gate tests (
c39522ed)F/3304 - Add GPT single-sector partition test (
47ef6b4e)F/3305 - Add sector flag unit coverage (
afa96411)F/3306 - Fix XMSS keygen param fallback (
a60461f1)F/3308 - Zeroize XMSS key readback buffer (
05f5f5cb)F/3309 - Zero LMS key verify buffer (
47e1f77f)