0x73696d616f - `TitlesGraph::checkSignature()` modifier does not implement EIP712 correctly as the `data` argument is not hashed in `ACK_TYPEHASH`

[sherlock-admin4]

0x73696d616f

medium

TitlesGraph::checkSignature() modifier does not implement EIP712 correctly as the data argument is not hashed in ACK_TYPEHASH

Summary

TitlesGraph::checkSignature() modifier computes the digest with _hashTypedData(keccak256(abi.encode(ACK_TYPEHASH, edgeId, data)));, but dynamic types must be hashed when computing the structHash.

Vulnerability Detail

EIP712 specification states exactly how all types should be encoded when calculating the structHash, but the protocol does not follow them in the TitlesGraph::checkSignature() modifier. From the EIP712 documentation:

The dynamic values bytes and string are encoded as a keccak256 hash of their contents.

As can be seen, the parameter data from ACK_TYPEHASH = keccak256("Ack(bytes32 edgeId,bytes data)"); is not hashed in the digest bytes32 digest = _hashTypedData(keccak256(abi.encode(ACK_TYPEHASH, edgeId, data)));.

Impact

Protocol does not follow EIP712 correctly, which is considered a valid issue according to the contest readme.

Is the codebase expected to comply with any EIPs? Can there be/are there any deviations from the specification? strict implementation of EIPs 1271 (Graph), 712 (Graph, Edition), 2981 (Edition), 1155 (Edition)

Code Snippet

TitlesGraph::checkSignature() modifier.

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

Tool used

Manual Review

Vscode

Recommendation

Hash data in the mentioned digest bytes32 digest = _hashTypedData(keccak256(abi.encode(ACK_TYPEHASH, edgeId, keccak256(data))));

Duplicate of #74