bugfix: vdisk import bumps txn write count
What the developer wrote
Authored by scgbckbone
bugfix: vdisk import bumps txn write count
The C VirtDisk importer memcpy's into the PSRAM TXN staging regions without going through PSRAMWrapper.write_at(), so txn_write_count is not bumped and the pre-signing re-hash takes its fast path over possibly-clobbered bytes.
Bump the counter and revoke any PSRAM lease before copy_file, so post-review tampering through the virtual disk is caught and stale staged bytes cannot be downloaded.
Make the simulator model copy_file as a direct PSRAM write, then exercise the real VirtDisk event path during a pending PSBT approval and verify signing aborts.
What changed, and why it matters
This commit fixes a bug in the COLDCARD hardware wallet's virtual disk (VirtDisk) feature. When a user imported a file via the virtual disk, the firmware's fast C importer copied data directly into a sensitive memory area used to stage pending Bitcoin transactions. Because it bypassed the normal tracking mechanism, the wallet did not realize the transaction data had been touched. An attacker or malicious program with virtual disk access could therefore overwrite a transaction you were about to approve, and the wallet might sign the altered version instead of the one you reviewed on screen. The fix records the write, revokes any pending download lease, and adds a test proving the wallet now aborts signing when this happens.
Treat this as a security fix and include it in the next firmware release. Ensure the regression test passes on both simulator and real hardware. Review other PSRAM consumers for similar direct-write bypasses of txn_write_count or ALLOWED_DOWNLOAD invalidation. No CVE or advisory is supplied; consider requesting one if the vendor confirms security relevance.
Security signals we found
Bypass of transaction integrity counter (txn_write_count) via direct memory copy
Post-review tampering of staged signing data through virtual disk import
Missing invalidation of staged download lease (ALLOWED_DOWNLOAD)
Pre-signing hash fast-path operating on potentially clobbered bytes
Addition of regression test for transaction-modification abort path
Evidence from the diff
The PSRAM transaction staging region tracks writes via PSRAMWrapper.write_at() and a txn_write_count counter. The C-based VirtDisk importer (VBLKDEV.copy_file) performs a direct memcpy into that region, so txn_write_count was not incremented and ALLOWED_DOWNLOAD was not cleared. As a result, a post-review, pre-sign virtual disk import could clobber staged PSBT/transaction bytes without invalidating the cached hash, allowing the fast re-hash path to operate over modified data. The patch increments txn_write_count and sets glob.ALLOWED_DOWNLOAD = None before copy_file. It also updates the simulator to model copy_file as a direct PSRAM write (matching real hardware) and adds test_virtdisk_import_invalidates_pending_psbt to verify that signing aborts with ‘Transaction modified’.
Changed components
shared/vdisk.pyunix/variant/sim_vdisk.pytesting/test_vdisk.pyCOLDCARD VirtDisk / PSRAM transaction staging subsystemInspect captured patch +59 / −11
### shared/vdisk.py
@@ -103,6 +103,12 @@ def import_file(self, filename, sz):
# copy file into another area of PSRAM where rest of system can use it
assert sz <= MAX_UPLOAD_LEN # too big
+ # C importer memcpy's into the TXN staging regions without going
+ # through the PSRAM wrapper — record it, and revoke any lease of
+ # those staged bytes, so post-review tampering can't hide
+ glob.ALLOWED_DOWNLOAD = None
+ glob.PSRAM.txn_write_count += 1
+
# I could not resist doing this in C... since we already have the
# data in memory, why mess around with file concepts?
actual = VBLKDEV.copy_file(0, filename.split('/')[-1])
### testing/test_vdisk.py
@@ -9,6 +9,7 @@
from hashlib import sha256
from txn import *
from base64 import b64encode, b64decode, encodebytes
+from ckcc_protocol.protocol import CCProtocolPacker, CCProtoError
def test_vd_basics(dev, virtdisk_path, is_simulator):
@@ -212,6 +213,46 @@ def hack(psbt):
else:
assert _txn == txn
+def test_virtdisk_import_invalidates_pending_psbt(
+ dev, fake_txn, start_sign, cap_story, virtdisk_path,
+ virtdisk_wipe, settings_set, sim_eval, need_keypress, press_cancel):
+ settings_set('vidsk', 2) # enable + auto-consume
+ virtdisk_wipe()
+ time.sleep(.4) # let the vdisk monitor settle
+
+ start_sign(fake_txn(3, 3), finalize=True)
+ for _ in range(100):
+ title, _ = cap_story()
+ if title == 'OK TO SEND?':
+ break
+ time.sleep(.1)
+ else:
+ pytest.fail('no approval screen')
+
+ before = int(sim_eval("__import__('glob').PSRAM.txn_write_count"))
+
+ # A firmware import uses the native PSRAM copy and overwrites the pending
+ # transaction before its own authorization attempt is rejected as busy.
+ with open(virtdisk_path('replacement.dfu'), 'wb') as f:
+ f.write(bytes(0x50000))
+
+ for _ in range(50):
+ if int(sim_eval("__import__('glob').PSRAM.txn_write_count")) != before:
+ break
+ time.sleep(.1)
+ else:
+ pytest.fail('VirtDisk import did not invalidate staged PSRAM')
+
+ need_keypress('y')
+ with pytest.raises(CCProtoError, match='Transaction modified'):
+ while True:
+ time.sleep(.1)
+ done = dev.send_recv(CCProtocolPacker.get_signed_txn(), timeout=None)
+ if done is not None:
+ break
+
+ press_cancel()
+
def test_virtdisk_oversized_psbt_rejected(press_select, virtdisk_path, cap_story, virtdisk_wipe,
press_cancel, goto_home, sd_cards_eject,
settings_set, sim_exec):
### unix/variant/sim_vdisk.py
@@ -29,6 +29,18 @@ def set_inserted(self, en):
def wipe(self):
print("sim-virtdisk: wipe (not implemented)")
+ def copy_file(self, offset, filename):
+ # Match the native implementation: copy directly into the lower
+ # PSRAM mapping without going through PSRAMWrapper.write_at().
+ print("sim-virtdisk: read %s" % filename)
+ with open(SIMDIR_PATH + filename, 'rb') as f:
+ contents = f.read()
+
+ from glob import PSRAM
+ assert offset + len(contents) <= PSRAM.length
+ PSRAM._wr[offset:offset+len(contents)] = contents
+ return len(contents)
+
@classmethod
async def monitor_task(cls, self):
# works, but hard to manage the atask
@@ -72,17 +84,6 @@ def unmount(self, written_files, readonly=False):
for fn in written_files:
self.ignore.add(fn.split('/')[-1])
- def import_file(self, filename, sz):
- # copy file into another area of PSRAM where rest of system can use it
- print("sim-virtdisk: read %s" % filename)
- with open(filename, 'rb') as f:
- contents = f.read(sz)
- from glob import PSRAM
- runt = (4 - sz % 4)
- sz = sz + runt
- PSRAM.write_at(0, sz)[:] = contents + bytes(runt)
- return sz
-
vdisk.VirtDisk = SimulatedVirtDisk
# EOFWhy this scored 67/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.