Steadefi
Steadefi
DeFiHardhatFoundryOracle
35,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Deposits and Withdrawal from Vault doesn't account deadline for transaction to complete

tripathi

Summary

Deposits and Withdrawal from Vault doesn't account deadline for transaction to complete. Transaction may not be optimal for user after certain deadline but protocol will execute them anyway

Vulnerability Details

Protocol should provide their users with an option to limit the execution of their pending actions such as deposits and withdrawal. The most common solution is to include deadline timestamp and reverts if execution is after that timestamps like Uniswap.

Alice deposits 1000 index token to make delta position. She did some offchain computation and find that is she can get svTokens/GMXLP token from depositing before certain time she can avail some other benefits.
tx submitted in mempool and got executed after time she was expecting making a loss for Alice.

There can be muttiple cases where depositors and withdrawers would need deadline checks to make correct strategy for them which can't be implemented here

File:contracts/strategy/gmx/GMXTypes.sol
​
struct DepositParams {
// Address of token depositing; can be tokenA, tokenB or lpToken
address token;
// Amount of token to deposit in token decimals
uint256 amt;
// Minimum amount of shares to receive in 1e18
uint256 minSharesAmt;
// Slippage tolerance for adding liquidity; e.g. 3 = 0.03%
uint256 slippage;
// Execution fee sent to GMX for adding liquidity
uint256 executionFee;
}
​
struct WithdrawParams {
// Amount of shares to burn in 1e18
uint256 shareAmt;
// Address of token to withdraw to; could be tokenA, tokenB or lpToken
address token;
// Minimum amount of token to receive in token decimals
uint256 minWithdrawTokenAmt;
// Slippage tolerance for removing liquidity; e.g. 3 = 0.03%
uint256 slippage;
// Execution fee sent to GMX for removing liquidity
uint256 executionFee;
}
​

Adding a deadline param can solve this issue. Please refer to uniswap and some other protocol which implement such checks with slippage

Impact

Unexpected trade would get executed maliciously

Tools Used

Manual Review

Recommendations

Add deadline parameter in deposits and withdrawal structs and revert if tx is executed after deadline

Updates

Lead Judging Commences

hans Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

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