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

Unexpected revert caused by hardcoded slippage during high volatility times

angrymustacheman

Summary

During swapping process, the slippage parameter is hardcoded to fixed value. So at times of high volatility, the swapping might fail causing problems at different functionalities of the protocol.

Vulnerability Details

Let's take example of GMXDeposit.sol. After depositing the funds, if the one of the afterDepositChecks fails then the processDeposit would stop at the moment and would go to catch condition shown below where self.status
is assigned value GMXTypes.Status.Deposit_Failed:
https://github.com/Cyfrin/2023-10-SteadeFi/blob/main/contracts/strategy/gmx/GMXDeposit.sol#L170C3-L186C6

try GMXProcessDeposit.processDeposit(self) {
// Mint shares to depositor
self.vault.mint(self.depositCache.user, self.depositCache.sharesToUser);
​
self.status = GMXTypes.Status.Open;
​
emit DepositCompleted(
self.depositCache.user,
self.depositCache.sharesToUser,
self.depositCache.healthParams.equityBefore,
self.depositCache.healthParams.equityAfter
);
} catch (bytes memory reason) {
self.status = GMXTypes.Status.Deposit_Failed;
​
emit DepositFailed(reason);
}

As deposit function cannot be further continued due to failure in after deposit checks, so to withdraw liquidity from GMX ,first processDepositFailure is called then further processDepositFailureLiquidityWithdrawal is called to repay the borrowed loan amount. In processDepositFailureLiquidityWithdrawal, swapping between tokenA and tokenB occurs to Adjust amount to repay for both tokens due to slight differences from liqudiity withdrawal and swaps. The problem is while swapping the slippage parameter is set to a fixed number - self.minSlippage at :
https://github.com/Cyfrin/2023-10-SteadeFi/blob/main/contracts/strategy/gmx/GMXDeposit.sol#L307

...
_sp.tokenIn = _tokenFrom;
_sp.tokenOut = _tokenTo;
_sp.amountIn = IERC20(_tokenFrom).balanceOf(address(this));
_sp.amountOut = _tokenToAmt;
@> _sp.slippage = self.minSlippage;
_sp.deadline = block.timestamp;
...

which is assigned a fixed percentage of 1% at:
https://github.com/Cyfrin/2023-10-SteadeFi/blob/main/scripts/strategy/gmx/deploy-strategy-gmx.ts#L65

const minSlippage = ethers.parseUnits('0.01', 4) // 1%

which is further assigned at the time of construction at GMXVault,sol:
https://github.com/Cyfrin/2023-10-SteadeFi/blob/main/contracts/strategy/gmx/GMXVault.sol#L82

_store.minSlippage = store_.minSlippage;

The swappers used for swapping of tokens are Uniswap and TraderJoe. The auto slippage percentage of Uniswap ranges from 0.5% - 5% . So , due to setting hardcoded slippage causes two problems:

  1. During high volatility times, swap might fail automatically, as the slippage% would be >1%

  2. During low volatility times when slippage is <1% , it would lead to unnecessary loss of funds.

Impact

Unexpected revert caused by hardcoded slippage during high volatility times

Tools Used

Manual review,
Reference Article: https://dacian.me/defi-slippage-attacks#heading-hard-coded-slippage-may-freeze-user-funds

Recommendations

Take in slippage parameter as the processDepositFailure function and add necessary checks for the same.

Updates

Lead Judging Commences

hans Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Other
angrymustacheman Submitter
over 1 year ago
hans Auditor
over 1 year ago
angrymustacheman Submitter
over 1 year ago
hans Auditor
over 1 year ago
hans Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Missing slippage protection on swaps

Support

FAQs

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