search

code-423n4 / 2022-02-aave-lens-findings

0 stars 0 forks source link

QA Report #55

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Title

Use of ecrecover is susceptible to signature malleability

Impact

The _validateRecoveredAddress function of LensNFTBase contract calls the Solidity ecrecover function directly to verify the given signatures. However, the ecrecover EVM opcode allows malleable (non-unique) signatures and thus is susceptible to replay attacks.

Proof of Concept

https://github.com/aave/lens-protocol/blob/main/contracts/core/base/LensNFTBase.sol#L165

Recommended Mitigation Steps

Consider using OpenZeppelin’s ECDSA library (which prevents this malleability) instead of the built-in function.

Zer0dot commented 2 years ago

I don't believe this is valid because we increment nonces for each signature, so the modified signature cannot be replayed. @miguelmtzinf @LHerskind @donosonaumczuk Any input?

donosonaumczuk commented 2 years ago

Yes, as far as I understand, the mentioned malleability becomes a problem when signature uniqueness is required, as given a valid signature you can trivially generate another one that is also valid. But this should not be a problem in our case.

In our signature scheme, replay attacks within the same chain are prevented by the signer-nonce combination, while cross-chain replay attacks are prevented by the domain separator (contains chain ID, contract address, etc).

Zer0dot commented 2 years ago

Agreed @donosonaumczuk