Zaros Part 1

Summary

The fillOffChainOrders function in settlementBranch.sol contains incorrect logic for validating fill prices against target prices for offchain orders. This discrepancy can prevent valid orders from being filled, contrary to the trader's expectations. The comments correctly describe the intended behavior, but the implementation does not align with these comments.

Vulnerability Details

The function fillOffChainOrders contains the following logic to validate the fill price of an order:

Issue:

The logic as currently implemented checks:

• Buy Orders: ctx.isBuyOrder && ctx.offchainOrder.targetPrice <= ctx.fillPriceX18.intoUint256()

  • The right implementation expects the fill price to be less than or equal to the target price for a buy order.(according to the comments).

• Sell Orders: !ctx.isBuyOrder && ctx.offchainOrder.targetPrice >= ctx.fillPriceX18.intoUint256()

  • The right implementation expects the fill price to be greater than or equal to the target price for a sell order.(according to the comments)

Problem:

The comments correctly state the intended behavior, but the current implementation does otherwise. For offchain orders:

• Buy Order: The target price should generally be above the current price (indicating the maximum price a trader is willing to pay).

• Sell Order: The target price should generally be below the current price (indicating the minimum price a trader is willing to accept).

Given this, the current logic can prevent valid limit orders from being filled, as the target price is incorrectly treated as a constraint rather than a limit.

Impact

• Unfilled Orders: Correctly placed orders might not be filled, causing traders to miss potential trading opportunities.

Tools Used

• Manual code review

Recommendations

1. Correct the Validation Logic:
   Update the logic to correctly reflect the intention of limit prices for offchain orders. The revised logic should be:

   ctx.isFillPriceValid = (ctx.isBuyOrder && ctx.offchainOrder.targetPrice >= ctx.fillPriceX18.intoUint256())

   || (!ctx.isBuyOrder && ctx.offchainOrder.targetPrice <= ctx.fillPriceX18.intoUint256());