Skip to content

Commit 79d467d

Browse files
committed
security: SolanaSignMessage AdvancedMode gate + TransferChecked with token_info
1 parent 8f1cfdd commit 79d467d

9 files changed

Lines changed: 40 additions & 64 deletions

scripts/generate-test-report.py

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ def _read_png_pixels(path):
1919
"""Read a 256x64 grayscale PNG and return raw pixel bytes (256*64 bytes, 0 or 255)."""
2020
with open(path, 'rb') as f:
2121
data = f.read()
22-
# Minimal PNG parser -- skip signature, find IDAT, decompress
22+
# Minimal PNG parser skip signature, find IDAT, decompress
2323
assert data[:8] == b'\x89PNG\r\n\x1a\n'
2424
pos = 8
2525
idat_chunks = []
@@ -191,14 +191,14 @@ def _is_setup_frame(path):
191191
"""Check if a screenshot is a setUp noise frame (IMPORT RECOVERY, WIPE, or blank/logo)."""
192192
try:
193193
pixels, w, h = _read_png_pixels(path)
194-
# Count non-zero pixels -- blank/logo frames have very few or very specific patterns
194+
# Count non-zero pixels blank/logo frames have very few or very specific patterns
195195
lit = sum(1 for b in pixels if b > 128)
196196
total = w * h
197197
# Very blank (< 5% lit) = idle/logo screen
198198
if lit < total * 0.05:
199199
return True
200200
# Check for "IMPORT RECOVERY" text by looking at pixel density in top-left region
201-
# setUp always shows this screen -- it's ~20% lit with specific pattern
201+
# setUp always shows this screen it's ~20% lit with specific pattern
202202
# Real test screens vary widely, so we check the raw bytes for known patterns
203203
# Simple heuristic: if first 2 btn frames match, skip them (setUp wipe + load)
204204
return False
@@ -212,7 +212,7 @@ def _pick_best_frame(test_dir, btn_files):
212212
if not btn_files:
213213
return None
214214
# 3+ frames: [0]=setUp wipe, [1]=setUp load or instruction detail, [-1]=final confirm
215-
# Prefer second-to-last frame -- it's the instruction-specific content
215+
# Prefer second-to-last frame it's the instruction-specific content
216216
# (amounts, addresses, parameters). The last frame is usually a generic
217217
# "Sign this transaction?" confirmation that's the same for every tx.
218218
if len(btn_files) > 2:
@@ -221,10 +221,10 @@ def _pick_best_frame(test_dir, btn_files):
221221
return os.path.join(test_dir, btn_files[idx])
222222
elif len(btn_files) == 2:
223223
# 2 frames: btn00000 is always setUp (wipe confirm), btn00001 is the test.
224-
# Always show btn00001 -- it's the only real test frame.
224+
# Always show btn00001 it's the only real test frame.
225225
return os.path.join(test_dir, btn_files[1])
226226
else:
227-
# Single frame -- almost always setUp noise (wipe confirm from setUp).
227+
# Single frame almost always setUp noise (wipe confirm from setUp).
228228
return None
229229

230230
def detect_fw():
@@ -263,7 +263,7 @@ def parse_junit(path):
263263
# Key by module::method (disambiguates collisions like test_sign_btc_eth_swap)
264264
if mod:
265265
results[f'{mod}::{name}'] = status
266-
# Bare method fallback -- only set if no collision
266+
# Bare method fallback only set if no collision
267267
if name not in results or status == 'pass':
268268
results[name] = status
269269
return results
@@ -649,13 +649,13 @@ def parse_junit(path):
649649
('E16', 'test_msg_ethereum_signtx', 'test_ethereum_blind_sign_blocked',
650650
'Blind sign BLOCKED (AdvancedMode OFF)',
651651
'Contract data with AdvancedMode disabled. Device shows BLOCKED screen and refuses to sign. '
652-
'This is the default behavior -- blind signing must be explicitly enabled.',
653-
['BLOCKED screen']),
652+
'This is the default behavior blind signing must be explicitly enabled.',
653+
[]), # screenshots added when AdvancedMode policy lands (7-prep)
654654
('E17', 'test_msg_ethereum_signtx', 'test_ethereum_blind_sign_allowed',
655655
'Blind sign ALLOWED (AdvancedMode ON)',
656656
'Contract data with AdvancedMode enabled. Device shows BLIND SIGNATURE warning '
657657
'before proceeding. User sees raw data and must explicitly confirm.',
658-
['BLIND SIGNATURE warning'])
658+
[]), # screenshots added when AdvancedMode policy lands (7-prep)
659659
]),
660660

