0x73696d616f - Manipulated `edge.acknowledged` due to signature applying both to `TitlesGraph::acknowledgeEdge()` and `TitlesGraph::unacknowledgeEdge()`

[sherlock-admin4]

0x73696d616f

medium

Manipulated edge.acknowledged due to signature applying both to TitlesGraph::acknowledgeEdge() and TitlesGraph::unacknowledgeEdge()

Summary

TitlesGraph::acknowledgeEdge() and TitlesGraph::unacknowledgeEdge() use exactly the same signature, which means that someone may frontrun one of these calls and do the opposite of the intended.

Vulnerability Detail

TitlesGraph::acknowledgeEdge() and TitlesGraph::unacknowledgeEdge() allow the creator or entity to modifiy the acknowledged status of an edge using a signature signed offchain. However, the same signature can be used in both functions, allowing attackers to spot the signature and frontrun the transaction in the mempool and set edge.acknowledged to the opposite value of what was intended.

The creator may overwrite the value later using one of TitlesGraph::acknowledgeEdge() or TitlesGraph::unacknowledgeEdge() that do not require a signature, but it would still leave a window of opportunity where the acknowledgement was gamed. As stated in the readme, issues showing manipulation of acknowledgement should be considered valid, demonstrating the relevance of this finding:

In addition to the security of funds, we would also like there to be focus on the sanctity of the data in the TitlesGraph and the permissioning around it (only the appropriate people/contracts can signal reference and acknowledgement of reference).

Impact

Manipulated edge.acknowledged value by an attacker.

Code Snippet

TitlesGraph::acknowledgeEdge() and TitlesGraph::unacknowledgeEdge()

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

Tool used

Manual Review

Vscode

Recommendation

Use a different typehash for each function. The arguments may remain the same (edgeId and data), changing the struct name from Ack to AcknowledgeEdge and UnacknowledgeEdge, should be enough to generate different signatures.

Duplicate of #273