H-07. Incorrect slippage protection on deposits

Submitted by ElHaj, hash, 0xhacksmithh. Selected submission by: hash.

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.

plain text
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
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;