sherlock-audit / 2023-06-dinari-judging

5 stars 4 forks source link

chainNue - `requestOrder` lack of slippage (mint & burn) #72

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

chainNue

high

requestOrder lack of slippage (mint & burn)

Summary

requestOrder lack of slippage (mint & burn)

Vulnerability Detail

User call requestOrder believing Operator will fillOrder in short time. Issue might happen when Operator delayed to fulfil the order, meanwhile price has change to unfavourable position for user to buy (or sell)

File: OrderProcessor.sol
46:     // Specification for an order
47:     struct OrderRequest {
48:         // Recipient of order fills
49:         address recipient;
50:         // Bridged asset token
51:         address assetToken;
52:         // Payment token
53:         address paymentToken;
54:         // Amount of incoming order token to be used for fills
55:         uint256 quantityIn;
56:         // price enquiry for the request
57:         uint256 price;
58:     }

Lack of slippage check from requestOrder will make user's order will be fulfilled in an undesirable price or time. Since Order is created with GTC the only way to stop users 'pending' order is through requestCancel which it also need to wait until operator execute it.

In a scenario where a sudden price movement of dShares, while user is not aware of this, will put user into losing state. For example:

  1. User create requestOrder when 1 dShares is 10 USDC thinking it will never be below 5 USDC and above 20 USDC.
  2. If there is a sudden price change resulting 1 dShares now 1 USDC, user might think this is dumped, and don't want to fill the order. Or in reverse if the dShares now 100 USDC, user may think it's overprice.
  3. Since the Order doesn't have any slippage protection mechanism, the previous condition will happen.
File: BuyOrderIssuer.sol
121:         // Construct order
122:         order = Order({
123:             recipient: orderRequest.recipient,
124:             assetToken: orderRequest.assetToken,
125:             paymentToken: orderRequest.paymentToken,
126:             // Buy order
127:             sell: false,
128:             // Market order
129:             orderType: OrderType.MARKET,
130:             assetTokenQuantity: 0,
131:             // Hold fees back from order amount
132:             paymentTokenQuantity: orderRequest.quantityIn - totalFees,
133:             price: orderRequest.price,
134:             // Good until cancelled
135:             tif: TIF.GTC,
136:             // Emit fees held back from order amount
137:             fee: totalFees
138:         });

This issue also open on the other side, selling (burning) dShares, which doesn't have any slippage protection.

Impact

User's order will fulfilled in undesirable price, losing user's asset

Code Snippet

https://github.com/sherlock-audit/2023-06-dinari/blob/main/sbt-contracts/src/issuer/OrderProcessor.sol#L244

Tool used

Manual Review

Recommendation

Implement slippage protection mechanism for Order (buy / sell)