Steadefi
Steadefi
DeFiHardhatFoundryOracle
35,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Incorrect slippage protection on deposits

hash

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;
​
Updates

Lead Judging Commences

hans Auditor
over 1 year ago
hans Auditor
over 1 year ago
hans Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Wrong calculation of minMarketTokenAmt

Impact: High Likelihood: High Logic error in the core functionality that can cause loss of user funds.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.