Summary

GlobalConfigurationBranch contract allows for the removal of collateral types from the liquidation priority list without safeguards for existing deposits. This can lead to a situation where user-deposited collateral becomes inaccessible during liquidations, potentially resulting in significant financial losses for users.

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/GlobalConfigurationBranch.sol#L271C3-L280C6

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

Proof of concept

1: User A deposits 100 USDC as collateral.

2: Protocol owner removes USDC from the collateral liquidation priority list.

3: User A's position becomes eligible for liquidation.

4: During liquidation, the 100 USDC is not recognized as valid collateral and is effectively lost.

5: User A suffers a complete loss of their 100 USDC deposit.

Impact

1: User funds can become inaccessible if their deposited collateral type is removed from the priority list.

2: Liquidations may fail to account for all user collateral, leading to incomplete or unfair liquidations.

3: System's collateral accounting may become inconsistent with actual user deposits.

4: Potential for significant financial losses for affected users.

Tools Used

Manaul Review

Recommendations

1: Implement a check in the removeCollateralFromLiquidationPriority function to prevent removal if there are active deposits

2: Implement a graceful deprecation process for collateral types.

3: Update the liquidation process to handle deprecated collateral types.

4: Add an emergency withdrawal mechanism for users to recover funds from deprecated collateral types.