medium

Missing minimum token amounts in the emergency contract functions allows MEV ...

Contest
Reward

Total

202.59 USDC

24.12 USDC
Selected
33.76 USDC
24.12 USDC
24.12 USDC
24.12 USDC
24.12 USDC
24.12 USDC
24.12 USDC
Selected Submission

Missing minimum token amounts in the emergency contract functions allows MEV bots to take advantage of the protocols emergency situation

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-10-SteadeFi/blob/0f909e2f0917cb9ad02986f631d622376510abec/contracts/strategy/gmx/GMXEmergency.sol#L83-L90

https://github.com/Cyfrin/2023-10-SteadeFi/blob/0f909e2f0917cb9ad02986f631d622376510abec/contracts/strategy/gmx/GMXEmergency.sol#L54-L61

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:

function 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:

struct 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:

function 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.