Fix short PSBT uploads at firmware probe boundary
What the developer wrote
Authored by scgbckbone
Fix short PSBT uploads at firmware probe boundary
What changed, and why it matters
This update fixes a bug in the COLDCARD hardware wallet where short PSBT (Partially Signed Bitcoin Transaction) files could be mistaken for partial firmware uploads when their size landed near a specific firmware-probe boundary. The fix adds a length check so the device only treats a chunk as a firmware header probe when the incoming data block is exactly 256 bytes long. If the bug were triggered, a legitimate PSBT upload might be misclassified, potentially causing the upload to fail or behave unexpectedly. There is no evidence in the commit of malicious exploitation or a security bypass.
Treat as a routine bugfix. Users who upload PSBTs over USB should update to a release containing this commit once available. No immediate incident-response action is indicated by the supplied materials.
Security signals we found
Incorrect file-type classification of user-supplied data
Boundary-condition bug in upload parser
Potential denial-of-service or unexpected failure of PSBT signing workflow
No memory-safety or cryptographic weakness visible in the diff
Evidence from the diff
In shared/usb.py, handle_upload() inspects incoming file uploads to decide whether the data is a firmware header. It previously checked only whether the current offset pos matched FW_HEADER_OFFSET & ~255 (a 256-byte-aligned boundary near the firmware header offset). A short PSBT whose total size caused the final chunk to land at that boundary could be misinterpreted as a firmware header probe. The patch adds an additional condition len(here) == 256, so the probe logic only fires when a full 256-byte chunk is present. A regression test is added for PSBT lengths 0x3f01, 0x3f02, and 0x3f03 bytes, which straddle that boundary.
Changed components
shared/usb.py: handle_upload()COLDCARD USB file upload protocolPSBT transaction signing workflowInspect captured patch +6 / −1
### releases/Next-ChangeLog.md
@@ -13,6 +13,7 @@ This lists the new changes that have not yet been published in a normal release.
- Bugfix: Prevent duplicate WIF Store entries after restarting
- Change: Block `SIGHASH_SINGLE` and `SIGHASH_SINGLE|ANYONECANPAY` by default because they can leave later transaction outputs modifiable after signing. They remain available when Sighash Checks is set to Warn.
Thanks to [@instagibbs](https://github.com/instagibbs) for reporting this issue.
+- Bugfix: Fixed PSBT uploads being mistaken for partial firmware uploads.
# Mk Specific Changes
### shared/usb.py
@@ -821,7 +821,7 @@ async def handle_upload(self, offset, total_size, data):
# length and appends hdr, but that's kinda a bug, so support both
is_trailer = (pos == (total_size - FW_HEADER_SIZE) or pos == total_size)
- if pos == (FW_HEADER_OFFSET & ~255):
+ if pos == (FW_HEADER_OFFSET & ~255) and len(here) == 256:
hdr = memoryview(here)[-128:]
magic, = unpack_from('<I', hdr[0:4])
if magic == FW_HEADER_MAGIC:
### testing/test_usb.py
@@ -148,6 +148,10 @@ def test_upload_long(dev, pkt_len, count=5, data=None):
# clear screen / test a degerate case
dev.send_recv(CCProtocolPacker.upload(256, 256, b''))
+@pytest.mark.parametrize('data_len', [0x3f01, 0x3f02, 0x3f03])
+def test_upload_psbt_at_firmware_probe_boundary(dev, data_len):
+ dev.upload_file(b'psbt\xff' + bytes(data_len - 5))
+
def test_upload_fails(dev):
# incorrect file upload cases
Why this scored 44/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.