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

Wrong judgement result about `isFillPriceValid`

AllTooWell

Summary

Wrong judgement result about isFillPriceValid because of wrong implemented code.

Vulnerability Details

The validity of the fill price depends on order type (is buy order or not) and the size of fillPrice and targetPrice.

// 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 can see the implemented code and comment is inconsistency. In my understanding, buy order shows traders expect the price to rise. So the fill price should be less than target price. In this way traders maximize their profits if the price has indeed increased. Sell orders is in opposite case. The fill price should be larger than target price for sell orders.
So the imnplemented code is wrong. The comment is right. The judgement result about isFillPriceValid is wrong.

Impact

The normal order can't be fulfilled because of wrong judgement result about isFillPriceValid.

Tools Used

manual

Recommendations

- ctx.isFillPriceValid = (ctx.isBuyOrder && ctx.offchainOrder.targetPrice <= ctx.fillPriceX18.intoUint256()) || (!ctx.isBuyOrder && ctx.offchainOrder.targetPrice >= ctx.fillPriceX18.intoUint256());
​
+ ctx.isFillPriceValid = (ctx.isBuyOrder && ctx.offchainOrder.targetPrice >= ctx.fillPriceX18.intoUint256()) || (!ctx.isBuyOrder && ctx.offchainOrder.targetPrice <= ctx.fillPriceX18.intoUint256());
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year 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.

Give us feedback!