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

Flow state isn't reset during Paraswap swap leading to loss of funds for withdrawers

wellbyt3

Summary

When a user deposits to a 1x leverage long PerpetualVault and the swap is done using only Paraswap, flow isn't reset to NONE which bricks withdraws, leading to loss of funds for withdrawers.

Vulnerability Details

When a user deposits to a PerpetualVault with an open long position with 1x leverage on GMX, the flow is set to DEPOSIT and the nextAction is set to INCREASE_ACTION:

function deposit(uint256 amount) external nonReentrant payable {
​
...SNIP...
​
@> flow = FLOW.DEPOSIT;
​
...SNIP...
​
if (positionIsClosed) {
​
MarketPrices memory prices;
_mint(counter, amount, false, prices);
_finalize(hex'');
​
} else {
​
_payExecutionFee(counter, true);
@> nextAction.selector = NextActionSelector.INCREASE_ACTION;
nextAction.data = abi.encode(beenLong);
}
}

Next, the keeper calls runNextAction() which clear the nextAction state and calls _runSwap():

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);
...SNIP...
}

If the swap is done using only Paraswap (i.e. PROTOCOL = DEX), then we run the swap and mint the user their shares, but forget to call _finalize() to reset the flow back to NONE.

function _runSwap(
bytes[] memory metadata,
bool isCollateralToIndex, //t
MarketPrices memory prices) internal returns (bool completed) {
...SNIP...
if (_protocol == PROTOCOL.DEX) {
uint256 outputAmount = _doDexSwap(data, isCollateralToIndex);
if (flow == FLOW.DEPOSIT) {
_mint(counter, outputAmount + swapProgressData.swapped, true, prices);
// @audit forget to call _finalize() to reset flow state
}
...

This bricks any function that calls _noneFlow() including:

  • deposit()

  • withdraw()

  • run()

  • claimCollateralRebates()

When the owner notices this, they can call PerpetualVault.setVaultState() to set the flow back to NONE.

The big problem here is the amount of time it takes the owner to notice the issue. If a user is trying to withdraw because they believe the price of the index token is going to drop, they can't withdraw.

If the price does indeed drops, this user will lose funds in the amount of the difference between when they tried to withdraw and when the state of flow was actually reset so they could withdraw.

Impact

Loss of funds for withdrawer.

Tools Used

Manual review

Recommendations

Call _finalize to reset flow state after Paraswap swap:

function _runSwap(bytes[] memory metadata, bool isCollateralToIndex, MarketPrices memory prices) internal returns (bool completed) {
...SNIP...
​
if (flow == FLOW.DEPOSIT) {
​
_mint(counter, outputAmount + swapProgressData.swapped, true, prices);
+. _finalize(hex'');
​
...SNIP...
}
Updates

Lead Judging Commences

n0kto Lead Judge 9 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.

Give us feedback!