Signature Malleability in `MetaTxLib.sol`

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-lens/blob/cdef6ebc6266c44c7068bc1c4c04e12bf0d67ead/contracts/libraries/MetaTxLib.sol#L469-L483

Vulnerability details

Impact

Replay attacks are possible thanks to signature malleability.

Proof of Concept

• The ecrecover EVM opcode has a limitation that allows for non-unique signatures, which means that different signatures can produce the same output.
• According to EIP-2, any transaction signatures with an s-value greater than secp256k1n/2 are considered invalid. However, validating such signatures by passing them to the _verify function is still possible.
• To create a malleable signature, one can flip the s value from s to secp256k1n - s and also flip the v value (27 -> 28, 28 -> 27). Surprisingly, the resulting signature would still be valid.
• MetaTxLib.sol#_validateRecoveredAddress() is used as a wrapper for ecrecover to reduce code size, used in meta-tx specific functions. It is called in functions like validateSetProfileMetadataURISignature , validateSetFollowModuleSignature, validateChangeDelegatedExecutorsConfigSignature, validateSetProfileImageURISignature, validatePostSignature, validateCommentSignature, validateMirrorSignature, validateBurnSignature, validateFollowSignature, validateUnfollowSignature, validateSetBlockStatusSignature, validateLegacyCollectSignature, validateActSignature.

  File: 2023-07-lens/contracts/libraries/MetaTxLib.sol
  function _validateRecoveredAddress(bytes32 digest, Types.EIP712Signature calldata signature) private view {
      if (signature.deadline < block.timestamp) revert Errors.SignatureExpired();
      // If the expected address is a contract, check the signature there.
      if (signature.signer.code.length != 0) {
          bytes memory concatenatedSig = abi.encodePacked(signature.r, signature.s, signature.v);
          if (IERC1271(signature.signer).isValidSignature(digest, concatenatedSig) != EIP1271_MAGIC_VALUE) {
              revert Errors.SignatureInvalid();
          }
      } else {
          address recoveredAddress = ecrecover(digest, signature.v, signature.r, signature.s);
          if (recoveredAddress == address(0) || recoveredAddress != signature.signer) {
              revert Errors.SignatureInvalid();
          }
      }
  }

Tools Used

Manual review

Recommended Mitigation Steps

To address this issue, OpenZeppelin has taken measures to handle malleable signatures. You can find more information on how they tackled this problem at this GitHub link: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/cryptography/ECDSA.sol#L125.

Assessed type

Access Control

[c4-pre-sort]

141345 marked the issue as duplicate of #52

[Picodes]

nonce is incremented to prevent replay attacks

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid