Zaros Part 1

Summary

Vulnerability Details

The OffchainOrder.Data.targetPrice work as price slippage to avoid trade with unwanted price. For buy orders, generally, the targetPrice acts as the maximum price at which buyers are agreed to fullfill their trade. And for sell orders, the targetPrice is the minimum price at which seller are ready to accept their trade.

If the fillPrice goes against the supplied targetPrice, then the trade should not be continue(following check ensure that).
https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/SettlementBranch.sol#L290

However, the issue is the ctx.isFillPriceValid perform the incorrect validation for buy/sell orders. It should be,

• if isBuyOrder == true, thenfillPrice <= targetPrice

• if isBuyOrder == false, then targetPrice <= fillPrice

but the check is quite opposite in the current code, https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/SettlementBranch.sol#L286-L287

As a result, the orders will get executed at unwanted fillPrice, bringing potential loss to the user position.

Impact

1. Orders will get executed at unwantedly higher price than the max targetPrice(for ex. in buyOrders), loss of funds to user

2. If fillPrice is under/above the targetPrice for buy/sell orders, than the legitimate trade will get skipped.

Tools Used

Recommendations