Skip to content

fix: reject EIP-1559 transactions missing chain_id#428

Closed
BitHighlander wants to merge 0 commit into
keepkey:developfrom
BitHighlander:fix/eip1559-require-chainid
Closed

fix: reject EIP-1559 transactions missing chain_id#428
BitHighlander wants to merge 0 commit into
keepkey:developfrom
BitHighlander:fix/eip1559-require-chainid

Conversation

@BitHighlander

Copy link
Copy Markdown
Collaborator

Problem

hash_rlp_number(0) returns immediately without hashing any bytes:

static void hash_rlp_number(uint32_t number) {
  if (!number) { return; }   // hashes nothing
  ...
}

But rlp_calculate_number_length(0) returns 1 (for the 0x80 byte that should represent integer 0 in RLP).

For an EIP-1559 transaction sent without chain_id (has_chain_id = false), chain_id defaults to 0. The length calculation adds 1 byte to rlp_length for it, but hash_rlp_number(0) hashes nothing. The RLP list header claims N bytes but only N−1 are hashed — corrupting the keccak pre-image. The resulting signature recovers to a wrong address.

The legacy EIP-155 path is safe because send_signature() guards with if (chain_id) { hash_rlp_number(chain_id); }. The EIP-1559 path in ethereum_signing_init() had no such guard:

if (ethereum_tx_type == ETHEREUM_TX_TYPE_EIP_1559) {
    hash_rlp_number(chain_id);   // silent corruption if chain_id == 0
}

Fix

Fail fast at the input validation stage. EIP-1559 requires chain_id by spec (EIP-1559 §4.2: "chainId is the chain ID of the signing chain").

if (ethereum_tx_type == ETHEREUM_TX_TYPE_EIP_1559 && !msg->has_chain_id) {
    fsm_sendFailure(FailureType_Failure_SyntaxError, _("EIP-1559 requires chain_id"));
    ethereum_signing_abort();
    return;
}

Notes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR prevents malformed EIP-1559 Ethereum transactions (type 0x02) from being signed when chain_id is omitted, avoiding a mismatch between the computed RLP length and the bytes actually hashed (which can corrupt the keccak preimage and lead to signatures recovering to the wrong address).

Changes:

  • Add an explicit input-validation check to reject EIP-1559 transactions when msg->has_chain_id == false.
  • Return a Failure_SyntaxError with a specific message and abort signing early.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@pastaghost pastaghost closed this May 23, 2026
@pastaghost pastaghost force-pushed the fix/eip1559-require-chainid branch from 73bd71b to bc8bd3c Compare May 23, 2026 14:37
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.

3 participants