Bound microSD message read at read time, not via separate stat
What the developer wrote
Authored by Jack
Bound microSD message read at read time, not via separate stat
A separate stat in its own CardSlot() left a TOCTOU window where a swapped file could still be read unbounded. Move the cap into a bounded read_fn inside the read.
What changed, and why it matters
This commit fixes a security bug in the Passport hardware wallet's microSD message-signing flow. Previously, the firmware checked a file's size in one operation and then read it in another. Because the microSD card could be swapped between those two moments, a malicious file larger than the allowed limit could still be loaded. The fix reads only a bounded number of bytes in a single operation, closing that window.
Treat this as a security fix and include it in the next firmware release. Review other flows that stat-then-read files from microSD for similar TOCTOU issues. Consider whether ReadFileFlow should offer a built-in max_bytes option to make bounded reads the default.
Security signals we found
TOCTOU race condition between file-size stat and file read
Bypass of MSG_SIGNING_MAX_LENGTH length limit via microSD swap
Unbounded read of attacker-controlled file from removable media
Fix enforces bound at read time inside a single CardSlot() lifetime
Evidence from the diff
The patch removes a separate os.stat() call inside its own CardSlot() context and instead passes a bounded read_fn to ReadFileFlow so the size cap is enforced during the actual read within one CardSlot() lifetime. This eliminates a TOCTOU (time-of-check/time-of-use) race where an attacker who swaps the microSD card or file between stat and read could bypass MSG_SIGNING_MAX_LENGTH. The patch also moves UTF-8 decoding into the bounded path and handles decode failures explicitly.
Changed components
ports/stm32/boards/Passport/modules/flows/health_check_microsd_flow.pyHealthCheckMicrosdFlow.parse_message()ReadFileFlow integration for microSD message signingInspect captured patch +22 / −14
diff --git a/ports/stm32/boards/Passport/modules/flows/health_check_microsd_flow.py b/ports/stm32/boards/Passport/modules/flows/health_check_microsd_flow.py
index 4de06c8..d2b40a8 100644
--- a/ports/stm32/boards/Passport/modules/flows/health_check_microsd_flow.py
+++ b/ports/stm32/boards/Passport/modules/flows/health_check_microsd_flow.py
@@ -53,34 +53,42 @@ class HealthCheckMicrosdFlow(Flow):
async def parse_message(self):
from flows import ReadFileFlow
- # Cap file size for general message signing. Health-check protocol
- # files have implicit length constraints and are exempted.
+ # Bound the read inside one CardSlot() lifetime; a separate pre-stat
+ # would leave a TOCTOU window if the card/file is swapped mid-flow.
if self.normal_signing:
- import os
- from files import CardSlot, CardMissingError
from pages import ErrorPage
from public_constants import MSG_SIGNING_MAX_LENGTH
- try:
- with CardSlot() as card:
- size = os.stat(self.file_path)[6]
- except CardMissingError:
- await ErrorPage('microSD card removed.').show()
+ cap = MSG_SIGNING_MAX_LENGTH + 1
+
+ def bounded_read(fd):
+ return fd.read(cap)
+
+ raw = await ReadFileFlow(self.file_path, binary=True, read_fn=bounded_read).run()
+
+ if not raw:
self.set_result(False)
return
- if size > MSG_SIGNING_MAX_LENGTH:
+ if len(raw) > MSG_SIGNING_MAX_LENGTH:
await ErrorPage(
'Message file is too long. Max length is {} bytes.'.format(MSG_SIGNING_MAX_LENGTH)
).show()
self.set_result(False)
return
- data = await ReadFileFlow(self.file_path, binary=False).run()
+ try:
+ data = raw.decode('utf-8')
+ except (UnicodeError, AttributeError):
+ await ErrorPage('Message file contains invalid text.').show()
+ self.set_result(False)
+ return
+ else:
+ data = await ReadFileFlow(self.file_path, binary=False).run()
- if not data:
- self.set_result(False)
- return
+ if not data:
+ self.set_result(False)
+ return
self.lines = data.split('\n')
self.goto(self.common_flow)
Why this scored 59/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.