Incorrect slippage protection on deposits

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-10-SteadeFi/blob/0f909e2f0917cb9ad02986f631d622376510abec/contracts/strategy/gmx/GMXDeposit.sol#L136-L140

Vulnerability Details

The slippage on deposits is enforced by the minMarketTokenAmt parameter. But in the calculation of minMarketTokenAmt, the slippage is factored on the user's deposit value and not the leveraged amount which is actually being deposited to GMX.

  function deposit(
    GMXTypes.Store storage self,
    GMXTypes.DepositParams memory dp,
    bool isNative
  ) external {
    
    ....... more code 

    if (dp.token == address(self.lpToken)) {
      // If LP token deposited
      _dc.depositValue = self.gmxOracle.getLpTokenValue(
        address(self.lpToken),
        address(self.tokenA),
        address(self.tokenA),
        address(self.tokenB),
        false,
        false
      )
      * dp.amt
      / SAFE_MULTIPLIER;
    } else {
      // If tokenA or tokenB deposited
      _dc.depositValue = GMXReader.convertToUsdValue(
        self,
        address(dp.token),
        dp.amt
      );
    }
    
     ....... more code

    _alp.tokenAAmt = self.tokenA.balanceOf(address(this));
    _alp.tokenBAmt = self.tokenB.balanceOf(address(this));
    _alp.minMarketTokenAmt = GMXManager.calcMinMarketSlippageAmt(
      self,
      _dc.depositValue,
      dp.slippage
    );
    _alp.executionFee = dp.executionFee;


    _dc.depositKey = GMXManager.addLiquidity(
      self,
      _alp
    );

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

But vaults with leverage greater than 1 will be adding more than _dc.depositValue worth of liquidity in which case the calculated minMarketTokenAmt will result in a much higher slippage.

Example Scenario

• The vault is a 3x leveraged vault
• User deposits 1 usd worth tokenA and sets slippage to 1%.
• The minMarketTokenAmt calculated is worth 0.99 usd
• The actual deposit added is worth 3 usd due to leverage
• The deposit receives 2.90 worth of LP token which is more than 1% slippage

Impact

Depositors can loose value

Recommendations

Use the actual deposit value instead of the user's initial deposit value when calculating the minMarketTokenAmt

diff --git a/contracts/strategy/gmx/GMXDeposit.sol b/contracts/strategy/gmx/GMXDeposit.sol
index 1b28c3b..aeba68b 100644
--- a/contracts/strategy/gmx/GMXDeposit.sol
+++ b/contracts/strategy/gmx/GMXDeposit.sol
@@ -135,7 +135,15 @@ library GMXDeposit {
     _alp.tokenBAmt = self.tokenB.balanceOf(address(this));
     _alp.minMarketTokenAmt = GMXManager.calcMinMarketSlippageAmt(
       self,
-      _dc.depositValue,
+      GMXReader.convertToUsdValue(
+        self,
+        address(self.tokenA),
+        _alp.tokenAAmt
+      ) + GMXReader.convertToUsdValue(
+        self,
+        address(self.tokenB),
+        _alp.tokenBAmt
+      ),
       dp.slippage
     );
     _alp.executionFee = dp.executionFee;