DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: medium
Invalid

Unchecked Token Transfer Return Values Leading to Silent Failures

Summary

The _transferToken function does not properly check the return values of token transfers, which could lead to silent failures and loss of funds.

Vulnerability Details

The function attempts to transfer tokens using ERC20's transfer method but doesn't properly utilize the SafeERC20 library despite it being imported. The empty try-catch block silently ignores successful transfers, and the fallback transfer to treasury may also fail silently.

Code sippet:

  • This line uses raw transfer instead of safeTransfer:

try collateralToken.transfer(depositInfo[depositId].recipient, amount - fee) {}
  • The empty try block ignores the transfer result:

try collateralToken.transfer(...) {} // No success verification
  • The catch block attempts another unchecked transfer:

catch {
collateralToken.transfer(treasury, amount - fee);
emit TokenTranferFailed(depositInfo[depositId].recipient, amount - fee);
}

Impact

  • Token transfers could fail silently without proper reversion

  • Funds could be lost or stuck in the contract

  • State changes might occur even when transfers fail

  • Users might believe transfers succeeded when they actually failed

PoC:

contract TokenTransferTest {
function testFailedTransfer() external {
// Assume contract setup with a non-compliant ERC20 token
PerpetualVault vault = PerpetualVault(vault_address);
// 1. Deploy mock token that returns false on transfer
MockERC20 mockToken = new MockERC20();
// 2. Setup deposit with mock token
uint256 depositId = 1;
uint256 amount = 100;
// 3. Transfer will fail silently
vault._transferToken(depositId, amount);
// 4. State changes occur despite failed transfer
assert(vault.totalDepositAmount() != originalAmount); // Will pass even though transfer failed
}
}
contract MockERC20 {
function transfer(address, uint256) external returns (bool) {
return false; // Simulate failed transfer
}
}

Tools Used

Manual Review

Recommendations

  • Use SafeERC20's safeTransfer consistently:

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) {
SafeERC20.safeTransfer(collateralToken, treasury, fee);
}
}
// Main transfer with proper reversion
SafeERC20.safeTransfer(collateralToken, depositInfo[depositId].recipient, amount - fee);
totalDepositAmount -= depositInfo[depositId].amount;
emit GovernanceFeeCollected(address(collateralToken), fee);
}
  • Remove the try-catch block and allow failed transfers to revert

  • Add explicit success checks for all token operations

  • Implement proper error handling with specific error messages

  • Consider adding events for successful transfers

Updates

Lead Judging Commences

n0kto Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Out of scope
n0kto Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Out of scope

Support

FAQs

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