Zaros Part 1

Summary

Off-chain orders are signed messages, sent on the frontend and filled by keepers at a later time.

These orders include a targetPrice which represents the price at which the user ideally wants his order to be filled.

Here is how an off-chain order is constructed

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/leaves/OffchainOrder.sol

Keepers fulfill these order using the SettlementBranch::fillOffchainOrders() function and has to pass the price requirements :

Where :

• ctx.offchainOrder.targetPrice is the ideal price set by the user

• ctx.fillPriceX18 is the current price of the asset

• ctx.isBuyOrder is a boolean that determines whether the order is a buy or sell order depending on the size delta of the trade

Only if the fill price is valid, the trade is filled.

Vulnerability Details

The logic in the above is incorrect, ideally, a user would want to buy his assets at a price lower or equal to the price he initially wanted and would want to sell his assets at a price higher or equal to the price he initially wanted.

The implemented logic can be summerized like such :

• for buyOrder : the current price is valid if the current price of the asset (ctx.fillPriceX18) is equal or GREATER than the intended price (ctx.offchainOrder.targetPrice)

• for sellOrder : the current price is valid if the current price of the asset (ctx.fillPriceX18) is equal or LESS than the intended price (ctx.offchainOrder.targetPrice)

The comments above the code section also counter indicates the check :

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.

Impact

The price check is flawed making off-chain orders will never be executed at a better price for users.

Tools Used

Manual review

Recommendations

Invert the comparison signs in the problematic code section