code-423n4 / 2023-07-lens-findings

0 stars 0 forks source link

Implementation error of EIP-712 due to wrong Typehash can lead to tx reverts #107

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-lens/blob/main/contracts/libraries/constants/Typehash.sol#L23 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/libraries/constants/Typehash.sol#L15 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/libraries/constants/Typehash.sol#L25 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/libraries/MetaTxLib.sol#L147-L148 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/libraries/MetaTxLib.sol#L245-L246 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/libraries/MetaTxLib.sol#L275-L276 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/libraries/constants/Types.sol#L181-L182 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/libraries/constants/Types.sol#L210-L211 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/libraries/constants/Types.sol#L239-L240

Vulnerability details

Impact

Signed transactions implementing EIP-712 signatures correctly will revert as their on-chain counterpart will be different.

Affected functions:

Proof of Concept

The functions postWithSig(), commentWithSig(), and quoteWithSig() from LensHub all use EIP-712 to validate the owner/executor signature.

The contract implements incorrectly the Typehash for the EIP-712 signature, which will not be able to validate correct signatures, thus reverting those transactions.

The error is on the Typehashes assuming address collectModule, and bytes collectModuleInitData, instead of the corresponding address[] actionModules, and bytes[] actionModulesInitDatas, which have different types.

Note the use of address collectModule, and bytes collectModuleInitData on the Typehashes:

bytes32 constant POST = keccak256('Post(uint256 profileId,string contentURI,address collectModule,bytes collectModuleInitData,address referenceModule,bytes referenceModuleInitData,uint256 nonce,uint256 deadline)');

bytes32 constant COMMENT = keccak256('Comment(uint256 profileId,string contentURI,uint256 pointedProfileId,uint256 pointedPubId,uint256[] referrerProfileIds,uint256[] referrerPubIds,bytes referenceModuleData,address collectModule,bytes collectModuleInitData,address referenceModule,bytes referenceModuleInitData,uint256 nonce,uint256 deadline)');

bytes32 constant QUOTE = keccak256('Quote(uint256 profileId,string contentURI,uint256 pointedProfileId,uint256 pointedPubId,uint256[] referrerProfileIds,uint256[] referrerPubIds,bytes referenceModuleData,address collectModule,bytes collectModuleInitData,address referenceModule,bytes referenceModuleInitData,uint256 nonce,uint256 deadline)');

These are the functions where the corresponding digests are calculated:

    function validatePostSignature(Types.EIP712Signature calldata signature, Types.PostParams calldata postParams) // @audit check
        external
    {
        _validateRecoveredAddress(
            _calculateDigest(
                keccak256(
                    abi.encode(
                        Typehash.POST,
                        postParams.profileId,
                        keccak256(bytes(postParams.contentURI)),
@>                      postParams.actionModules,                                       // @audit address[] actionModules
@>                      _hashActionModulesInitDatas(postParams.actionModulesInitDatas), // @audit bytes[] actionModulesInitDatas
                        postParams.referenceModule,
                        keccak256(postParams.referenceModuleInitData),
                        _getAndIncrementNonce(signature.signer),
                        signature.deadline
                    )
                )
            ),
            signature
        );
    }

    function validateCommentSignature(
        Types.EIP712Signature calldata signature,
        Types.CommentParams calldata commentParams
    ) external {
        bytes32 contentURIHash = keccak256(bytes(commentParams.contentURI));
        bytes32 referenceModuleDataHash = keccak256(commentParams.referenceModuleData);
        bytes32 actionModulesInitDataHash = _hashActionModulesInitDatas(commentParams.actionModulesInitDatas);
        bytes32 referenceModuleInitDataHash = keccak256(commentParams.referenceModuleInitData);
        uint256 nonce = _getAndIncrementNonce(signature.signer);
        uint256 deadline = signature.deadline;
        bytes memory encodedAbi = _abiEncode(
            ReferenceParamsForAbiEncode(
                Typehash.COMMENT,
                commentParams.profileId,
                contentURIHash,
                commentParams.pointedProfileId,
                commentParams.pointedPubId,
                commentParams.referrerProfileIds,
                commentParams.referrerPubIds,
                referenceModuleDataHash,
@>              commentParams.actionModules,   // @audit address[] actionModules
@>              actionModulesInitDataHash,     // @audit bytes[] actionModulesInitDatas
                commentParams.referenceModule,
                referenceModuleInitDataHash,
                nonce,
                deadline
            )
        );
        _validateRecoveredAddress(_calculateDigest(keccak256(encodedAbi)), signature);
    }

    function validateQuoteSignature(Types.EIP712Signature calldata signature, Types.QuoteParams calldata quoteParams)
        external
    {
        bytes32 contentURIHash = keccak256(bytes(quoteParams.contentURI));
        bytes32 referenceModuleDataHash = keccak256(quoteParams.referenceModuleData);
        bytes32 actionModulesInitDataHash = _hashActionModulesInitDatas(quoteParams.actionModulesInitDatas);
        bytes32 referenceModuleInitDataHash = keccak256(quoteParams.referenceModuleInitData);
        uint256 nonce = _getAndIncrementNonce(signature.signer);
        uint256 deadline = signature.deadline;
        bytes memory encodedAbi = _abiEncode(
            ReferenceParamsForAbiEncode(
                Typehash.QUOTE,
                quoteParams.profileId,
                contentURIHash,
                quoteParams.pointedProfileId,
                quoteParams.pointedPubId,
                quoteParams.referrerProfileIds,
                quoteParams.referrerPubIds,
                referenceModuleDataHash,
@>              quoteParams.actionModules,   // @audit address[] actionModules
@>              actionModulesInitDataHash,   // @audit bytes[] actionModulesInitDatas
                quoteParams.referenceModule,
                referenceModuleInitDataHash,
                nonce,
                deadline
            )
        );
        _validateRecoveredAddress(_calculateDigest(keccak256(encodedAbi)), signature);
    }

These are the corresponding type declarations:

    struct PostParams {
        uint256 profileId;
        string contentURI;
@>      address[] actionModules;
@>      bytes[] actionModulesInitDatas;
        address referenceModule;
        bytes referenceModuleInitData;
    }

    struct CommentParams {
        uint256 profileId;
        string contentURI;
        uint256 pointedProfileId;
        uint256 pointedPubId;
        uint256[] referrerProfileIds;
        uint256[] referrerPubIds;
        bytes referenceModuleData;
@>      address[] actionModules;
@>      bytes[] actionModulesInitDatas;
        address referenceModule;
        bytes referenceModuleInitData;
    }

    struct QuoteParams {
        uint256 profileId;
        string contentURI;
        uint256 pointedProfileId;
        uint256 pointedPubId;
        uint256[] referrerProfileIds;
        uint256[] referrerPubIds;
        bytes referenceModuleData;
@>      address[] actionModules;
@>      bytes[] actionModulesInitDatas;
        address referenceModule;
        bytes referenceModuleInitData;
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Fix the Typehashes for COMMENT, POST, and QUOTE:

Assessed type

Other

donosonaumczuk commented 1 year ago

This is a duplicate or subset from #141 - We proceed with the same resolution.

c4-sponsor commented 1 year ago

donosonaumczuk marked the issue as disagree with severity

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #141

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory