sherlock-audit / 2024-02-perpetual-judging

2 stars 2 forks source link

solmaxis69 - Users can't maintain position healthiness by increasing its respective margin during inactive market, which allows for liquidation frontrunning when the market becomes active again #42

Closed sherlock-admin4 closed 7 months ago

sherlock-admin4 commented 8 months ago

solmaxis69

medium

Users can't maintain position healthiness by increasing its respective margin during inactive market, which allows for liquidation frontrunning when the market becomes active again

Summary

During market inactivity, users' accountValue can go below the maintenance margin requirement if price moves in the direction opposite to their positions, thus making these positions a subject to liquidation. However, when market becomes active again, MEV bots could frontrun any attempt to maintain position healthiness with a call to liquidation.

Vulnerability Detail

Market is considered inactive if either the systemSuspended storage variable returns true or mapping marketSuspendedMap for a particular marketId returns true. Both values can be changed by an admin in SystemStatus.suspendSystem() or SystemStatus.suspendMarket() functions.

The only way a user can increase their accountValue is by increasing account's margin by depositing collateral, which is the only way for the user to maintain position healthiness if price goes in the opposite direction. However, in case when position becomes liquidatable during market inactivity, user can't call the deposit() function to update their margin account and increase their accountValue. So anyone who tries to call deposit() when market becomes active again can be unfairly liquidated by MEV bots due to frontrun.

It should be noted that the protocol is going to be deployed on Optimism and we recognise that optimism's mempool is private and visible only to the Sequencer. However, as per optimism's docs "transactions are generally executed in priority fee order (highest fee first)." So it is still possible to frontrun users by paiyng high gas fees.

Impact

Unfair position liquidation and loss of funds for users

Code Snippet

https://github.com/sherlock-audit/2024-02-perpetual/blob/main/perp-contract-v3/src/vault/Vault.sol#L93-L94 https://github.com/sherlock-audit/2024-02-perpetual/blob/main/perp-contract-v3/src/systemStatus/SystemStatus.sol#L39-L57

Tool used

Manual Review

Recommendation

Add a grace period after market becomes active again or allow for increasing position's margin during inactivity.

Duplicate of #59

sherlock-admin2 commented 7 months ago

2 comment(s) were left on this issue during the judging contest.

santipu_ commented:

Invalid following Sherlock rules because admin is TRUSTED.

takarez commented:

admin pausing the system causing loss or liquidation is invalid according to sherlock rules