search

LedgerHQ / app-ethereum

Ethereum wallet application for Ledger devices
Apache License 2.0
206 stars 224 forks source link

Permissive TX RLP deserialization causes early signing dialog when APDU packet boundary occurs immediately before chain ID #409

Open prestwich opened 1 year ago

prestwich commented 1 year ago

Description

During tx signing packet exchange, when an APDU packet boundary occurs immediately before the chain-id in an unsigned RLP-serialized EIP155 transaction, the Ledger device completes deserialization of the incomplete RLP bytestring. This prevents the ledger from accepting the last packet, containing the chain ID. The Chain ID is then set to 0 during the user-facing signing flow, because it has not been received by the ledger device. This results in the user accidentally signing valid non-EIP155 transactions, and potential replay on other chains.

This seems to be a misimplementation of RLP within the ledger app on the device, as it completes deserialization and begins the signing flow, despite not receiving a valid RLP bytestring.

Packets sent to the device:

{ ins: 4, p1: 0, p2: 0, data: "058000002c8000003c800000000000000000000000f901e980830f4240830205b5943cd1dfb81c50a5300c60a181ed145a7286d81e0a80b901c4183fb413000000000000000000000000794a61358d6845594f94dc1db02a252b5b4814ad0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000", response_len: None }

{ ins: 4, p1: 128, p2: 0, data: "000000000000000000000000000000000000000000000000000000000001400000000000000000000000000000000000000000000000000000000000000180000000000000000000000000000000000000000000000000000000000000000b41544f4b454e5f494d504c000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000b41544f4b454e5f494d504c00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000043078303000000000000000000000000000000000000000000000000000000000", response_len: None }

Third pakcet queued, but unsent, as the ledger device entered signing flow and did not send a response:

{ ins: 4, p1: 128, p2: 0, data: "0a0000", response_len: None }

After sending these packets to the device, the device starts the user signing flow.

Invalid transaction RLP payload the ledger received:

0xf901e980830f4240830205b5943cd1dfb81c50a5300c60a181ed145a7286d81e0a80b901c4183fb413000000000000000000000000794a61358d6845594f94dc1db02a252b5b4814ad0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000001400000000000000000000000000000000000000000000000000000000000000180000000000000000000000000000000000000000000000000000000000000000b41544f4b454e5f494d504c000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000b41544f4b454e5f494d504c00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000043078303000000000000000000000000000000000000000000000000000000000

Valid transaction RLP payload that ought to have been received:

f901e980830f4240830205b5943cd1dfb81c50a5300c60a181ed145a7286d81e0a80b901c4183fb413000000000000000000000000794a61358d6845594f94dc1db02a252b5b4814ad0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000001400000000000000000000000000000000000000000000000000000000000000180000000000000000000000000000000000000000000000000000000000000000b41544f4b454e5f494d504c000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000b41544f4b454e5f494d504c000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000430783030000000000000000000000000000000000000000000000000000000000a0000

Transaction RLP with chunk boundaries inserted:

f901e980830f4240830205b5943cd1dfb81c50a5300c60a181ed145a7286d81e0a80b901c4183fb413000000000000000000000000794a61358d6845594f94dc1db02a252b5b4814ad0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000

000000000000000000000000000000000000000000000000000000000001400000000000000000000000000000000000000000000000000000000000000180000000000000000000000000000000000000000000000000000000000000000b41544f4b454e5f494d504c000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000b41544f4b454e5f494d504c00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000043078303000000000000000000000000000000000000000000000000000000000

0a0000

Notice the 3 bytes missing (!)

Your environment

Steps to reproduce

Issue location:

here, I believe, but I'm not familiar enough with the internals to say definitively

Expected behaviour

Ledger app should deserialize only valid RLP, and wait for additional packets if RLP is invalid

Actual behaviour

Ledger app deserializes invalid RLP

Proposed solution

Change RLP deserializer to prevent successful deserialization of invalid input

adrienlacombe commented 1 year ago

thank you @prestwich could this help? https://github.com/LedgerHQ/ledger-live/blob/11e4c3a0279612a8119e66bd4e3ebefcd689e417/libs/ledgerjs/packages/hw-app-eth/src/Eth.ts#L253

prestwich commented 1 year ago

We are applying a similar mitigation

Are you saying this is acknowledged and wontfix? Accepting invalid RLP seems like bad behavior in general

prestwich commented 1 year ago

ping on this, as people are still discovering this issue in the wild