Zaros Part 1

Summary

The implementation of price validation for order filling is incorrect and leads to outcomes directly opposite of intented behaviour. This can lead to orders being filled at unfavorable prices, which causes financial losses for users.

Vulnerability Details

In SettlementBranch:fillOffchainOrders, a check is performed to ensure that the price filled is valid. The fill price should never be higher than the target price with a buy order and never be lower with a sell order.
However, the code implements exactly the opposite. It allows buy orders to be filled at prices higher than the target price and sell orders at prices lower than the target price.

This will certainly lead to financial losses to users.

Example:

Note that the NatSpec comments correctly describe how the code is supposed to work but the implementation is the complete opposite.

The implemented logic

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

Impact

There will be financial losses to overpayment with buy orders and under receiving payment with sell orders.

The impact is High and the likelyhood is guaranteed, so we believe this findings merits a High severity.

Tools Used

Manual Review

Recommendations

Reverse the comparison operators in the price validation logic to align with the intended behavior.