[M-02] Cancel is allowed even after the order was filled

[code423n4]

Lines of code

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

Vulnerability details

A maker can call cancel on-chain regardless of whether the order was already filled or even exercised by a taker or not.

Impact/Effects

In case it was already filled, this oporation:

1. Emit a misleading CancelledOrder event,
2. Confuse the maker as his call to cancel was not reverted,
3. Make cancelledOrders mapping inconsistent by saving true value for this order hash.

Proof of Concept

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;

    emit CancelledOrder(orderHash, order);
}

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

Easy to verify that orders can be cancelled after they being filled.

Recommended Mitigation Steps

Change cancel code to:

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

    bytes32 orderHash = hashOrder(order);

    require(_ownerOf[uint256(orderHash)] == address(0), "Order already filled"); //owner is `address(0)` for unfilled orders, in contrast to `maker_addr` and `0xdead` for filled and exercised respectively

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

    emit CancelledOrder(orderHash, order);
}

[outdoteth]

Duplicate: Order can be cancelled even if order was already filled: https://github.com/code-423n4/2022-06-putty-findings/issues/396

[HickupHH3]

Part of warden's QA report: #429