maushish - Lack of proper cross-chain EIP-712 parameters could lead to wrong edges getting acknowledged.

[sherlock-admin4]

maushish

medium

Lack of proper cross-chain EIP-712 parameters could lead to wrong edges getting acknowledged.

Summary

In the current implementation of checkSignature modifier there is no involvement of chain-id , nonce parameters due to which malicious actor could replay a signature and either unacknowledge or acknowledge an edge.

Vulnerability Detail

As clearly mentioned in the readme file

The current implementation of checkSignature follows a modified version of EIP-712
https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/graph/TitlesGraph.sol#L40

    /// @notice Modified to check the signature for a proxied acknowledgment.
    modifier checkSignature(
        bytes32 edgeId,
        bytes calldata data,
        bytes calldata signature
    ) {
        bytes32 digest = _hashTypedData(
            keccak256(abi.encode(ACK_TYPEHASH, edgeId, data))// message with /x19... prefix
        );
        if (
            !edges[edgeId].to.creator.target.isValidSignatureNowCalldata( digest,signature) || _isUsed[keccak256(signature)]
        ) {
            revert Unauthorized();
        }
        _;
        _isUsed[keccak256(signature)] = true;
    }

Because the chain ID is not included in the data, all signatures are also valid when the project is launched on a chain with another chain ID.
Signature without chain-id, nonces are not safe along with the standard specified in EIP 712.

Impact

Mentioned in the summary.

Code Snippet

    function acknowledgeEdge(
        bytes32 edgeId_,
        bytes calldata data_,
        bytes calldata signature_
    )
        external
        checkSignature(edgeId_, data_, signature_)
        returns (Edge memory edge)
    {
        return _setAcknowledged(edgeId_, data_, true);
    }

Tool used

Manual Review

Recommendation

Implement the use of nonce and chain-id in checkSignature modifier.

[ccashwell]

This is not valid. Indeed, the chainId is being validated as part of the EIP712 domain.