ni8mare - A large buy order is prone to a sandwich attack.

[sherlock-admin]

ni8mare

medium

A large buy order is prone to a sandwich attack.

Summary

As the title suggests, a large buy order is susceptible to a sandwich attack.

Vulnerability Detail

Consider that there is a big buy order submitted by a victim through requestOrder. This transaction is publicly seen by everyone while it is in the mempool. A frontrunner in turn could also submit a buy order, which increases the price of the asset for the victim and then back run the victim by immediately submitting a sell order, basically a sandwich attack. This would mean that the victim who submitted a big buy order is not able to get the intended amount of assets (or get enough underlying asset tokens minted) that they would have wanted.

This is based on the comments from the sponsor in the Sherlock Discord channel for Dinari, when asked whether the operators could fill the orders at their own discretion or if are they filled depending on their order of submission. This is what they said:

Regarding the order fulfillment, orders are filled in the order of submission

This means that the front runner's buy order will be filled first, then the victim's buy order and finally the sell order.

Impact

The victim who submitted a big buy order is not able to get the intended amount of assets (or get enough underlying asset tokens minted) that they would have wanted because their order is sandwiched.

Code Snippet

https://github.com/dinaricrypto/sbt-contracts/blob/6d36760def25449c3f35f6ed38128a7eaf352903/src/issuer/OrderProcessor.sol#L244

    function requestOrder(OrderRequest calldata orderRequest, bytes32 salt) public nonReentrant whenOrdersNotPaused {
        // Reject spam orders
        if (orderRequest.quantityIn == 0) revert ZeroValue();
        // Check for whitelisted tokens
        _checkRole(ASSETTOKEN_ROLE, orderRequest.assetToken);
        _checkRole(PAYMENTTOKEN_ROLE, orderRequest.paymentToken);
        bytes32 orderId = getOrderIdFromOrderRequest(orderRequest, salt);
        // Order must not already exist
        if (_orders[orderId].remainingOrder > 0) revert DuplicateOrder();

        // Get order from request and move tokens
        Order memory order = _requestOrderAccounting(orderRequest, orderId);

        // Send order to bridge
        emit OrderRequested(orderId, order.recipient, order, salt);

        // Initialize order state
        uint256 orderAmount = order.sell ? order.assetTokenQuantity : order.paymentTokenQuantity;
        _orders[orderId] = OrderState({requester: msg.sender, remainingOrder: orderAmount, received: 0});
        _numOpenOrders++;
    }

Tool used

Manual Review

Recommendation

This attack is possible because the user never inputs the minimum amount of underlying asset tokens that would want to be minted. So, it is recommended that the requestOrder function also take a minAmountToMint parameter from the user.