Squilliam - [M-3] Unused return value from `GRAPH.createEdge` in `Edition.publish` (Incorrect Error Handling + Potential Inconsistency)

[sherlock-admin3]

Squilliam

medium

[M-3] Unused return value from GRAPH.createEdge in Edition.publish (Incorrect Error Handling + Potential Inconsistency)

Summary

The Edition.publish function calls GRAPH.createEdge to create an edge in the TitlesGraph contract, but it ignores the return value from the createEdge function. This can lead to incorrect error handling and potential inconsistencies in the graph structure if the edge creation fails silently.

Vulnerability Detail

Add this code to Edition.t.sol

contract MockGraph {
    function createEdge(Node memory, Node memory, bytes memory) external returns (bool) {
        return false; // Always return false to simulate failure
    }
}

contract EditionTest is Test {
    // ...

    function testExploit_UnusedReturnValue() public {
        // Create a new MockGraph instance
        MockGraph mockGraph = new MockGraph();

        // Deploy a new Edition contract with the MockGraph instance
        Edition exploitedEdition = new Edition();
        exploitedEdition.initialize(
            feeManager,
            TitlesGraph(address(mockGraph)),
            address(this),
            address(this),
            Metadata({label: "Exploited Edition", uri: "ipfs.io/exploited-edition", data: new bytes(0)})
        );

        // Call the publish function
        exploitedEdition.publish(
            address(1),
            10,
            0,
            0,
            new Node[](0),
            Strategy({
                asset: address(0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE),
                mintFee: 0.01 ether,
                revshareBps: 2500,
                royaltyBps: 250
            }),
            Metadata({label: "Unused Return Value", uri: "ipfs.io/unused-return-value", data: new bytes(0)})
        );

        // The test passes if no assertion fails, indicating that the return value is not being used
    }
}

In this test case:

1. We create a MockGraph contract that always returns false from the createEdge function.
2. We deploy a new Edition contract with the MockGraph instance.
3. We call the publish function on the exploitedEdition contract.
4. The test passes, proving that the return value from createEdge is not being used.

Impact

If the createEdge function in the TitlesGraph contract returns false or encounters an error, the Edition.publish function will continue executing without handling the failure case. This can result in an inconsistent state where the Edition.sol contract believes the edge was created successfully, but it was actually not created in the TitlesGraph.

This vulnerability can cause:

1. Inconsistencies in the graph structure.
2. It could result in broken relationships between nodes, where the Edition.sol contract assumes that an edge has been created successfully, but in reality, it hasn't been reflected in the TitlesGraph
3. Unintended behavior and other security vulnerabilities
4. Even if the inconsistencies in the graph structure do not lead to critical issues, (which it probably will, especially over time) they can still increase the maintenance and debugging efforts for the development team.

Code Snippet

This vulnerability is from line 127 in the Edition.sol contract and can be found here: https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/editions/Edition.sol?plain=1#L127

Tool used

Foundry Manual Review

Recommendation

To mitigate this vulnerability, ensure that all the return values of the function calls are used.