sherlock-audit / 2024-04-titles-judging

9 stars 6 forks source link

fibonacci - TitlesGraph's acknowledge/unacknowledge edge functions are vulnerable to signature malleability #228

Closed sherlock-admin3 closed 4 months ago

sherlock-admin3 commented 4 months ago

fibonacci

high

TitlesGraph's acknowledge/unacknowledge edge functions are vulnerable to signature malleability

Summary

Valid signatures could be crafted by a malicious user to replay previously signed messages.

Vulnerability Detail

A user can acknowledge an edge using an ECDSA signature.

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);
}

The signed message hash is used to check if previously messages have been processed by the contract.

_isUsed[keccak256(signature)] = true;

A malicious user can slightly modify the three values v, r, and s to create other valid signatures. In this case, the signature will be valid, but the hash will be different, allowing the check above to pass. (Reference)

To validate the signature, TitlesGraph utilizes SignatureCheckerLib by solady, which does not include a check for non-malleability, assuming that the signed message will contain a nonce. (Reference)

However, it is observed that the nonce is not included in the message.

bytes32 digest = _hashTypedData(keccak256(abi.encode(ACK_TYPEHASH, edgeId, data)));

Impact

The state of acknowledging or unacknowledging an edge can be reset by a malicious user.

Example:

  1. A user wishes to acknowledge an edge. To do this, they create a signature (1).
  2. The user then changed their mind and decided to unacknowledge the edge. To do this, they create a new signature (2).
  3. An attacker can modify the signature created in step 1 and use it to make the edge acknowledged again.
  4. If the user repeats their attempt to unacknowledge the edge, the attacker can again invert the state, this time by modifying the signature created in step 2. And so on.

Code Snippet

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

Tool used

Manual Review

Recommendation

Include the nonce in the signed message.

Duplicate of #279

ccashwell commented 4 months ago

This is only possible if the original caller uses a signature with an S-value in the vulnerable range, which no commonly used Ethereum client libs allow. Won't fix.

0xf1b0 commented 3 months ago

Escalate

It is a duplicate of issue #279, not #284. Additionally, issue #279 is not a duplicate of issue #273.

sherlock-admin3 commented 3 months ago

Escalate

It is a duplicate of issue #279, not #284. Additionally, issue #279 is not a duplicate of issue #273.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

WangSecurity commented 3 months ago

Agree with the escalation, planning to accept and duplicate with #279

Evert0x commented 3 months ago

Result: Medium Duplicate of #279

sherlock-admin2 commented 3 months ago

Escalations have been resolved successfully!

Escalation status: