Zaros Part 1

Lines of code

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/TradingAccountBranch.sol#L386-L392

Impact

In case a collateral/margin is removed from the collateralLiquidationPriority and its loanToValue is set to 0, a user can not withdraw his deposited collateral if his account does not meet the initialMargin requirements. This results in the collateral being stuck in the protocol until the user meets the initialMargin requirements even though the collateral has no value in the protocol.

Proof of Concept

Users can withdraw collateral they deposited to the protocol by calling withdrawMargin. Before the withdrawal is executed it is ensure that the account still meets the initialMarginRequirements for the open positions after the withdrawal of the collateral. For this, the required initialMargin for the open positions is determined and checked against the marginBalance of the account after the collateral is withdrawn:

The issue arises from the fact that this check is also done even if the collateral which should be withdrawn is no longer contributing to the marginBalance of the account. This can happen when the collateral was active before when the user deposited it but got removed from the collateralLiquidationPriority and therefore its loanToValue value was set to 0 after that. This would lead to the collateral being stuck in the protocol if the account does not meet the initialMargin requirements even though the collateral has no value in the protocol.

Recommended Mitigation Steps

Add an if statement at the beginning of the margin checks that determines if the loanToValue value for the collateral that should be withdrawn is 0. If this is the case, skip the margin checks and execute the withdrawal.