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

Protocol Recovery Mechanism at Risk Due to Unhandled Token Transfer Failures

rampage

Summary

The PerpetualVault.sol contract's flow cancellation mechanism can fail when token transfers are rejected, particularly during the _cancelFlow() operation. This creates a situation where the protocol's recovery mechanism becomes ineffective if the collateral token transfer is blocked (e.g., due to USDC blacklisting).

Vulnerability Details

The vulnerability stems from the inconsistent handling of token transfers between two functions:

  1. The _transferToken() function properly implements a try-catch mechanism:

​
/**
* @dev Collect fee from the withdraw amount and transfer tokens to the user.
* Collect fee only when the user got the profit.
*/
function _transferToken(uint256 depositId, uint256 amount) internal {
uint256 fee;
if (amount > depositInfo[depositId].amount) {
fee = (amount - depositInfo[depositId].amount) * governanceFee / BASIS_POINTS_DIVISOR;
if (fee > 0) {
collateralToken.safeTransfer(treasury, fee);
}
}
-> try collateralToken.transfer(depositInfo[depositId].recipient, amount - fee) {}
catch {
collateralToken.transfer(treasury, amount - fee);
emit TokenTranferFailed(depositInfo[depositId].recipient, amount - fee);
}
totalDepositAmount -= depositInfo[depositId].amount;
emit GovernanceFeeCollected(address(collateralToken), fee);
}
​

  1. However, the _cancelFlow() function lacks this safety mechanism:

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 {}
}
// Setting flow to liquidation has no meaning.
// The aim is to run FINAIZE action. (swap indexToken to collateralToken);
flow = FLOW.LIQUIDATION;
nextAction.selector = NextActionSelector.FINALIZE;
}

Impact

  • Protocol recovery operations may fail completely when token transfers are rejected

  • User funds could become locked in the protocol

  • While the owner can modify vault state through setVaultState(), this doesn't resolve the underlying issue of locked deposits

Tools Used

Manual Code Review

Recommendations

Implement consistent error handling in the _cancelFlow() function by adding a try-catch block for token transfers:

​
function _cancelFlow() internal {
if (flow == FLOW.DEPOSIT) {
uint256 depositId = counter;
- collateralToken.safeTransfer(depositInfo[depositId].owner, depositInfo[depositId].amount);
+ try collateralToken.safeTransfer(depositInfo[depositId].owner, depositInfo[depositId].amount) {}
+ catch {
+ collateralToken.safeTransfer(treasury, depositInfo[depositId].amount);
+ emit TokenTransferFailed(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 {}
}
// Setting flow to liquidation has no meaning.
// The aim is to run FINAIZE action. (swap indexToken to collateralToken);
flow = FLOW.LIQUIDATION;
nextAction.selector = NextActionSelector.FINALIZE;
}
Updates

Lead Judging Commences

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

Appeal created

riceee Auditor
9 months ago
n0kto Lead Judge
8 months ago
n0kto Lead Judge 8 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.

Give us feedback!