Skip to content

test(7.15): Stack 07 audit remediation for ERC-7730 and streamed EIP-712 - #113

Merged
BitHighlander merged 51 commits into
masterfrom
audit/715-stack07-remediation
Sep 25, 2026
Merged

BitHighlander merged 51 commits into
masterfrom
audit/715-stack07-remediation

Conversation

@BitHighlander

Copy link
Copy Markdown
Owner

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, so assertIn passed for any device response. The request is now sent raw, and the first response must be the exact Failure.
  • 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-byte bytes leaf.
  • 4f80b08 — compiler fix. Run through the firmware's own verifier, 56 of the 1,450 official registry formats compiled to programs the device refuses. tokenAmount with 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.

BitHighlander and others added 30 commits September 20, 2026 01:30
…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.
@BitHighlander
BitHighlander merged commit 9cea0ad into master Sep 25, 2026
4 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant