fugazzi - Edges cannot be acknowledged

[sherlock-admin4]

fugazzi

medium

Edges cannot be acknowledged

Summary

The implementation to manage the acknowledgement of edges in the TitlesGraph contract is inherently broken.

Vulnerability Detail

The management of the acknowledged flag for edges in the TitlesGraph contract is done using the _setAcknowledged() internal function:

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

As we can see, the Edge structure is first copied from storage to memory (line 205) and then the new value is updated in this memory reference (line 206). There is no copy back from memory from storage, meaning the change isn't persisted.

Proof of Concept

function test_AcknowledgeEdge_broken() public {
    TitlesGraph graph = new TitlesGraph(address(this), address(this));

    address entityFrom = makeAddr("entityFrom");
    address creatorFrom = makeAddr("creatorFrom");
    (address creatorTo, uint256 creatorToPk) = makeAddrAndKey("creatorTo");

    Node memory from = Node({
        nodeType: NodeType.TOKEN_ERC1155,
        creator: Target({target: creatorFrom, chainId: block.chainid}),
        entity: Target({target: entityFrom, chainId: block.chainid}),
        data: abi.encode(1)
    });

    Node memory to = Node({
        nodeType: NodeType.ACCOUNT,
        creator: Target({target: creatorTo, chainId: block.chainid}),
        entity: Target({target: creatorTo, chainId: block.chainid}),
        data: ""
    });

    // Create edge
    vm.prank(entityFrom);
    graph.createEdge(from, to, "");

    bytes32 edgeId = keccak256(abi.encode(from, to));

    // edge is by default unack
    (,,bool acknowledged,) = graph.edges(edgeId);
    assertFalse(acknowledged);

    // creator acknowledges edge
    vm.prank(creatorTo);
    graph.acknowledgeEdge(edgeId, "");

    // edge is still unacked!
    (,,acknowledged,) = graph.edges(edgeId);
    assertFalse(acknowledged);
}

Impact

Contract functionality is broken as there is no way to handle acknowledgement of edges.

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

Modify the acknowledged field in storage, then copy the structure to memory.

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

    edges[edgeId_].acknowledged = acknowledged_;
    edge = edges[edgeId_];

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

Duplicate of #212