test(eth): pin the multi-byte chain_id EIP-1559 regression - #31
Closed
BitHighlander wants to merge 1 commit into
Closed
test(eth): pin the multi-byte chain_id EIP-1559 regression#31BitHighlander wants to merge 1 commit into
BitHighlander wants to merge 1 commit into
Conversation
Firmware fix ed6db167 ("EIP-1559 chainId hashing wrong for multi-byte chain
IDs") shipped with no test at a chain id that reproduces it. Every EIP-1559
case in this file uses chain_id 1 or 3 — both single-byte — so the bug had zero
coverage in the file that tests the feature, and can silently return.
The bug: hash_rlp_field((uint8_t*)&chain_id, 1) fed only the least-significant
byte into keccak on little-endian ARM. Base (8453 = 0x2105) hashed 0x05, so the
signature recovered to an unrelated address with no funds. The RLP length was
computed correctly from the full value and the legacy EIP-155 path was always
right — only the EIP-1559 hash step was wrong.
A golden r/s would need a device run to produce, so this is a differential
instead. Sign one identical tx under two chain ids the BUGGY firmware cannot
distinguish:
8453 = 0x2105 low byte 0x05, two-byte value
4357 = 0x1105 low byte 0x05, two-byte value
Same low byte and same RLP length header, so the broken code hashes a
byte-identical pre-image for both. Signing is deterministic (RFC 6979), so
buggy firmware returns the same signature twice and the assertion fails.
Correct firmware hashes 0x21 0x05 vs 0x11 0x05 and they differ. No golden
value, no crypto deps — the repo has ecdsa but no keccak, so recovering the
signer address was not an option.
Version-gated to 7.15.0 so it SKIPs rather than fails on firmware predating
the fix.
Worth recording for whoever tests this by hand: Vault cannot reach this path at
all. It forces legacy transactions for chainId >= 256 (eip1559Ok = chainId <
256 in swap.ts), which is why ordinary Vault sends on Base were never broken.
The BEX builds type-2 with no such guard, so a dApp transaction through the
extension is the only manual route to it.
Owner
Author
|
Closing — wrong target. SOP for python-keepkey is to PR into the canonical branch of the single open PR into upstream master (currently Reopened with the full reconcile set as keepkey#212. |
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.
Firmware fix ed6db167 ("EIP-1559 chainId hashing wrong for multi-byte chain
IDs") shipped with no test at a chain id that reproduces it. Every EIP-1559
case in this file uses chain_id 1 or 3 — both single-byte — so the bug had zero
coverage in the file that tests the feature, and can silently return.
The bug: hash_rlp_field((uint8_t*)&chain_id, 1) fed only the least-significant
byte into keccak on little-endian ARM. Base (8453 = 0x2105) hashed 0x05, so the
signature recovered to an unrelated address with no funds. The RLP length was
computed correctly from the full value and the legacy EIP-155 path was always
right — only the EIP-1559 hash step was wrong.
A golden r/s would need a device run to produce, so this is a differential
instead. Sign one identical tx under two chain ids the BUGGY firmware cannot
distinguish:
Same low byte and same RLP length header, so the broken code hashes a
byte-identical pre-image for both. Signing is deterministic (RFC 6979), so
buggy firmware returns the same signature twice and the assertion fails.
Correct firmware hashes 0x21 0x05 vs 0x11 0x05 and they differ. No golden
value, no crypto deps — the repo has ecdsa but no keccak, so recovering the
signer address was not an option.
Version-gated to 7.15.0 so it SKIPs rather than fails on firmware predating
the fix.
Worth recording for whoever tests this by hand: Vault cannot reach this path at
all. It forces legacy transactions for chainId >= 256 (eip1559Ok = chainId <
256 in swap.ts), which is why ordinary Vault sends on Base were never broken.
The BEX builds type-2 with no such guard, so a dApp transaction through the
extension is the only manual route to it.