DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: low
Valid

Cancel flow operation can revert

Target

contracts/PerpetualVault.sol

Vulnerability Details

The cancelFlow function is used to terminate an ongoing flow due to issues on the GMX side of the protocol. A key invariant maintained throughout the protocol is that keeper operations should never revert.

To enforce this, all functions called by the keeper that interact with tokens, user addresses, or user-provided addresses are wrapped in try/catch statements to gracefully handle unexpected failures. However, within the _cancelFlow function, a token transfer is performed to an address without being wrapped in a try/catch statement. If this transfer operation fails, the entire keeper operation would revert, violating a fundamental property of the keeper flow.

function cancelFlow() external nonReentrant gmxLock {
_onlyKeeper();
_cancelFlow();
}
function _cancelFlow() internal {
if (flow == FLOW.DEPOSIT) {
uint256 depositId = counter;
collateralToken.safeTransfer(depositInfo[depositId].owner, depositInfo[depositId].amount);
totalDepositAmount = totalDepositAmount - depositInfo[depositId].amount;
EnumerableSet.remove(userDeposits[depositInfo[depositId].owner], depositId);
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner,
depositInfo[counter].executionFee
) {} catch {}
delete depositInfo[depositId];
} else if (flow == FLOW.WITHDRAW) {
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner,
depositInfo[counter].executionFee
) {} catch {}
}

Possible Causes of Transfer Failure:

  • The recipient address is blacklisted (e.g., USDC, USDT).

  • The contract holds fewer tokens than the transfer amount.

Impact

Keeper-called functions do not emit events, making it difficult to detect unexpected reverts. This lack of visibility can lead to unintended execution failures and disrupt the overall operation flow.

Tools Used

Manual Review

Recommendations

Wrap the collateral token transfer operation in a try/catch statement to gracefully handle unexpected failures without causing a revert.

Updates

Lead Judging Commences

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

finding_cancelFlow_blacklisted

Likelihood: Extremely Low, when user is blacklisted between the deposit/withdraw and cancelFlow is called by the Keeper. Impact: Medium/High, cancelFlow DoS.

Support

FAQs

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