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

Improper targetPrice check in fillOffchainOrders

jennifersun

Summary

Zeros make use of fillOffchainOrders to support limit order, take profit, stop loss order. But the targetPrice's check is improper for some scenarios.

Vulnerability Details

From the previous cyfrin audit report, the sponsors mentions that we have implemented a new "Off-Chain" order feature which allows users to specify a targetPrice. This feature is flexible enough to implement limit, stop, tp/sl and other types of trigger-based orders using some additional off-chain code.

The vulnerability is that the targetPrice's meaning is a little difference for limit order, tp or sl. We should check the targetPrice correctly.
For example:

  • The market's current market price is 120.

  • Alice submits one limit order to buy some token with target price 100.

  • Keepers can trigger fillOffchainOrders successfully with current market price 120. Because we match the check logic ctx.isBuyOrder && ctx.offchainOrder.targetPrice <= ctx.fillPriceX18.intoUint256().

Although the keeper is trusted, the keeper will trigger the fillOffchainOrders when the keeper find that we match the traders' requirement. However, considering the possible reorg, or the suddenly price jump, it's also possible that traders will take some unexpected loss because the non-functional target price protection.

function fillOffchainOrders(
uint128 marketId,
OffchainOrder.Data[] calldata offchainOrders,
bytes calldata priceData
)
external
onlyOffchainOrdersKeeper(marketId)
{
...
// cache the order side
ctx.isBuyOrder = ctx.offchainOrder.sizeDelta > 0;
​
// 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.
// support limit order/ take profit/ stop loss
// @audit opposite ???
ctx.isFillPriceValid = (ctx.isBuyOrder && ctx.offchainOrder.targetPrice <= ctx.fillPriceX18.intoUint256())
|| (!ctx.isBuyOrder && ctx.offchainOrder.targetPrice >= ctx.fillPriceX18.intoUint256());
if (!ctx.isFillPriceValid) {
continue;
}
​

Impact

Traders may take some unexpected fund loss because of the non-functional target-price.

Tools Used

Manual

Recommendations

Add one new parameter to mark the order type, limit order, stop loss, take profit and check the target price according to the different order type.

Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Validated
Assigned finding tags:

fillOffchainOrders inverses the comparison between `fillPrice` and `targetPrice`

Support

FAQs

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