What changed, and why it matters
This commit fixes a PIN-uniqueness check in the COLDCARD's 'Seed Vault / Secure Spending Policy' setup. Previously, when a user picked an unlock PIN, the code only compared it against the main PIN and visible 'trick' PINs, but missed hidden trick PINs. That could let the same PIN be used for two different security features, which can cause confusion or allow a hidden trick PIN to silently override the intended unlock behavior. The patch now asks the secure element (SE2) whether the PIN is already in use anywhere before accepting it.
Treat this as a low-to-moderate security hardening fix. Verify that `tp.get_by_pin()` correctly returns a slot for every stored trick PIN, including hidden, duress, and brick-me PINs, and that the new test `test_use_trick_pin_as_unlock` passes on both Q1 and legacy hardware variants. Consider whether any previously created SSSP configurations could have reused a hidden trick PIN and advise users who rely on trick PINs to review their settings after updating.
Security signals we found
PIN uniqueness check bypassed for hidden trick PINs
Secure-element (SE2) lookup added to enforce global PIN uniqueness
New regression test specifically covers hidden trick PIN collision
Duplicate error message refactored into shared helper
Evidence from the diff
In shared/ccc.py, the SSSP unlock PIN setup previously built a local set of existing PINs from tp.all_tricks() and the main PIN, then accepted the new PIN if it was not in that set. Because all_tricks() apparently does not include hidden trick pins, a hidden trick PIN could be chosen as the SSSP unlock PIN. The patch removes the local set comparison, keeps only the main-PIN check, and then queries tp.get_by_pin(new_pin) (which consults the SE2 trick-PIN slots) to verify uniqueness before calling define_unlock_pin(). It also refactors the duplicate-PIN error message into tp.err_unique_pin(). Tests are updated to cover hidden trick PINs and new trick-PIN menu options.
Changed components
shared/ccc.py (SSSP unlock PIN setup)shared/trick_pins.py (TrickPinMgmt helper and TrickPinMenu edit flow)testing/test_se2.pytesting/test_sssp.pyInspect captured patch +48 / −15
diff --git a/shared/ccc.py b/shared/ccc.py
index be7f2be..546ff8f 100644
--- a/shared/ccc.py
+++ b/shared/ccc.py
@@ -1083,15 +1083,12 @@ disable this feature.
# just a tourist
return
-
# re-use existing PIN if there for some reason
new_pin = tp.has_sp_unlock()
if not new_pin:
- # all existing PINS
- have = set(tp.all_tricks())
- have.add(pa.pin.decode())
-
+ have = tp.all_tricks()
+ main_pin = pa.pin.decode()
while 1:
lll = LoginUX()
lll.is_setting = True
@@ -1101,14 +1098,17 @@ disable this feature.
if new_pin is None:
return
- if (new_pin not in have):
- tp.define_unlock_pin(new_pin)
- break
+ # weak check - does not spot hidden trick pins
+ if (new_pin != main_pin) and (new_pin not in have):
+ # verify uniqueness with SE2
+ b, slot = tp.get_by_pin(new_pin)
+ if slot is None:
+ tp.define_unlock_pin(new_pin)
+ break
- await ux_show_story("That PIN (%s) is already in use. All PIN codes must be unique."
- % new_pin)
+ await tp.err_unique_pin(new_pin)
- # all features disabled to to start
+ # all features disabled to start
settings.set('sssp', dict(en=False, pol={}))
settings.save()
diff --git a/shared/trick_pins.py b/shared/trick_pins.py
index 7a36a38..fa38b20 100644
--- a/shared/trick_pins.py
+++ b/shared/trick_pins.py
@@ -407,6 +407,12 @@ class TrickPinMgmt:
b, slot = tp.update_slot(pin.encode(), new=True,
tc_flags=flags, tc_arg=arg, secret=new_secret)
except: pass
+
+ @staticmethod
+ async def err_unique_pin(pin):
+ # standardized error UX
+ return await ux_show_story(
+ "That PIN (%s) is already in use. All PIN codes must be unique." % pin)
tp = TrickPinMgmt()
@@ -552,8 +558,7 @@ class TrickPinMenu(MenuSystem):
have.remove(existing_pin)
if (new_pin == self.current_pin) or (new_pin in have):
- await ux_show_story("That PIN (%s) is already in use. All PIN codes must be unique." % new_pin)
- return
+ return await tp.err_unique_pin(new_pin)
# check if we "forgot" this pin, and read it back if we did.
# - important this is after the above checks so we don't reveal any trick pin used
diff --git a/testing/test_se2.py b/testing/test_se2.py
index b0c88ca..75f1825 100644
--- a/testing/test_se2.py
+++ b/testing/test_se2.py
@@ -301,8 +301,10 @@ def new_trick_pin(goto_trick_menu, pick_menu_item, cap_menu, press_select,
time.sleep(.1)
m = cap_menu()
assert m[0] == f'[{new_pin}]'
- assert set(m[1:]) == {'Duress Wallet', 'Just Reboot', 'Wipe Seed', \
- 'Delta Mode', 'Look Blank', 'Brick Self', 'Login Countdown'}
+ assert set(m[1:]) == {'Duress Wallet', 'Just Reboot', 'Wipe Seed', 'Delta Mode',
+ 'Look Blank', 'Policy Unlock',
+ 'Policy Unlock & Wipe' if is_q1 else 'P.U. & Wipe',
+ 'Brick Self', 'Login Countdown'}
pick_menu_item(op_mode)
diff --git a/testing/test_sssp.py b/testing/test_sssp.py
index 8795a90..264c207 100644
--- a/testing/test_sssp.py
+++ b/testing/test_sssp.py
@@ -524,11 +524,15 @@ def test_remove_sssp(setup_sssp, pick_menu_item, press_select, cap_story, cap_me
assert "spending policy settings forgotten" in story
press_select()
+ time.sleep(.1)
assert not settings_get("sssp")
+
tps = settings_get("tp")
if tps:
assert "11-11" not in tps
+ assert not settings_get("sssp")
+
def test_use_main_pin_as_unlock(setup_sssp, cap_story):
# not allowed
@@ -540,4 +544,26 @@ def test_use_main_pin_as_unlock(setup_sssp, cap_story):
assert "already in use" in story
assert "PIN codes must be unique" in story
+
+@pytest.mark.parametrize("hide", [True, False])
+def test_use_trick_pin_as_unlock(hide, setup_sssp, cap_story, new_trick_pin, pick_menu_item,
+ press_select, clear_all_tricks):
+ clear_all_tricks()
+ pin = "11-11"
+ new_trick_pin(pin, 'Wipe Seed', 'Wipe the seed and maybe do more')
+ pick_menu_item('Wipe & Reboot')
+ press_select()
+ press_select()
+ if hide:
+ pick_menu_item(f"↳{pin}")
+ pick_menu_item("Hide Trick")
+ press_select() # confirm
+
+ with pytest.raises(Exception):
+ setup_sssp(pin)
+
+ _, story = cap_story()
+ assert "already in use" in story
+ assert "PIN codes must be unique" in story
+
# EOF
Why this scored 43/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.