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

`SettlementBranch::fillOffchainOrders` is implemented wrongly

cryptedoji

Summary

In SettlementBranch::fillOffchainOrders function, the ctx.isFillPriceValid conditional logic is implemented in contrary to what the workings of the protocol says.

Vulnerability Details

based on the workings of the protocol as directed in comments, to check if a fillPrice is valid, when taking an offChain Buy order, the targetPrice must be greater than or equal to the fillPrice AND the reverse is the case when taking a Sell order; However, the ctx.isFillPriceValid conditional logic is doing the direct opposite

// 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());
​

Impact

  • No offchain order will be taken and if orders are taken based on the logic implemented, it will lead to lose of funds for traders

Tools Used

  • Manual review

Recommendations

// 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());
​
+ 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 10 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.