074.md

Straight Cinnabar Grasshopper

Medium

Wrong implementation of EIP712 in VVVVCTokenDistributor::_isSignatureValid function

Summary

VVVVCTokenDistributor::_isSignatureValid is used to check whether a signature is valid or not. Due a to a failure in encoding the arrays in the params, the function will behave unexpectedly and will put the protocol in serious risk.

Root Cause

The function _isSignatureValid takes a ClaimParams memory _params struct as input, which contains the following fields:

https://github.com/sherlock-audit/2024-11-vvv-exchange-update/blob/main/vvv-platform-smart-contracts/contracts/vc/VVVVCTokenDistributor.sol#L46C2-L54C6

    struct ClaimParams {
        address kycAddress;
        address projectTokenAddress;
        address[] projectTokenProxyWallets;
        uint256[] tokenAmountsToClaim;
        uint256 nonce;
        uint256 deadline;
        bytes signature;
    }

address[] projectTokenProxyWallets and uint256[] tokenAmountsToClaim are arrays. According to the EPI712 specification, arrays should be encoded following this:

The array values are encoded as the keccak256 hash of the concatenated encodeData of their contents (i.e. the encoding of SomeType[5] is identical to that of a struct containing five members of type SomeType).

You can read the specification here.

When we look at the function, we see that that is not the case, and projectTokenProxyWallets and tokenAmountsToClaim arrays are passed to the struct without being encoded themselves first.

https://github.com/sherlock-audit/2024-11-vvv-exchange-update/blob/main/vvv-platform-smart-contracts/contracts/vc/VVVVCTokenDistributor.sol#L152C3-L182C1

    /**
     * @notice Checks if the provided signature is valid
     * @param _params A ClaimParams struct containing the investment parameters
     * @return true if the signer address is recovered from the signature, false otherwise
     */
    function _isSignatureValid(ClaimParams memory _params) private view returns (bool) {
        bytes32 digest = keccak256(
            abi.encodePacked(
                "\x19\x01",
                DOMAIN_SEPARATOR,
                keccak256(
                    abi.encode(
                        CLAIM_TYPEHASH,
                        _params.kycAddress,
                        _params.projectTokenAddress,
         @>             _params.projectTokenProxyWallets,
         @>             _params.tokenAmountsToClaim,
                        _params.nonce,
                        _params.deadline
                    )
                )
            )
        );

        address recoveredAddress = ECDSA.recover(digest, _params.signature);

        bool isSigner = recoveredAddress == signer;
        bool isExpired = block.timestamp > _params.deadline;
        return isSigner && !isExpired;
    }

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

This will result in the improper functioning of signature checks and can seriously hinder the functionality of the protocol.

PoC

As mentioned, refer to the EIP712 specification for more details => https://eips.ethereum.org/EIPS/eip-712#definition-of-encodedata

Here's a valid issue that talks about the same problem => https://solodit.cyfrin.io/issues/m-14-encodeddata-argument-of-hashstruct-is-not-calculated-perfectly-for-eip712-singed-messages-in-cultureindexsol-code4rena-collective-collective-git

Also check this Ethereum Stack Exchange discussion => https://ethereum.stackexchange.com/questions/125105/signing-an-array-whit-eth-signtypeddata-v4

Mitigation

The recommended solution is to encode the arrays before passing them to the struct. Change the function to this.

 function _isSignatureValid(ClaimParams memory _params) private view returns (bool) {
        bytes32 digest = keccak256(
            abi.encodePacked(
                "\x19\x01",
                DOMAIN_SEPARATOR,
                keccak256(
                    abi.encode(
                        CLAIM_TYPEHASH,
                        _params.kycAddress,
                        _params.projectTokenAddress,
         @>             keccak256(abi.encodePacked(_params.projectTokenProxyWallets)),
         @>             keccak256(abi.encodePacked(_params.tokenAmountsToClaim)),
                        _params.nonce,
                        _params.deadline
                    )
                )
            )
        );

        address recoveredAddress = ECDSA.recover(digest, _params.signature);

        bool isSigner = recoveredAddress == signer;
        bool isExpired = block.timestamp > _params.deadline;
        return isSigner && !isExpired;
    }