sherlock-audit / 2024-04-titles-judging

9 stars 6 forks source link

cryptic - Attacker can front-run `acknowledgeEdge` and `unacknowledgeEdge`, causing DoS #218

Closed sherlock-admin3 closed 4 months ago

sherlock-admin3 commented 4 months ago

cryptic

medium

Attacker can front-run acknowledgeEdge and unacknowledgeEdge, causing DoS

Summary

TitlesGraph::acknowledgeEdge and TitlesGraph::unacknowledgeEdge and both susceptible to front-run attacks.

When a call to acknowledgeEdge is made, an attacker can extract the parameters, including the signature, and call unacknowledgeEdge instead. When the user's call to acknowledgeEdge is executed, it will revert since the signature has already been used, therefore user will not be able to acknowledge the edge. This can be done for unacknowledgeEdge as well, effectively causing DoS of acknowledging and unacknowledging edges via front-run.

Vulnerability Detail

TitlesGraph::acknowledgeEdge #L118

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

TitlesGraph::unacknowledgeEdge #L146

    function unacknowledgeEdge(bytes32 edgeId_, bytes calldata data_, bytes calldata signature_)
        external
@>      checkSignature(edgeId_, data_, signature_)
        returns (Edge memory edge)
    {
        return _setAcknowledged(edgeId_, data_, false);
    }

TitlesGraph::checkSignature

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

Both functions require signatures that is checked via the checkSignature modifier. If the signature has already been used, the call will revert. The problem is that both function calls can be front-run, allowing the following scenarios:

unacknowledgeEdge front-run: Attacker extracts parameters and calls acknowledgeEdge with the same signature. Once the call to unacknowledgeEdge is executed, it will revert since the signature has already been used. The caller will not be able to unacknowledge the edge.

acknowledgeEdge front-run: Attacker extracts parameters and calls unacknowledgeEdge with the same signature. Once the call to acknowledgeEdge is executed, it will revert since the signature has already been used. The caller will not be able to acknowledge the edge.

Impact

Denial of Service of acknowledging and unacknowledging edges, causing discrepancies and manipulation with the status of edges.

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/graph/TitlesGraph.sol#L118

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/graph/TitlesGraph.sol#L146

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/graph/TitlesGraph.sol#L40-L50

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/graph/TitlesGraph.sol#L200

Tool used

Manual Review

Recommendation

Consider implementing a check to see if caller is the signer

Duplicate of #273