dmihal / eth-permit

Lightweight library for signing ERC-2612 permit signatures.
https://www.npmjs.com/package/eth-permit
139 stars 30 forks source link

Bug when signing permits on a Ledger wallet #17

Open scorpion9979 opened 1 year ago

scorpion9979 commented 1 year ago

I noticed that there is a bug in splitSignatureToRSV function when attempting to sign permits via a Ledger wallet and then trying to pass the resulting signature to an on-chain Ethereum contract method that consumes it. This is due to how Ledger produces vrs signatures with a canonical v value of {0,1}, while Ethereum's ecrecover accepts a non-standard v value of {27,28}. More on that here and here.

Metamask handles this internally when signing a permit directly through it, since it uses ecsign that normalizes the v value for Ethereum. But the problem is when signing a permit internally on a Ledger and then relay the resulting signature to Metamask, the v value is still {0,1}.

This could be resolved by replacing splitSignatureToRSV by ether's splitSignature, since it handles normalizing the resulting v value to Ethereum (i.e., 0 -> 27, 1 -> 28). Plus, it includes handling other special types signatures that are also not handled in splitSignatureToRSV's implementation (i.e., EIP-2098 signatures).