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

Denial-of-Service via Incomplete Finalization in Deposit Flow

mohitisimmortal

Summary

When a position is open and a user deposits tokens, if the keeper’s off-chain metadata consists of a single element (i.e. metadata.length == 1) with protocol set to DEX, the deposit flow proceeds through the _runSwap() function. In this branch, the contract calls _mint() without subsequently finalizing the flow (i.e. without calling _finalize()). As a result, the flow remains active and is never reset, so any future call to deposit, which begins by checking _noneFlow() will revert. This effectively creates a denial-of-service (DoS) condition for deposit, withdrawal, and run() calls.

Vulnerability Details

During positionIsClosed == false, user deposits some tokens, now keeper runs- runNextAction(), then flow goes into- _runSwap(metadata, true, prices), from there if metadata of length==1, then it goes into below else block-

994 else {
995 if (metadata.length != 1) {
996 revert Error.InvalidData();
997 }
998 (PROTOCOL _protocol, bytes memory data) = abi.decode(metadata[0], (PROTOCOL, bytes));
999 if (_protocol == PROTOCOL.DEX) {
1000 uint256 outputAmount = _doDexSwap(data, isCollateralToIndex);
1001
1002 // update global state
1003 if (flow == FLOW.DEPOSIT) {
1004 // last `depositId` equals with `counter` because another deposit is not allowed before previous deposit is completely processed
1005 @> _mint(counter, outputAmount + swapProgressData.swapped, true, prices);
1006 }

  • here it mints tokens, but never calls _finalize()- which is responsible to set flow again to None. And it creates a denial-of-service state.

Impact

High

  • DOS

Likelihood

High

  • since keeper do many transcations and can make any length of metadata from 0 - 2, so its much likely that signals can make metadata.length==1 with protocol==DEX.

Tools Used

Manual Review

Recommendations

  • add a _finalize() call immediately after _mint(...) in the branch handling metadata.length == 1.

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!