Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

The function `createMarketOrder` should implement a price target protection just as `fillOffchainOrders` does

draiakoo

Summary

The function createMarketOrder should implement a price target protection just as fillOffchainOrders does

Vulnerability Details

When a user creates a market order onchain, the price that will be used to compute if the order is allowed will be using the index price of a Chainlink price feed. But when this order will be actually executed, the bid and ask price will be used instead which can vary significantly from the price that the user expected when he created the market order.
Looking into the fillOffchainOrders implementation, the user does not initiate the order onchain, he just signs a bunch of data that will be gathered by a keeper and will execute the following function:

function fillOffchainOrders(
uint128 marketId,
OffchainOrder.Data[] calldata offchainOrders,
bytes calldata priceData
)
external
onlyOffchainOrdersKeeper(marketId)
{
...
// buy order -> match against the ask price
// sell order -> match against the bid price
ctx.indexPriceX18 = ctx.isBuyOrder ? ctx.askX18 : ctx.bidX18;
​
// verify the provided price data against the verifier and ensure it's valid, then get the mark price
// based on the returned index price.
ctx.fillPriceX18 = perpMarket.getMarkPrice(sd59x18(ctx.offchainOrder.sizeDelta), ctx.indexPriceX18);
​
// if the order increases the trading account's position (buy order), the fill price must be less than or
// equal to the target price, if it decreases the trading account's position (sell order), the fill price
// must be greater than or equal to the target price.
ctx.isFillPriceValid = (ctx.isBuyOrder && ctx.offchainOrder.targetPrice <= ctx.fillPriceX18.intoUint256())
|| (!ctx.isBuyOrder && ctx.offchainOrder.targetPrice >= ctx.fillPriceX18.intoUint256());
​
// we don't revert here because we want to continue filling other orders.
if (!ctx.isFillPriceValid) {
continue;
}
...
}
​
struct Data {
uint128 tradingAccountId;
uint128 marketId;
int128 sizeDelta;
uint128 targetPrice;
bool shouldIncreaseNonce;
uint120 nonce;
bytes32 salt;
uint8 v;
bytes32 r;
bytes32 s;
}

As we can see, the user signs a field in the data that is the targetPrice. This price is used to set a maximum price that the user is willing to accept when he is buying or the minimum price that the user is going to accept when he is selling. So basically there is a security check for the user to not get an order with a non acceptable price.
However, when a user creates a market order onchain, he will not have this price protection and can get his order executed with a price that he did not expect.

Impact

Medium

Tools Used

Manual review

Recommendations

I would implement the same price protection when creating the market order onchain

Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

fillMarketOrder lacks slippage protection

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.