Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

The Vault will be stuck in Deposit Flow if a 1x long position is swapped using Paraswap.

wickie

Summary

When users deposit collateral during a 1x long position via PerpetualVault.sol::deposit(), the keepers handle the processing of the deposit and minting of shares in the next transaction via runNextAction(). It can choose to swap via GMX or Paraswap or both. The swapping of collateral into index tokens can be Paraswap + GMX, GMX or Paraswap. When swapping via Paraswap + GMX or just GMX, the minting of shares and resetting of the flow is handled properly in afterOrderExecution(). However, if the keeper is just using Paraswap, the deposit is processed and shares are minted but the flow is not finalized, leading the vault in the FLOW state, breaking the functionality of the contract, until the owner manually reset the flow.

Vulnerability Details

When the users deposit collateral token into the vault via deposit() while a 1x long position is open,

function deposit(uint256 amount) external nonReentrant payable {
_noneFlow();
if (depositPaused == true) {
revert Error.Paused();
}
if (amount < minDepositAmount) {
revert Error.InsufficientAmount();
}
if (totalDepositAmount + amount > maxDepositAmount) {
revert Error.ExceedMaxDepositCap();
}
flow = FLOW.DEPOSIT;
collateralToken.safeTransferFrom(msg.sender, address(this), amount);
counter++;
depositInfo[counter] = DepositInfo(amount, 0, msg.sender, 0, block.timestamp, address(0));
totalDepositAmount += amount;
EnumerableSet.add(userDeposits[msg.sender], counter);
​
if (positionIsClosed) {
MarketPrices memory prices;
_mint(counter, amount, false, prices);
_finalize(hex'');
} else {
_payExecutionFee(counter, true);
// mint share token in the NextAction to involve off-chain price data and improve security
nextAction.selector = NextActionSelector.INCREASE_ACTION;
nextAction.data = abi.encode(beenLong);
}
}

The users deposit is processed by the keeper in runNextAction(). Since the position is 1x long, it will be processed via _runSwap(), with FLOW.DEPOSIT.

function runNextAction(MarketPrices memory prices, bytes[] memory metadata) external nonReentrant gmxLock {
_onlyKeeper();
Action memory _nextAction = nextAction;
delete nextAction;
if (_nextAction.selector == NextActionSelector.INCREASE_ACTION) {
(bool _isLong) = abi.decode(_nextAction.data, (bool));
​
if (_isLongOneLeverage(_isLong)) {
_runSwap(metadata, true, prices);
} else {
....

In _runSwap(), when the function swap via Paraswap, it will execute the swap via _doDexSwap() and mint the shares to the user.

function _doDexSwap(bytes memory data, bool isCollateralToIndex) internal returns (uint256 outputAmount) {
(address to, uint256 amount, bytes memory callData) = abi.decode(data, (address, uint256, bytes));
IERC20 inputToken;
IERC20 outputToken;
if (isCollateralToIndex) {
inputToken = collateralToken;
outputToken = IERC20(indexToken);
} else {
inputToken = IERC20(indexToken);
outputToken = collateralToken;
}
uint256 balBefore = outputToken.balanceOf(address(this));
ParaSwapUtils.swap(to, callData);
outputAmount = IERC20(outputToken).balanceOf(address(this)) - balBefore;
emit DexSwap(address(inputToken), amount, address(outputToken), outputAmount, isCollateralToIndex);
}
​
function _runSwap(bytes[] memory metadata, bool isCollateralToIndex, MarketPrices memory prices) internal returns (bool completed) {
....
} else {
if (metadata.length != 1) {
revert Error.InvalidData();
}
(PROTOCOL _protocol, bytes memory data) = abi.decode(metadata[0], (PROTOCOL, bytes));
if (_protocol == PROTOCOL.DEX) {
uint256 outputAmount = _doDexSwap(data, isCollateralToIndex);
// update global state
if (flow == FLOW.DEPOSIT) {
// last `depositId` equals with `counter` because another deposit is not allowed before previous deposit is completely processed
@> _mint(counter, outputAmount + swapProgressData.swapped, true, prices);
} else if (flow == FLOW.WITHDRAW) {
_handleReturn(outputAmount + swapProgressData.swapped, false, true);
} else {
// in the flow of SIGNAL_CHANGE, if `isCollateralToIndex` is true, it is opening position, or closing position
_updateState(!isCollateralToIndex, isCollateralToIndex);
}
return true;
} else {
_doGmxSwap(data, isCollateralToIndex);
return false;
}
}
}

After the users are minted shares, _finalize() isnt called nor a next action is set to be called by the keeper. This leave the vault in a FLOW.DEPOSIT state as the resetting of FLOW state is done after mint() by calling _finalize() with empty data, making function with _nonFlow() check reverts.(deposit(), withdraw(), run()), until the owner manually resets the vault state.

function _finalize(bytes memory data) internal {
if (flow == FLOW.WITHDRAW) {
(uint256 prevCollateralBalance, bool positionClosed, bool refundFee) = abi.decode(data, (uint256, bool, bool));
uint256 withdrawn = collateralToken.balanceOf(address(this)) - prevCollateralBalance;
_handleReturn(withdrawn, positionClosed, refundFee);
} else {
delete swapProgressData;
delete flowData;
delete flow;
}
}

Impact

Vault will be unusable after users deposit into a 1x long position and the swap from collateral token to index token is done via Paraswap, until the owner manually resets it.

Tools Used

Manual Review

Recommendations

Call _finalize() with empty data after calling mint() in _runSwap() during a DEPOSIT flow.

Updates

Lead Judging Commences

n0kto Lead Judge 8 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_deposit_1x_long_dex_positionIsOpened_DoS_Flow

Likelihood: Medium/High, - Leverage = 1x - beenLong = True - positionIsClosed = False - Metadata → 1 length and Dex Swap Impact: Medium/High, DoS on any new action before the admin uses setVaultState Since this seems to be the most probable path for a 1x PerpVault, this one deserves a High.

Support

FAQs

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