Zaros Part 1

Summary

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/leaves/TradingAccount.sol#L89-L91

Problem is that it's not called when offchain orders are filled, therefore this security measure it bypassed.

Vulnerability Details

There are 2 ways to create order: on-chain and off-chain. It validates positions limit during creation of on-chain order.

However there is no such process when user creates off-chain order. Because user just signs data and submits it to backend.

There is significant evidence that this check is absent on backend: function validatePositionsLimit() is internal, can't be called from outside.

Moreover it can't be checked guaranteed off-chain, consider a scenario:

1. User submits off-chain order.

2. Backend checks positions limit - check passed successfully.

3. User submits on-chain order.

4. To the moment both orders will be filled.

5. As a result, positions limit is bypassed.

Impact

Account can bypass positions limit by submitting off-chain orders. It breaks max gas usage assumptions per account during liquidation.

Protocol uses Chainlink Keepers to execute liquidations. bytes checkData to pass is static and configured during Keeper registration. These data specifies bounds at which Keeper will check and liquidate accounts:

It was implemented to avoid out-of-gas error during this checks and liquidations. As you can see, bypassing positions limit per account will break those configured Keepers - so at worst can it can halt liquidation flow.

Tools Used

Manual Review

Recommendations.

Validate positions limit when you fill orders, i.e. move it to function SettlementBranch._fillOrder(). So that it's checked when orders are filled instead of when created. Because it can't be correctly checked when off-chain order is created.