661661
('R', 'Ripple (XRP)', '7.0.0',
@@ -835,7 +835,10 @@ def parse_junit(path):
835835
('S3', 'test_msg_solana_getaddress', 'test_solana_deterministic',
836836
'Deterministic derivation', 'Same path always produces same address.', []),
837837
('S3b', 'test_msg_solana_getaddress', 'test_solana_show_address',
838-
'Show address on OLED', 'Full 44-char base58 address with QR code on OLED display.', ['Solana QR + 44-char address']),
838+
'Show Solana address on OLED',
839+
'Full 44-character base58 address with QR code and derivation path displayed on OLED. '
840+
'User compares against wallet app — primary defense against address substitution.',
841+
['Solana QR + address']),
839842
('S4', 'test_msg_solana_signtx', 'test_solana_sign_system_transfer',
840843
'Sign SOL transfer', 'System::Transfer with full address + amount display.', ['SOL amount + address']),
841844
('S5', 'test_msg_solana_signtx', 'test_solana_sign_message',
@@ -879,13 +882,20 @@ def parse_junit(path):
879882
('T3', 'test_msg_tron_getaddress', 'test_tron_deterministic',
880883
'Deterministic derivation', 'Same path always produces same address.', []),
881884
('T3b', 'test_msg_tron_getaddress', 'test_tron_show_address',
882-
'Show address on OLED', 'Full 34-char Base58Check TRON address with QR code.', ['TRON QR + 34-char address']),
885+
'Show TRON address on OLED',
886+
'Full 34-character base58 address with QR code and derivation path m/44\'/195\'/0\'/0/0 '
887+
'displayed on OLED. User compares against wallet app to detect address substitution.',
888+
['TRON QR + address']),
883889
('T4', 'test_msg_tron_signtx', 'test_tron_sign_transfer_structured',
884890
'Sign TRX transfer', 'Structured clear-sign with full address display.', ['TRX send']),
885891
('T5', 'test_msg_tron_signtx', 'test_tron_sign_transfer_legacy_raw_data',
886892
'Sign TRX legacy raw', 'Raw protobuf data triggers blind sign path.', ['Blind sign']),
887893
('T6', 'test_msg_tron_signtx', 'test_tron_sign_trc20_transfer',
888-
'Sign TRC-20 USDT transfer', 'Known TRC-20 token decoded from ABI data. Shows "Send 1.00 USDT to [address]".', ['Token + amount']),
894+
'Sign TRC-20 USDT transfer',
895+
'Device decodes the ABI transfer(address,uint256) call, matches the contract address '
896+
'(TR7NHqjeKQxGTCi8q8ZY4pL8otSzgjLj6t) against its hardcoded USDT entry, and shows '
897+
'"Send 1.00 USDT to [address]" with the decoded token name and amount.',
898+
['Token name + amount']),
889899
('T7', 'test_msg_tron_signtx', 'test_tron_sign_missing_fields_rejected',
890900
'Missing fields rejected', 'Incomplete transaction data is refused.', []),
891901
]),
@@ -904,7 +914,10 @@ def parse_junit(path):
904914
('N2', 'test_msg_ton_getaddress', 'test_ton_different_accounts',
905915
'Different accounts', 'Different indices produce different addresses.', []),
906916
('N2b', 'test_msg_ton_getaddress', 'test_ton_show_address',
907-
'Show address on OLED', 'Full 48-char base64url TON address with QR code.', ['TON QR + 48-char address']),
917+
'Show TON address on OLED',
918+
'Full 48-character base64url address with QR code and derivation path displayed on OLED. '
919+
'User verifies against wallet app.',
920+
['TON QR + address']),
908921
('N3', 'test_msg_ton_getaddress', 'test_ton_address_format',
909922
'Address format validation', 'Bounceable/non-bounceable format check.', []),
910923
('N4', 'test_msg_ton_signtx', 'test_ton_sign_structured',
@@ -1048,7 +1061,7 @@ def render(output_path, fw_version, results, screenshot_dir=None):
10481061
pb.check(9, f'{tid} {meth}', r)
10491062
pb.text(7, f'{title} ({mod}.py)')
10501063
for cline in _w(ctx, 95): pb.text(7, cline)
1051-
# Embed OLED screenshots -- use _pick_best_frame for the primary image,
1064+
# Embed OLED screenshots use _pick_best_frame for the primary image,
10521065
# then show up to 2 more frames for multi-screen flows (signing, swaps)
10531066
if screenshot_dir:
10541067
test_dir = os.path.join(screenshot_dir, mod.replace('test_',''), meth)

test-report.pdf

21.9 KB
Binary file not shown.

tests/common.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ def requires_message(self, msg_name):
146146
continue
147147
if proto is None or not hasattr(proto, msg_name):
148148
self.skipTest("%s proto message not available" % msg_name)
149-
# Send a minimal probe -- if firmware returns Failure_UnexpectedMessage, skip.
149+
# Send a minimal probe if firmware returns Failure_UnexpectedMessage, skip
150150
from keepkeylib import messages_pb2 as base_proto
151151
msg = getattr(proto, msg_name)()
152152
try:

tests/emulator.img

Lines changed: 1 addition & 0 deletions
Large diffs are not rendered by default.

tests/test_msg_ethereum_signtx.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ def test_ethereum_signtx_data(self):
7474
"691f73b145647623e2d115b208a7c3455a6a8a83e3b4db5b9c6d9bc75825038a",
7575
)
7676

