sherlock-audit / 2024-04-titles-judging

9 stars 6 forks source link

ZanyBonzy - Functions requiring signatures will fail for EIP712 signers #260

Closed sherlock-admin3 closed 4 months ago

sherlock-admin3 commented 4 months ago

ZanyBonzy

medium

Functions requiring signatures will fail for EIP712 signers

Summary

The hashed digest for checking signature is not fully EIP-712 compliant which will lead to integration issues for EIP-712 signers.

Vulnerability Detail

The TitleGraphs contract allow users to acknowledge/unacknowledge an edge using an ECDSA signature. The issue is that the signature is not EIP-712 compliant causing that EIP-712 compliant signers will not be able to acknowledge and unacknowledge edges as the function will revert.

    modifier checkSignature(bytes32 edgeId, bytes calldata data, bytes calldata signature) {
        bytes32 digest = _hashTypedData(keccak256(abi.encode(ACK_TYPEHASH, edgeId, data))); //@note
        if (
            !edges[edgeId].to.creator.target.isValidSignatureNowCalldata(digest, signature)
                || _isUsed[keccak256(signature)]
        ) {
            revert Unauthorized();
        }
        _;
        _isUsed[keccak256(signature)] = true;
    }

Accroding to EIP-712, the data parameter which is being included as part of the signature, is a dynamic type, which the EIP explains that it should be encoded as the hash of the contents; As it stands, the data parameter is being encoded as-is.

Impact

The data being signed is not being encoded as per the EIP-712 specification, which will result in unexpected integration failures with EIP712-compliant signers that perform the encoding in the appropriate way resulting in them not being able to use the acknowledgeEdge and unacknowledgeEdge functions.

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/d7f60952df22da00b772db5d3a8272a988546089/wallflower-contract-v2/src/graph/TitlesGraph.sol#L41 https://github.com/sherlock-audit/2024-04-titles/blob/d7f60952df22da00b772db5d3a8272a988546089/wallflower-contract-v2/src/graph/TitlesGraph.sol#L120 https://github.com/sherlock-audit/2024-04-titles/blob/d7f60952df22da00b772db5d3a8272a988546089/wallflower-contract-v2/src/graph/TitlesGraph.sol#L148

Tool used

Manual Code Review

Recommendation

Encode the dynamic data parameter as per the EIP-712 specification.

    modifier checkSignature(bytes32 edgeId, bytes calldata data, bytes calldata signature) {
        bytes32 digest = _hashTypedData(keccak256(abi.encode(ACK_TYPEHASH, edgeId, keccak256(data))));
...
    }

Duplicate of #74