Lucky Peach Barbel
Medium
The validate function in InvariantLib
contains a critical flaw in its stale price check logic. The condition !(newOrder.isEmpty() && newOrder.collateral.gte(Fixed6Lib.ZERO))
is incorrectly placed, allowing the stale price check to be bypassed when the order is empty and collateral is non-negative, even if the sender has an open position. Specifically, the code snippet:
if (
!(context.latestPositionLocal.magnitude().isZero() && context.pendingLocal.isEmpty()) && // sender has no position
!(newOrder.isEmpty() && newOrder.collateral.gte(Fixed6Lib.ZERO)) && // sender is depositing zero or more into account, without position change
(
!context.latestOracleVersion.valid ||
context.currentTimestamp - context.latestOracleVersion.timestamp >= context.riskParameter.staleAfter
) // price is not stale
) revert IMarket.MarketStalePriceError();
fails to enforce the stale price check when newOrder.isEmpty()
and newOrder.collateral.gte(Fixed6Lib.ZERO)
are true, regardless of whether the sender has an open position. This means that even if the oracle price is stale, the function will not revert if the order is empty and collateral is non-negative, allowing unsafe operations (e.g., liquidations or position updates) to proceed using outdated prices.
The primary impact of this bug is that stale oracle prices can be used for critical operations, such as liquidations or margin calculations, leading to incorrect state transitions and potential financial losses for users. For example, a liquidation could be executed based on an outdated price, unfairly penalizing a user whose position would otherwise be solvent with the latest price.
Restructure the conditional logic to ensure the stale price check is enforced whenever the sender has an open position, regardless of whether the order is empty or collateral is non-negative.