Merge pull request #696 from Foundation-Devices/fix/bootloader-image-bounds
What the developer wrote
Authored by Jacksper13
Merge pull request #696 from Foundation-Devices/fix/bootloader-image-bounds
SFT-8163: bound the declared image length and fail closed on a timestamp read
What changed, and why it matters
This update fixes two related weaknesses in the Passport hardware wallet's bootloader. First, it corrects a size-limit check so a malicious or malformed firmware update cannot claim a firmware length larger than the actual space reserved for firmware (previously the check allowed a value that was 2048 bytes too big, potentially letting a bad update overwrite adjacent memory). Second, it makes the bootloader refuse to install an update if it cannot read the currently installed firmware's timestamp, preventing an attacker or glitch from bypassing the anti-downgrade protection by making the secure element return zero.
Treat this as a security-relevant bootloader patch. Users should install any firmware release containing this commit and avoid installing firmware from untrusted or MicroSD-based recovery sources until updated. Developers should audit other consumers of fwlength to ensure FW_MAX_FWLENGTH is used consistently.
Security signals we found
Bounds-check correction for declared firmware image length
Fail-closed behavior added when secure-element timestamp read returns zero
Prevention of downgrade bypass via zero/tampered timestamp
Header-only length field semantics corrected (fwlength vs total size)
Recovery path now clears partial update from SPI flash on failure
Evidence from the diff
The patch touches three bootloader files. In fwheader.h it introduces FW_MAX_FWLENGTH as FW_MAX_SIZE minus FW_HEADER_SIZE, because hdr->info.fwlength stores only the firmware payload length while consumers add FW_HEADER_SIZE back. verify.c is updated to reject fwlength > FW_MAX_FWLENGTH instead of the looser FW_MAX_SIZE, closing a declared-length overflow window. In main.c the microsd_firmware_recovery() path now treats a secure-element timestamp read of 0 as a fatal error (with UI error and cleanup) rather than allowing the downgrade check to pass, since 0 is less than any valid timestamp. The commit message explicitly calls this ‘fail closed on a timestamp read’ and notes update.c already refused the same condition.
Changed components
ports/stm32/boards/Passport/bootloader/main.cports/stm32/boards/Passport/bootloader/verify.cports/stm32/boards/Passport/include/fwheader.hInspect captured patch +23 / −1
### ports/stm32/boards/Passport/bootloader/main.c
@@ -532,6 +532,21 @@ static void microsd_firmware_recovery(void) {
// Version downgrade check
uint32_t current_firmware_timestamp = se_get_firmware_timestamp(current_board_hash);
+ // A failed read comes back as zero, which no timestamp is less than, so the
+ // check below would pass anything. update.c refuses on the same condition.
+ if (current_firmware_timestamp == 0) {
+ timestamp_error:
+ if (ui_show_error("PASSPORT", "Recovery Error",
+ "Unable to read last firmware timestamp.\n\nFirmware will not be installed.",
+ &ICON_SHUTDOWN, &ICON_CHECKMARK, true) == KEY_RIGHT_SELECT) {
+ clear_update_from_spi_flash(FW_HEADER_SIZE + sd_card_hdr.info.fwlength);
+ return;
+ } else {
+ ui_ask_shutdown();
+ goto timestamp_error;
+ }
+ }
+
if (sd_card_hdr.info.timestamp < current_firmware_timestamp) {
downgrade_error:
if (ui_show_error("PASSPORT", "Recovery Error",
### ports/stm32/boards/Passport/bootloader/verify.c
@@ -42,7 +42,7 @@ secresult verify_header(passport_firmware_header_t* hdr) {
if (hdr->info.timestamp == 0) goto fail;
if (hdr->info.fwversion[0] == 0x0) goto fail;
if (hdr->info.fwlength < FW_HEADER_SIZE) goto fail;
- if (hdr->info.fwlength > FW_MAX_SIZE) goto fail;
+ if (hdr->info.fwlength > FW_MAX_FWLENGTH) goto fail;
// if (hdr->signature.pubkey1 == 0) goto fail;
if ((hdr->signature.pubkey1 != FW_USER_KEY) && (hdr->signature.pubkey1 > FW_MAX_PUB_KEYS)) goto fail;
### ports/stm32/boards/Passport/include/fwheader.h
@@ -10,7 +10,14 @@
#define FW_START (BL_FW_HDR_BASE)
#define FW_HEADER_SIZE 2048
+
+// Total budget for what gets written at FW_START: the header followed by the firmware.
#define FW_MAX_SIZE ((1792 * 1024) - 256)
+
+// `fwlength` in the header counts the firmware only - cosign.c writes it as
+// (file size - FW_HEADER_SIZE), and every consumer adds FW_HEADER_SIZE back to get
+// the total - so the value it may declare has to leave room for the header.
+#define FW_MAX_FWLENGTH (FW_MAX_SIZE - FW_HEADER_SIZE)
#define FW_HEADER_MAGIC 0x50415353
#define FW_HEADER_MAGIC_COLOR 0x53534150
#define FW_HDR ((passport_firmware_header_t*)(FW_START))Why this scored 70/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.