test(7.15): Stack 07 audit remediation for ERC-7730 and streamed EIP-712 - #113
Merged
Merged
Conversation
…oto regen Phase 1 firmware ships with no built-in metadata verification keys; signers are loaded at runtime with a mandatory on-device confirm and a per-tx warning screen naming the alias. Client + tests follow: - device-protocol pin -> 2ec999a9 (up/release-protocol + LoadClearsignSigner message 117); regenerate all pb2 modules (also picks up the zcash/thorchain proto updates the old 5c2d45fc pin was missing; messages-hive added to build_pb.sh so hive regenerates too) - client.load_clearsign_signer(key_id, pubkey, alias) + mapping entry - clear-signing tests: setUp loads the CI test key into slot 3 via the production path (alias 'CI Test'); new tests: load-before-verify order, cancel-at-load refusal, invalid pubkey / alias / key_id rejection - signed_metadata.py: slot-binding comments updated to the loaded-key model Co-Authored-By: Claude Fable 5 <[email protected]>
This suite codified the vulnerable behaviour as correct: six golden vectors asserting sig_v == 27/28, i.e. signatures with no EIP-155 replay protection, produced by calls that omitted chain_id entirely. That is independent confirmation the firmware defect is real and long-standing. Regenerated those six for chain_id=1. The new expected values do NOT come from the device under test -- tests/vectors/eip155_oracle.py reimplements the whole path from scratch (BIP39 -> BIP32 -> RLP -> keccak-256 -> RFC6979 ECDSA) and is negative-controlled by first reproducing all six shipped pre-EIP-155 vectors byte for byte. Run regenerate_eip155_vectors.py to re-derive them; if the negative control fails it refuses to emit anything. Four assertRaises(Exception, ...) sanity checks would otherwise have started passing for the wrong reason, raising "Chain Id out of bounds" instead of exercising the gas/nonce validation they exist to cover. They now pass chain_id explicitly, as do the blind-signing tests. Adds two regression tests, gated to 7.14.2: an omitted chain_id is refused, and an explicit chain_id=0 is refused. client.py used `if chain_id:` to decide whether to put the field on the wire, so an explicit chain_id=0 was silently dropped and became an omitted field -- a different case, which firmware handles differently. Now `is not None`. Adding chain_id=1 is backward-compatible, so the regenerated vectors pass on 7.14.1 as well; only the two new refusal tests are version-gated.
test(solana): gate plain text at its 7.15 capability
…oder Array-of-structs, Permit2 batch and multidimensional device walks now compare the device's domain/message hashes with a spec-derived reference encoder and recover the signature to the device address. The multidim walk is Stack 07 work and is gated by the structured-EIP-712 probe, not by the later evm-unknown-token-review capability.
…e wire set_expected_responses() raised a CallException whose text embedded the expected message, so the old assertIn passed whatever the device sent. Send EthereumSignTx with call_raw and require the first response to be Failure_ActionCancelled with the exact message (no ButtonRequest first).
…nputs The firmware-validation step ran only behind 'if validator:' so compile tests passed without ever reaching firmware. Missing validator or registry is now an explicit skip, and a failure when CI sets KK_REQUIRE_ERC7730_EVIDENCE=1.
…ong bytes Every signing walk now requires exactly one ButtonRequest_SignTx "Sign Typed Data" screen at the end. New device tests: declining that screen returns ActionCancelled and no signature; primaryType EIP712Domain signs keccak(0x1901||domainSeparator) (checked by the independent encoder and signer recovery); unlimited EIP-2612 and Permit2 amounts are refused before their leaf is displayed; a 1024-byte bytes leaf signs and verifies independently.
The registry conformance test had never run against a firmware validator. Run through the firmware's own catalog verifier, 56 of the 1,450 official calldata formats compiled to programs the device refuses: - 46 tokenAmount fields with no token compiled to formatter kind 3, which the verifier refuses without a token argument. With no token the device can only show the raw integer, so compile the raw formatter. - 8 paths iterated two nested arrays; the verifier accepts one "[]" step per path. Refuse them by name. - 2 ABIs nested deeper than the device's 8 levels. Refuse them by name. The registry test now requires every format to pass the firmware validator or be refused for one of those named device limits, and pins that count at 10.
The device response buffer is shared with DebugLink, and a state read during the final sign screen erased an address written before it. The signature still recovered to the right key, so only the reported address was wrong. Under screenshot capture the unfixed firmware returns an empty address and these assertions fail.
Firmware calldata and typed-data captures refuse ERC7730_ABI_MAX_DEPTH (8) or more path steps, and the catalog verifier now does too. Refuse such paths by name at compile time. No official registry format is affected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Carries the python-keepkey side of the full code audit of firmware PR #837. Firmware pins this branch at
4f80b08.This branch stacks on
audit/715-stack07-report-contract(1d42a1f), which no pull request carried before. The five new commits are:e97672c— checks streamed EIP-712 array, struct-array and multidimensional digests against an independent spec-derived encoder, and gates the multidimensional device test on the capability it actually needs.546dcb0— replaces the unlimited-approval assertion that could not fail. The expected-response exception embeds the expected text, soassertInpassed for any device response. The request is now sent raw, and the first response must be the exactFailure.0aa3c1a— the ERC-7730 firmware/registry conformance tests fail rather than skip when CI requires their inputs.d64749c— device coverage for the final typed-data sign screen and its decline, the domain-only digest, unlimited EIP-2612/Permit2 refusal and a 1024-bytebytesleaf.4f80b08— compiler fix. Run through the firmware's own verifier, 56 of the 1,450 official registry formats compiled to programs the device refuses.tokenAmountwith no token now compiles to the raw formatter. Nested array iteration and ABIs deeper than eight levels are refused by name.Each new assertion was shown to fail with the device behaviour or expected value altered.