bugfix: report staged firmware modification at upgrade approval
What the developer wrote
Authored by scgbckbone
bugfix: report staged firmware modification at upgrade approval
- FirmwareUpgradeRequest: fail with "Firmware modified" story instead of
bare assert when PSRAM write counter moved after staging
- main_pin_changer: assert no temporary wallet is active (menu already
hides the item; defense in depth)
- test_upgrades: assert tamper abort shows the failure message
- test_ccc: drop stale CCCFeature.last_fail_reason resets
What changed, and why it matters
This commit fixes a bug in the COLDCARD firmware upgrade process. Previously, if someone tampered with the staged firmware after the user saw the approval prompt but before they confirmed, the device would crash with a bare internal error (assert). Now it cleanly aborts and tells the user 'Firmware modified'. The commit also adds a small extra safety check in the main PIN change flow to ensure no temporary wallet is active, and updates tests accordingly.
Treat as a security-hardening bugfix worth including in release notes. No immediate emergency response is indicated, but verify that the new 'Firmware modified' failure path is reachable and correctly displayed on real hardware during firmware upgrade approval.
Security signals we found
Tamper-detection failure path hardened: assert replaced with user-facing failure story
Defense-in-depth assertion added to PIN-changing flow
Test coverage added for tamper-abort message
Evidence from the diff
In shared/auth.py, FirmwareUpgradeRequest.interact() replaces an assert that glob.PSRAM.psram_write_count equals the value captured when the upgrade story was shown with an explicit check that returns await self.failure(‘Firmware modified’). This prevents a bare AssertionError if the staged PSRAM firmware bytes are modified between prompt and approval. In shared/actions.py, main_pin_changer() adds assert not pa.tmp_value as defense-in-depth, since temporary wallets have no main PIN and the menu already hides the option. Tests are updated to assert the new failure message and to remove stale CCCFeature.last_fail_reason resets.
Changed components
shared/auth.py: FirmwareUpgradeRequest.interact()shared/actions.py: main_pin_changer()testing/test_upgrades.pytesting/test_ccc.pyInspect captured patch +6 / −6
### shared/actions.py
@@ -2089,6 +2089,7 @@ async def main_pin_changer(*a):
# - the bootloader maybe lying to us about main vs trick pin
# - what may look like just policy here, is in fact enforced by the bootrom code
#
+ assert not pa.tmp_value # menu already hides this; tmp wallet has no main PIN
from glob import dis
from login import LoginUX
from pincodes import BootloaderError, EPIN_OLD_AUTH_FAIL
### shared/auth.py
@@ -1609,7 +1609,9 @@ async def interact(self):
ch = await ux_show_story(msg)
if ch == 'y':
- assert glob.PSRAM.psram_write_count == self.psram_write_count
+ if glob.PSRAM.psram_write_count != self.psram_write_count:
+ # staged bytes changed since approval prompt was shown
+ return await self.failure("Firmware modified")
# Accepted:
# - write final file header, so bootloader will see it
### testing/test_ccc.py
@@ -1010,9 +1010,6 @@ def test_maxed_out(settings_set, setup_ccc, enter_enabled_ccc, ccc_ms_setup, sim
import_multisig(data=ms_conf)
press_select() # confirm multisig import
- # get rid of last violation - as it is held as global
- sim_exec('from ccc import CCCFeature; CCCFeature.last_fail_reason=""')
-
# sign with B (B does not have ccc in settings so CC is unaware that part of CCC is signing)
policy_sign(bitcoind_wo, base64.b64encode(part_psbt).decode()) # no violations
restore_main_seed()
@@ -1135,8 +1132,6 @@ def test_load_and_sign_key_C(settings_set, setup_ccc, enter_enabled_ccc, ccc_ms_
import_multisig(data=ms_conf)
press_select() # confirm multisig import
- # get rid of last violation - as it is held as global
- sim_exec('from ccc import CCCFeature; CCCFeature.last_fail_reason=""')
# no violations ccc not in C settings
policy_sign(bitcoind_wo, base64.b64encode(part_psbt).decode())
restore_main_seed(seed_vault=seed_vault)
### testing/test_upgrades.py
@@ -169,6 +169,8 @@ def test_upgrade_staged_image_tamper(make_firmware, upload_file, cap_story,
press_select()
time.sleep(1)
assert sim_eval("glob._fw_upgrade_called") == 'False'
+ _, story = cap_story()
+ assert "Firmware modified" in story
finally:
sim_exec("from pincodes import pa; import glob; "
"pa.firmware_upgrade = glob._fw_upgrade")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.