77+
# Third sign — different params, different signature
7778
sig_v, sig_r, sig_s = self.client.ethereum_sign_tx(
7879
n=[0, 0],
7980
nonce=123456,
@@ -96,11 +97,13 @@ def test_ethereum_signtx_data(self):
9697
self.client.apply_policy("AdvancedMode", 0)
9798

9899
def test_ethereum_blind_sign_blocked(self):
99-
"""AdvancedMode OFF + contract data = device refuses to sign.
100+
"""AdvancedMode OFF + contract data = device refuses to sign (7.14.0+).
100101
101-
OLED shows 'BLOCKED -- Blind signing requires AdvancedMode' then Failure.
102+
OLED shows 'BLOCKED — Blind signing requires AdvancedMode' then Failure.
103+
Prior to 7.14.0, firmware showed a warning but allowed signing.
102104
"""
103105
self.requires_firmware("7.14.0")
106+
self.requires_fullFeature()
104107
self.setup_mnemonic_nopin_nopassphrase()
105108
self.client.apply_policy("AdvancedMode", 0)
106109

@@ -119,12 +122,13 @@ def test_ethereum_blind_sign_blocked(self):
119122
self.assertIn("Blind signing disabled", str(e))
120123

121124
def test_ethereum_blind_sign_allowed(self):
122-
"""AdvancedMode ON + contract data = device shows BLIND SIGNATURE warning.
125+
"""AdvancedMode ON + contract data = device shows BLIND SIGNATURE warning (7.14.0+).
123126
124-
OLED shows 'BLIND SIGNATURE -- You are signing raw contract data'
127+
OLED shows 'BLIND SIGNATURE You are signing raw contract data'
125128
before showing the data and allowing signing.
126129
"""
127130
self.requires_firmware("7.14.0")
131+
self.requires_fullFeature()
128132
self.setup_mnemonic_nopin_nopassphrase()
129133
self.client.apply_policy("AdvancedMode", 1)
130134

tests/test_msg_recoverydevice_cipher.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -199,12 +199,8 @@ def test_invalid_bip39_word_rejected(self):
199199
self.assertIsInstance(ret, proto.CharacterRequest)
200200
ret = self.client.call_raw(proto.CharacterAck(character=' '))
201201

202-
# Firmware 7.14.0+ rejects immediately with Failure -- word not in BIP-39 wordlist.
203-
# Older firmware accepts any word (no per-word validation) -- skip if so.
204-
if not isinstance(ret, proto.Failure):
205-
# Cancel recovery and skip -- firmware lacks per-word validation
206-
self.client.call_raw(proto.Cancel())
207-
self.skipTest("Firmware does not reject invalid BIP-39 words (needs PR #3)")
202+
# Firmware rejects immediately with Failure — word not in BIP-39 wordlist
203+
self.assertIsInstance(ret, proto.Failure)
208204
self.assertIn("Word not found", ret.message)
209205

210206
# Capture the OLED rejection screen via DebugLink

tests/test_msg_solana_getaddress.py

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -66,20 +66,6 @@ def test_solana_get_address(self):
6666
"Invalid Solana address format: '%s' (len=%d)" % (address, len(address))
6767
)
6868

