Submitted by rvierdiiev, Cosine, Drynooo, 0xCiphky, SupaRoutis, innertia, 0xhacksmithh, dipp. Selected submission by: Cosine.
Relevant GitHub Links
Summary
When an emergency situation arises and the protocol pauses or resumes the operation of the vault. All funds of the vault are removed from GMX or added back to GMX without any protection against slippage. This allows MEV bots to take advantage of the protocol's emergency situation and make huge profits with it.
Vulnerability Details
When an emergency situation arises the protocol owners can call the emergencyPause function to remove all the liquidity from GMX:
plain textfunction emergencyPause( GMXTypes.Store storage self ) external { self.refundee = payable(msg.sender); GMXTypes.RemoveLiquidityParams memory _rlp; // Remove all of the vault's LP tokens _rlp.lpAmt = self.lpToken.balanceOf(address(this)); _rlp.executionFee = msg.value; GMXManager.removeLiquidity( self, _rlp ); self.status = GMXTypes.Status.Paused; emit EmergencyPause(); }
But the minimum tokens amount to get back when removing liquidity is not provided to the RemoveLiquidityParams:
plain textstruct RemoveLiquidityParams { // Amount of lpToken to remove liquidity uint256 lpAmt; // Array of market token in array to swap tokenA to other token in market address[] tokenASwapPath; // Array of market token in array to swap tokenB to other token in market address[] tokenBSwapPath; // Minimum amount of tokenA to receive in token decimals uint256 minTokenAAmt; // Minimum amount of tokenB to receive in token decimals uint256 minTokenBAmt; // Execution fee sent to GMX for removing liquidity uint256 executionFee; }
As it is not set, the default value 0 (uint256) is used. Therefore, up to 100% slippage is allowed.
The same parameters are also missing when normal operation resumes:
plain textfunction emergencyResume( GMXTypes.Store storage self ) external { GMXChecks.beforeEmergencyResumeChecks(self); self.status = GMXTypes.Status.Resume; self.refundee = payable(msg.sender); GMXTypes.AddLiquidityParams memory _alp; _alp.tokenAAmt = self.tokenA.balanceOf(address(this)); _alp.tokenBAmt = self.tokenB.balanceOf(address(this)); _alp.executionFee = msg.value; GMXManager.addLiquidity( self, _alp ); }
Therefore, MEV bots could take advantage of the protocol's emergency situation and as these trades include all funds of the vault it could lead to a big loss.
Ignoring slippage when pausing could be a design choice of the protocol to avoid the possibility of a revert and pause the system as quickly as possible. However, this argument does not apply during the resume.
Impact
Big loss of funds as all funds of the strategy vault are unprotected against MEV bots.
Tools Used
Manual Review
Recommendations
Implement a custom minMarketTokens parameter, but do not implement the usual slippage calculation, as this could potentially lead to new critical vulnerabilities. If for example the reason for this emergency situation is a no longer supported chainlink feed, which will lead to reverts and therefore also to DoS of the emergency close / withdraw flow.