Liquidity Management

Summary

The _transferToken function in the PerpetualVault contract fails to check the return value of the ERC20 transfer function when sending tokens to the recipient. This could result in silent transfer failures, where tokens are not successfully transferred but the contract proceeds as if they were.

Vulnerability Details

Affected Code

• Function: _transferToken(uint256, uint256)

• Lines:

  try collateralToken.transfer(depositInfo[depositId].recipient, amount - fee) {} // No return check

  catch {

  collateralToken.transfer(treasury, amount - fee); // Also no return check

  emit TokenTranferFailed(...);

  }

Root Cause

1. Ignored Return Value:

   • The transfer function for ERC20 tokens returns a boolean indicating success (true) or failure (false).

   • The _transferToken function does not check this return value, assuming the transfer always succeeds or it just reverts.

2. Ineffective try/catch:

   • Solidity’s try/catch only catches reverts, not false return values. If the token’s transfer function does not revert on failure (e.g., pre-OpenZeppelin 0.8.0), failed transfers will go undetected.

Impact

• Fund Loss: If the transfer to the recipient fails (e.g., due to insufficient balance or a paused token contract), the contract proceeds as if the transfer succeeded.

• Inconsistent State: The totalDepositAmount is decremented, and shares are burned, even if tokens were not actually transferred.

Proof of Concept (PoC)

Scenario

1. Token Configuration:

   • The collateralToken is a legacy ERC20 token that returns false (instead of reverting) on failed transfers.

2. Failed Transfer:

   • A user attempts to withdraw, but the recipient address has disabled receiving tokens (e.g., blacklisted).

   • collateralToken.transfer(...) returns false, but the try/catch does not trigger.

3. State Corruption:

   • The vault burns the user’s shares and reduces totalDepositAmount, but the tokens remain stuck in the contract.

Recommended Mitigation

1. Use safeTransfer for ERC20 Transfers

Replace transfer with OpenZeppelin’s safeTransfer, which reverts on failure:

2. Remove Redundant try/catch

If safeTransfer is used, the try/catch becomes unnecessary (as failures will revert):

3. Handle Non-Reverting Tokens

If non-reverting tokens must be supported, explicitly check the return value: