fix: address review on the 7.15 harness

Two correctness defects and four test-integrity ones.

eip712_stream: multidimensional arrays were validated against the WRONG
dimension. Solidity nests right-to-left -- in T[k][j] the outer array holds j
-- so int16[2][][4] parses to [2, 0, 4] while the first list a walker meets
holds 4. Levels are now consumed from the END. Single-dimension arrays are
unaffected (both ends coincide), which is why every existing test passed.

clearsign_abi: signed integers were encoded as unsigned. intN was rejected for
every negative value and ACCEPTED at or above 2^(N-1), which the EVM reads
back as negative -- calldata that does not mean what its declared type says.
Split the paths; intN is now range-checked to [-2^(N-1), 2^(N-1)-1] and
sign-extended.

test_msg_ethereum_thorchain_deposit: assertRaises((CallException, Exception))
accepts every failure, including a fixture that fails to build, so a security
gate could pass without the firmware ever refusing. Narrowed to CallException.

test_msg_solana_lut_attestation: two tests compared a degraded run against a
baseline without reloading the RAM-only signer that the preceding signing tore
down -- so they compared two identical baseline flows and would pass even if
bad signatures were accepted. The attested test above them already documents
this exact trap; the other two now reload too.

test_msg_thorchain_signtx: restores real verification. Both tests asserted only
r/s LENGTHS, which a wrong router, wrong calldata or wrong sighash would also
satisfy. They now reconstruct the legacy sighash and recover the signer,
comparing it to ethereum_get_address -- the pattern already proven in the
mayachain suite. Stronger than the frozen vectors this replaced, and it stays
correct across router changes. The superseded 7.14.2 vectors are kept as
comments.

test_msg_recoverydevice_cipher: gate raised to 7.15.1 to match its docstring.

.gitmodules: device-protocol tracks master again, not up/release-protocol.

NOT taken: forwarding a non-rune denom through thorchain_sign_tx. The firmware
this targets hardcodes "denom":"rune" in its sign-doc; only 7.15+ reads one.
nanopb SKIPS unknown fields, so forwarding it to older firmware would be
silently ignored and the device would sign a RUNE transfer while the host
believed otherwise. The refusal is fail-closed and stays, now with the reason
recorded at the guard.

Author: BitHighlander

.gitmodules

@@ -1,7 +1,7 @@
 [submodule "device-protocol"]
 path = device-protocol
 url = https://github.com/keepkey/device-protocol.git
-branch = up/release-protocol
+branch = master
 [submodule "keepkeylib/eth/ethereum-lists"]
 path = keepkeylib/eth/ethereum-lists
 url = https://github.com/keepkey/ethereum-lists.git

keepkeylib/clearsign_abi.py

@@ -54,12 +54,27 @@ def encode_static_args(types, values):
     for typ, val in zip(types, values):
         if typ == 'address':
             out += _addr_word(val)
-        elif typ.startswith('uint') or typ.startswith('int'):
-            digits = typ[4:] if typ.startswith('uint') else typ[3:]
+        elif typ.startswith('uint'):
+            digits = typ[4:]
             bits = int(digits) if digits else 256
             n = int(val)
-            assert 0 <= n < (1 << bits), 'value %r out of range for %s' % (val, typ)
+            assert 0 <= n < (1 << bits), (
+                'value %r out of range for %s' % (val, typ))
             out += n.to_bytes(32, 'big')
+        elif typ.startswith('int'):
+            # Signed types are NOT unsigned ones with a wider range. intN holds
+            # [-2^(N-1), 2^(N-1)-1] and is encoded two's-complement, sign-
+            # extended to the full word. Treating it as unsigned both rejected
+            # every negative value and silently accepted values at or above
+            # 2^(N-1), which the EVM reads back as NEGATIVE -- calldata that
+            # does not mean what the declared type says.
+            digits = typ[3:]
+            bits = int(digits) if digits else 256
+            n = int(val)
+            lo, hi = -(1 << (bits - 1)), (1 << (bits - 1)) - 1
+            assert lo <= n <= hi, (
+                'value %r out of range for %s (%d..%d)' % (val, typ, lo, hi))
+            out += n.to_bytes(32, 'big', signed=True)
         elif typ == 'bool':
             out += (1 if val else 0).to_bytes(32, 'big')
         elif typ.startswith('bytes') and typ != 'bytes' and not typ.endswith('[]'):

keepkeylib/client.py

@@ -1204,6 +1204,17 @@ def thorchain_sign_tx(
                     raise CallException("Thorchain.MsgSend", "Multiple amounts per send msg not supported")
 
                 denom = msg['value']['amount'][0]['denom']
+                # Fail CLOSED on any other denomination, deliberately.
+                #
+                # ThorchainMsgSend carries a `denom` field, but the firmware
+                # this talks to builds its amino sign-doc with the string
+                # "rune" HARDCODED (lib/firmware/thorchain.c) -- only 7.15+
+                # reads a denom and validates it. nanopb SKIPS unknown fields
+                # rather than rejecting them, so forwarding `denom` to older
+                # firmware would be silently ignored and the device would sign
+                # a rune transfer while the host believed it had sent another
+                # asset. Refusing is the only safe answer until the capability
+                # can be detected; do not "fix" this by passing denom through.
                 if denom != 'rune':
                     raise CallException("Thorchain.MsgSend", "Unsupported denomination: " + denom)
 

keepkeylib/eip712_stream.py

@@ -40,6 +40,17 @@ class Eip712Error(Exception):
     pass
 
 
+def _dimension(levels, used):
+    """The declared size of the array level being entered.
+
+    Solidity nests right-to-left: in `T[k][j]` the OUTER array has j elements,
+    so `int16[2][][4]` parses to [2, 0, 4] but the first list a walker meets
+    holds 4. Levels are therefore consumed from the END. With a single
+    dimension both ends coincide, which is why this went unnoticed.
+    """
+    return levels[len(levels) - 1 - used]
+
+
 def parse_solidity_type(type_str):
     """"uint256", "bytes32", "Person[3]", "int16[2][][4]" -> field descriptor.
 
@@ -243,7 +254,7 @@ def resolve_member_path(typed_data, path):
     for i in range(1, len(path)):
         index = path[i]
         if levels_used < len(field['array_levels']):
-            declared = field['array_levels'][levels_used]
+            declared = _dimension(field['array_levels'], levels_used)
             if not isinstance(value, list):
                 raise Eip712Error('Expected an array at %r' % (path[:i],))
             if declared and len(value) != declared:
@@ -269,7 +280,7 @@ def resolve_member_path(typed_data, path):
         value = value[member['name']]
 
     if levels_used < len(field['array_levels']):
-        declared = field['array_levels'][levels_used]
+        declared = _dimension(field['array_levels'], levels_used)
         if not isinstance(value, list):
             raise Eip712Error('Expected an array for a length request')
         if declared and len(value) != declared:

tests/test_msg_ethereum_thorchain_deposit.py

@@ -126,7 +126,7 @@ def test_deposit_with_expiry_non_thor_address_blind_sign_blocked(self):
         import keepkeylib.types_pb2 as types
 
         # No AdvancedMode, random contract address — should be rejected
-        with self.assertRaises((CallException, Exception)):
+        with self.assertRaises(CallException):
             self.client.ethereum_sign_tx(
                 n=parse_path("m/44'/60'/0'/0/0"),
                 nonce=3,
@@ -195,7 +195,7 @@ def test_deposit_unpinned_chain_blind_sign_blocked(self):
         memo = "=:ETH.ETH:0xabcdef1234567890abcdef1234567890abcdef12:0:t:0"
         data = _build_deposit_with_expiry_calldata(memo)
 
-        with self.assertRaises((CallException, Exception)):
+        with self.assertRaises(CallException):
             self.client.ethereum_sign_tx(
                 n=parse_path("m/44'/60'/0'/0/0"),
                 nonce=5,

tests/test_msg_recoverydevice_cipher.py

@@ -174,7 +174,7 @@ def test_invalid_bip39_word_rejected(self):
         BIP-39 wordlist must return Failure immediately.
         Requires firmware 7.15.1+ (per-word validation).
         """
-        self.requires_firmware("7.15.0")
+        self.requires_firmware("7.15.1")
         ret = self.client.call_raw(proto.RecoveryDevice(word_count=12,
                                    passphrase_protection=False,
                                    pin_protection=False,

tests/test_msg_solana_lut_attestation.py

@@ -169,6 +169,11 @@ def test_bad_signature_degrades_to_todays_flow(self):
         accounts = [b'\x51' * 32]
 
         base_codes, _ = self._screens(raw_tx=raw)
+        # Same reload as the attested case above: a completed signing tears the
+        # RAM-only session down, so without this the second run would find no
+        # signer, verify nothing, and pass vacuously by comparing two identical
+        # baseline flows -- which is exactly what this test must not do.
+        self._load_signer()
         bad_codes, resp = self._screens(
             raw_tx=raw, lut_account=accounts,
             lut_signature=b'\x00' * 64, lut_signer_key_id=SLOT)
@@ -190,6 +195,11 @@ def test_attestation_does_not_replay_onto_another_transaction(self):
         sig_for_a = self._attest(raw_a, accounts)
 
         base_codes, _ = self._screens(raw_tx=raw_b)
+        # Same reload as the attested case above: a completed signing tears the
+        # RAM-only session down, so without this the second run would find no
+        # signer, verify nothing, and pass vacuously by comparing two identical
+        # baseline flows -- which is exactly what this test must not do.
+        self._load_signer()
         replay_codes, _ = self._screens(
             raw_tx=raw_b, lut_account=accounts,
             lut_signature=sig_for_a, lut_signer_key_id=SLOT)

tests/test_msg_thorchain_signtx.py

@@ -7,6 +7,24 @@
 import keepkeylib.messages_pb2 as proto
 import keepkeylib.types_pb2 as proto_types
 from keepkeylib.tools import parse_path
+from keepkeylib.signed_metadata import eth_sighash_legacy, keccak256
+
+
+def recover_eth_signer(sig_r, sig_s, sig_v, digest, chain_id):
+    """Recover the 20-byte Ethereum signer from a legacy (EIP-155) signature.
+
+    Same helper as test_msg_mayachain_signtx.py. Recovering the signer -- rather
+    than asserting r/s lengths -- means a wrong digest, wrong calldata, wrong
+    key or wrong curve fails the test, and it stays correct across router
+    changes without re-freezing vectors, which a frozen (r,s) pair does not.
+    """
+    from ecdsa import VerifyingKey, SECP256k1, util
+    rec = sig_v - (35 + 2 * chain_id) if chain_id else sig_v - 27
+    keys = VerifyingKey.from_public_key_recovery_with_digest(
+        sig_r + sig_s, digest, SECP256k1, hashfunc=None,
+        sigdecode=util.sigdecode_string,
+    )
+    return keccak256(keys[rec].to_string())[-20:]
 
 DEFAULT_BIP32_PATH = "m/44h/931h/0h/0/0"
 
@@ -72,16 +90,13 @@ def test_sign_eth_btc_swap(self):
         self.requires_fullFeature()
         self.requires_firmware("7.1.0")
         self.setup_mnemonic_nopin_nopassphrase()
-        sig_v, sig_r, sig_s = self.client.ethereum_sign_tx(
-            n=[2147483692,2147483708,2147483648,0,0],
-            nonce=0x0,
-            gas_price=0x5FB9ACA00,
-            gas_limit=0x186A0,
-            value=0x00,
-            to=unhexlify('d37bbe5744d730a1d98d8dc97c42f0ca46ad7146'),  # THORChain router v4.1.1 (firmware-pinned)
-            address_type=0,
-            chain_id=1,
-            data=unhexlify('1fece7b4' +
+        address_n = [2147483692,2147483708,2147483648,0,0]
+        nonce = 0x0
+        gas_price = 0x5FB9ACA00
+        gas_limit = 0x186A0
+        value = 0x00
+        to = unhexlify('d37bbe5744d730a1d98d8dc97c42f0ca46ad7146')  # THORChain router v4.1.1
+        data = unhexlify('1fece7b4' +
             '000000000000000000000000345b297ec83add7ff74d2f7933651bffa037d956' +    # asgard vault address
             '0000000000000000000000000000000000000000000000000000000000000000' +    # asset ETH
             '000000000000000000000000000000000000000000000065945acd2b867ef000' +    # amount
@@ -90,13 +105,20 @@ def test_sign_eth_btc_swap(self):
             # SWAP:BTC.BTC:0x41e5560054824ea6b0732e656e3ad64e20e94e45:420
             '535741503a4254432e4254433a30783431653535363030353438323465613662' +    # thorchain transaction memo
             '30373332653635366533616436346532306539346534353a3432300000000000')
-        )   
-        # `to` updated to the firmware-pinned THORChain router; exact r/s
-        # change with it, so assert structure here and regenerate exact vectors
-        # on-device.
+        sig_v, sig_r, sig_s = self.client.ethereum_sign_tx(
+            n=address_n, nonce=nonce, gas_price=gas_price, gas_limit=gas_limit,
+            value=value, to=to, address_type=0, chain_id=1, data=data)
+        # Verify the signature is over the EXACT transaction above and by
+        # THIS device's key. Length checks alone would also pass for a wrong
+        # router, wrong calldata or wrong sighash; recovery would not.
         self.assertIn(sig_v, [37, 38])  # EIP-155 chain_id=1
         self.assertEqual(len(sig_r), 32)
         self.assertEqual(len(sig_s), 32)
+        digest = eth_sighash_legacy(nonce, gas_price, gas_limit, to, value,
+                                    data, 1)
+        signer = recover_eth_signer(sig_r, sig_s, sig_v, digest, 1)
+        # NB: KeepKeyTest's assertEqual override takes no msg argument.
+        self.assertEqual(signer, self.client.ethereum_get_address(address_n))
 
 
     def test_sign_btc_add_liquidity(self):
@@ -123,16 +145,13 @@ def test_sign_eth_add_liquidity(self):
         self.requires_fullFeature()
         self.requires_firmware("7.0.2")
         self.setup_mnemonic_nopin_nopassphrase()
-        sig_v, sig_r, sig_s = self.client.ethereum_sign_tx(
-                        n=[2147483692,2147483708,2147483648,0,0],
-            nonce=0x0,
-            gas_price=0x5FB9ACA00,
-            gas_limit=0x186A0,
-            value=0x00,
-            to=unhexlify('d37bbe5744d730a1d98d8dc97c42f0ca46ad7146'),  # THORChain router v4.1.1 (firmware-pinned)
-            address_type=0,
-            chain_id=1,
-            data=unhexlify('1fece7b4' +
+        address_n = [2147483692,2147483708,2147483648,0,0]
+        nonce = 0x0
+        gas_price = 0x5FB9ACA00
+        gas_limit = 0x186A0
+        value = 0x00
+        to = unhexlify('d37bbe5744d730a1d98d8dc97c42f0ca46ad7146')  # THORChain router v4.1.1
+        data = unhexlify('1fece7b4' +
             '0000000000000000000000000000000000000000000000000000000000000000' +
             '0000000000000000000000000000000000000000000000000000000000000000' +
             '0000000000000000000000000000000000000000000000000000000000000000' +
@@ -141,22 +160,28 @@ def test_sign_eth_add_liquidity(self):
             # ADD:ETH.ETH:0xc5b2608927ea95ed43f842f553e3a27b09c050e8:420
             '4144443a4554482e4554483a3078633562323630383932376561393565643433' +
             '663834326635353365336132376230396330353065383a343230000000000000')
-
-        )
-        # `to` updated to the firmware-pinned THORChain router; exact r/s
-        # change with it, so assert structure here and regenerate exact vectors
-        # on-device.
+        sig_v, sig_r, sig_s = self.client.ethereum_sign_tx(
+            n=address_n, nonce=nonce, gas_price=gas_price, gas_limit=gas_limit,
+            value=value, to=to, address_type=0, chain_id=1, data=data)
+        # Verify the signature is over the EXACT transaction above and by
+        # THIS device's key. Length checks alone would also pass for a wrong
+        # router, wrong calldata or wrong sighash; recovery would not.
         #
-        # 7.14.2 regenerated exact vectors for this calldata, but against the
-        # OLD `to` (0x41e5560054824ea6b0732e656e3ad64e20e94e45). `to` is an RLP
-        # field of the sighash, so they do not describe the tx signed above.
-        # Retained as the oracle for that superseded fixture:
+        # 7.14.2 froze exact vectors for this calldata against the OLD `to`
+        # (0x41e5560054824ea6b0732e656e3ad64e20e94e45). `to` is an RLP field of
+        # the sighash, so they describe a different transaction. Kept as the
+        # oracle for that superseded fixture:
         #   sig_v 37
         #   r 7adc5bda6e66b37a81962557c844509c4bfaa1e9217fc6d05968286d60b67dbf
         #   s 613479150c4cfbcdc8243055aa5137afc89826c4176c420a60409f139171831b
         self.assertIn(sig_v, [37, 38])  # EIP-155 chain_id=1
         self.assertEqual(len(sig_r), 32)
         self.assertEqual(len(sig_s), 32)
+        digest = eth_sighash_legacy(nonce, gas_price, gas_limit, to, value,
+                                    data, 1)
+        signer = recover_eth_signer(sig_r, sig_s, sig_v, digest, 1)
+        # NB: KeepKeyTest's assertEqual override takes no msg argument.
+        self.assertEqual(signer, self.client.ethereum_get_address(address_n))
 
     def test_thorchain_remove_liquidity(self):
         self.requires_fullFeature()