Use of ecrecover is susceptible to signature malleability

[code423n4]

Handle

0xRajeev

Vulnerability details

Impact

The ecrecover function is used to verify and execute Meta 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/code-423n4/2021-06-realitycards/blob/86a816abb058cc0ed9b6f5c4a8ad146f22b8034c/contracts/lib/NativeMetaTransaction.sol#L99-L104

https://github.com/code-423n4/2021-06-realitycards/blob/86a816abb058cc0ed9b6f5c4a8ad146f22b8034c/contracts/lib/NativeMetaTransaction.sol#L38-L48

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

[Splidge]

This issue will now be fixed, not because of the reasons discovered in this issue, but because of the reasons explained in #166

[Splidge]

Nevermind, issue #166 couldn't be solved using the recommended mitigation so this issue will remain unresolved for now.