Description

The ExecuteOrCancelCompensate message handler allows the compensator entity to cancel an order and send funds to the order creator.

receive(msg: ExecuteOrCancelCompensate) {
    // [...]
    if(compensate.orderType != null) {
        if (compensate.orderType == ORDER_TYPE_LP) {
            self.liquidityOrders.del(compensate.orderId);
        } else {
            self.perpOrders.del(compensate.orderId);
            self.perpOrderExs.del(compensate.orderId);
        }
    }
    // [...]
}

However, it does not check to ensure an order exists. The Tact documentation↗ specifies that the map's del function returns a boolean indicating whether the key was present in the map. So, if the key is not present, the user will be compensated even if the order does not exist.

A consequence of this is that — between the time when the compensator entity signs a transaction that calls the ExecuteOrCancelCompensate handler and it is actually executed on chain — there is the opportunity for an order to be canceled, executed, or otherwise deleted from the relevant map.

Note that a user can cancel their own order, meaning one aspect of the race condition may be controlled by the user. If the user has knowledge that the compensator will cancel their order, they may try to race the compensator to cancel the order themselves and thereby receive the funds twice.

Impact

In the worst-case scenario, a malicious attacker may be able to receive a double payment for canceling their order, thereby stealing funds from the protocol. This may also happen by accident if the protocol is very active.

Recommendations

Assert that the calls to del return true:

receive(msg: ExecuteOrCancelCompensate) {
    // [...]
    if(compensate.orderType != null) {
        if (compensate.orderType == ORDER_TYPE_LP) {
-            self.liquidityOrders.del(compensate.orderId);
+            require(self.liquidityOrders.del(compensate.orderId), "order not found");
        } else {
-            self.perpOrders.del(compensate.orderId);
-            self.perpOrderExs.del(compensate.orderId);
+            require(self.perpOrders.del(compensate.orderId), "order not found");
+            require(self.perpOrderExs.del(compensate.orderId), "order not found");
        }
    }
    // [...]
}

Remediation

This finding was addressed in the TON pool contract in commit 68b5af↗, and in the USDT pool in commit 0d6693↗ by implementing the suggested recommendation.