code-423n4 / 2022-06-putty-findings

5 stars 0 forks source link

Users can cancel irrelevant orders which do not exist #405

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L526-L532

Vulnerability details

Impact

Since cancel function does not have enough checks, users can cancel irrelevant orders which do not even exist.

Proof of Concept

https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L526-L532

    function cancel(Order memory order) public {
        require(msg.sender == order.maker, "Not your order");

        bytes32 orderHash = hashOrder(order);

        // mark the order as cancelled
        cancelledOrders[orderHash] = true;

cancel function just checks if msg.sender is order.maker. Therefore users can specify arbitral values at Order struct as long as order.maker has same value with msg.sender. This seems not cause stealing funds or other major issues, but it looks like an unexpected behavior.

Tools Used

Static analysis

Recommended Mitigation Steps

Simply add a check to confirm if the order itself actually exists by checking ownerOf function or other ways.

berndartmueller commented 2 years ago

As orders are created off-chain, there is no way to check the order's existence prior to canceling.

GalloDaSballo commented 2 years ago

Finding is technically valid, a user can set their own "fake" orders to cancelled, impact is that they pay gas for it

outdoteth commented 2 years ago

There is no concrete exploit provided here. It is expected behaviour that any Order can be cancelled.

HickupHH3 commented 2 years ago

no assets are at risk, only impact is user self-inflicting harm. thus, finding is invalid.