sherlock-audit / 2023-06-dinari-judging

5 stars 4 forks source link

hals - Operators can take escrow of any cancelled order #71

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

hals

medium

Operators can take escrow of any cancelled order

Summary

Operators can take escrow of any cancelled order

Vulnerability Detail

In DirectBuyIssuer contract/_cancelOrderAccounting function: Since the escrow of the order is not deleted when cancelling the order;any operator can take the full amount of the escrow of any cancelled order.

Impact

The protocol will be losing from its paymentToken as escrows payment for deleted orders,which will lead to orders not being able to be filled or cancelled due to insufficient balance of paymentToken (USDC or USDT).

Code Snippet

    function _cancelOrderAccounting(OrderRequest calldata order, bytes32 orderId, OrderState memory orderState)
        internal
        virtual
        override
    {
        // Prohibit cancel if escrowed payment has been taken and not returned or filled
        uint256 escrow = getOrderEscrow[orderId];
        if (orderState.remainingOrder != escrow) revert UnreturnedEscrow();

        // Standard buy order accounting
        super._cancelOrderAccounting(order, orderId, orderState);
    }

Tool used

Manual Review

Recommendation

In _cancelOrderAccounting function: delete the escrow of the order: delete getOrderEscrow[orderId];

Duplicate of #56