code-423n4 / 2021-09-sushitrident-findings

0 stars 0 forks source link

Use of ecrecover is susceptible to signature malleability #51

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

0xRajeev

Vulnerability details

Impact

The ecrecover function is used to verify and execute EIP-2612 permit transactions. The built-in EVM precompile ecrecover is susceptible to signature malleability (because of non-unique s and v values) which could lead to replay attacks (references: https://swcregistry.io/docs/SWC-117, https://swcregistry.io/docs/SWC-121 and https://medium.com/cryptronics/signature-replay-vulnerabilities-in-smart-contracts-3b6f7596df57).

While this is not exploitable for replay attacks in the current implementation because of the use of nonces, this may become a vulnerability if used elsewhere.

Proof of Concept

https://github.com/sushiswap/trident/blob/a03cd5d1945dc077cfb1632670271286a1eca6ab/contracts/pool/TridentERC20.sol#L108-L110

Tools Used

Manual Analysis

Recommended Mitigation Steps

Consider using OpenZeppelin’s ECDSA library (which prevents this malleability) instead of the built-in function: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/cryptography/ECDSA.sol

alcueca commented 3 years ago

Can't find a duplicate, @maxsam4?

maxsam4 commented 3 years ago

Can't find the duplicate but the issue is not relevant anyway. We mitigated the issue by using a nonce, as mentioned in the issue itself.

alcueca commented 3 years ago

Wouldn't be cheaper to use OpenZeppelin's version? Updating the nonce is an SSTORE.