Zaros Part 1

Summary

In SettlementBranch's function fillOffchainOrders,the condition for validating the fill price of an order is incorrectly implemented, leading to a scenario where orders won't be executed or may be executed at unfavorable prices. The logic for checking the validity of the fill price is reversed, causing buy orders to potentially execute at higher than intended prices and sell orders at lower than intended prices.

Vulnerability Details

The current implementation of the condition is as follows. Due to this, if orders are executed, then they will be at worse price. If there is favorable price for an order, it won't be executed.

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/SettlementBranch.sol#L283-L292

Example Scenario:

1. Buy Order:

   • Target Price: $100

   • Fill Price: $110

   • Expected: The fill price should be less than or equal to the target price.

   • Actual: The condition (ctx.isBuyOrder && ctx.offchainOrder.targetPrice <= ctx.fillPriceX18.intoUint256()) evaluates to true, allowing the buy order to execute at $110, which is higher than the target price.

2. Sell Order:

   • Target Price: $100

   • Fill Price: $90

   • Expected: The fill price should be greater than or equal to the target price.

   • Actual: The condition (!ctx.isBuyOrder && ctx.offchainOrder.targetPrice >= ctx.fillPriceX18.intoUint256()) evaluates to true, allowing the sell order to execute at $90, which is lower than the target price.

Impact

Traders may incur significant losses as buy orders could execute at prices higher than the target price, and sell orders could execute at prices lower than the target price.

Tools Used

Manual review

Recommendations

Correct the condition to ensure that buy orders execute at prices less than or equal to the target price, and sell orders execute at prices greater than or equal to the target price. The corrected condition is: