MetaMask / MetaMask/eth-ledger-bridge-keyring

Notice of possible breaking change for dapps relying on the result of EIP-712 signatures

Open
#178 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
84
Forks
122
PR merge metrics
No merged PRs in 30d

Description

In as soon as 4 weeks time, MetaMask will release a fix to our EIP-712 signing code for ledger devices, and this has the possibility of breaking dapps which explicitly relied on our raw signature return values.

As per the Ethereum Yellow Paper, the recovery identifier `v` - the last byte of the signature - should be either 27 (0x1b) or 28 (0x1c). However, since Metamask started supporting signatures for EIP-712 message, we have (incorrectly) returned signatures with a `v` value of either 0 or 1. While our signing logic is correct, and messages have been signed with the correct keys, the signature as returned to your dapp would have ended with the incorrect byte. To further illustrate the problem, some projects have had to explicitly work around this by explicitly checking the `v` value on the returned signature and if it matches 0 or 1, adding 27 to it.

A fix to this issue was made in a PR to our ledger keyring repo last summer: https://github.com/MetaMask/eth-ledger-bridge-keyring/pull/152/files This fix will be included in an upcoming release. With this fix, the last byte of the signature returned to your dapp, when a user with a Ledger account signs an EIP-712 message, will change.

For example, given the exact same inputs and signed with the exact same account, a signature that previously was returned as this `0x72d4e38a0e582e09a620fd38e236fe687a1ec782206b56d576f579c026a7e5b946759735981cd0c3efb02d36df28bb2feedfec3d90e408efc93f45b894946e3200` would now be returned as `0x72d4e38a0e582e09a620fd38e236fe687a1ec782206b56d576f579c026a7e5b946759735981cd0c3efb02d36df28bb2feedfec3d90e408efc93f45b894946e321b`

Note that recovery will not be impacted: if you could recover an address from any given signature before this change, you will still be able to recover an address after this change.

But this could break functionality of your dapp, for Ledger users, if you relied on the explicit signature hex string returned by MetaMask. In such a case, you may need to either migrate saved data or add logic that checks the last two bytes for either scenario (v being 0 or 1, OR v being 27 or 28).

This change will be released as early as 4 weeks from this posting.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with the linked eth-ledger-bridge-keyring PR #152 and inspect the Ledger EIP-712 signing path. Compare the old and new final recovery byte against the example signatures; done means the compatibility impact and any required handling for affected dapps are identified.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
blockchain, cryptography
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.