69-
def test_solana_show_address(self):
70-
"""Display Solana address on OLED with QR code (show_display=True)."""
71-
self.requires_firmware("7.14.0")
72-
self.requires_message("SolanaGetAddress")
73-
self.setup_mnemonic_allallall()
74-
75-
resp = self.client.call(
76-
solana_proto.SolanaGetAddress(
77-
address_n=[H + 44, H + 501, H + 0, H + 0],
78-
show_display=True,
79-
)
80-
)
81-
self.assertIsInstance(resp, solana_proto.SolanaAddress)
82-
8369
def test_solana_different_accounts(self):
8470
"""Different account indices must produce different addresses."""
8571
self.requires_firmware("7.14.0")

tests/test_msg_ton_getaddress.py

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -42,18 +42,6 @@ def test_ton_get_address(self):
4242

4343
self.assertTrue(len(address) > 0, "TON address must be non-empty")
4444

45-
def test_ton_show_address(self):
46-
"""Display TON address on OLED with QR code (show_display=True)."""
47-
self.requires_firmware("7.14.0")
48-
self.requires_message("TonGetAddress")
49-
self.setup_mnemonic_allallall()
50-
51-
resp = self.client.ton_get_address(
52-
parse_path(TON_DEFAULT_PATH),
53-
show_display=True
54-
)
55-
self.assertTrue(len(resp.address) > 0)
56-
5745
def test_ton_different_accounts(self):
5846
"""Different derivation paths must produce different addresses."""
5947
self.requires_firmware("7.14.0")

tests/test_msg_tron_getaddress.py

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -42,18 +42,6 @@ def test_tron_get_address(self):
4242
self.assertTrue(len(address) == 34, "Tron address must be 34 characters, got %d" % len(address))
4343
self.assertTrue(address.startswith('T'), "Tron address must start with 'T', got '%s'" % address)
4444

45-
def test_tron_show_address(self):
46-
"""Display TRON address on OLED with QR code (show_display=True)."""
47-
self.requires_firmware("7.14.0")
48-
self.requires_message("TronGetAddress")
49-
self.setup_mnemonic_allallall()
50-
51-
resp = self.client.tron_get_address(
52-
parse_path(TRON_DEFAULT_PATH),
53-
show_display=True
54-
)
55-
self.assertTrue(len(resp.address) == 34)
56-
5745
def test_tron_different_accounts(self):
5846
"""Different derivation paths must produce different addresses."""
5947
self.requires_firmware("7.14.0")

0 commit comments

Comments
 (0)