sherlock-audit / 2024-04-titles-judging

1 stars 1 forks source link

KupiaSec - The function `TitlesGraph._setAcknowledged()` doesn't function properly due to its reliance on a memory variable #378

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 2 months ago

KupiaSec

high

The function TitlesGraph._setAcknowledged() doesn't function properly due to its reliance on a memory variable

Summary

TitlesGraph._setAcknowledged() doesn't work becuase it rewrites the memory variable edge instead of the storage variable.

Vulnerability Detail

At L205 of TitlesGraph._setAcknowledged(), the variable edge is a memory variable, so rewriting edge doesn't change the storage variable edges[edgeId_].

    function _setAcknowledged(bytes32 edgeId_, bytes calldata data_, bool acknowledged_)
        internal
        returns (Edge memory edge)
    {
        if (!_edgeIds.contains(edgeId_)) revert NotFound();
205     edge = edges[edgeId_];
        edge.acknowledged = acknowledged_;

        if (acknowledged_) {
            emit EdgeAcknowledged(edge, msg.sender, data_);
        } else {
            emit EdgeUnacknowledged(edge, msg.sender, data_);
        }
    }

Impact

TitlesGraph._setAcknowledged() never works and results in the incorrect action of the entire system.

Code Snippet

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

Tool used

Manual Review

Recommendation

TitlesGraph._setAcknowledged() should be fixed as follows.

    function _setAcknowledged(bytes32 edgeId_, bytes calldata data_, bool acknowledged_)
        internal
        returns (Edge memory edge)
    {
        if (!_edgeIds.contains(edgeId_)) revert NotFound();
        edge = edges[edgeId_];
        edge.acknowledged = acknowledged_;
+       edges[edgeId_] = edge;

        if (acknowledged_) {
            emit EdgeAcknowledged(edge, msg.sender, data_);
        } else {
            emit EdgeUnacknowledged(edge, msg.sender, data_);
        }
    }

Duplicate of #212