RebateFi Hook

First Flight #53
Beginner FriendlyDeFi
100 EXP
View results
Submission Details
Severity: medium
Valid

withdrawTokens Uses Unsafe ERC20 Transfer

Root + Impact

Description

  • Normal Behavior:
    The withdrawTokens function allows the contract owner to transfer ERC20 tokens from the contract to a specified address. It should ensure:

    1. The transfer succeeds reliably for all ERC20 tokens, including those that do not return a boolean (USDT, Tether style).

    2. The destination address is valid.

    3. The contract has sufficient balance.

    Observed Issue:
    The current implementation uses a direct call to IERC20.transfer, which is unsafe:

  • Problems:

    1. Some tokens (e.g., USDT) do not return bool on transfer, leading to silent failures.

    2. No check for zero address → can burn tokens.

    3. No balance check → can revert unexpectedly or allow misconfiguration.

    4. Event parameters order is inconsistent with standard usage.

function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
IERC20(token).transfer(to, amount); // ❌ Unsafe
emit TokensWithdrawn(to, token , amount);
}

Risk

Likelihood:

  • Owner-triggered function, so attack surface is limited.

Still, misconfigured or unsupported tokens can lead to silent failures.

Impact:

  • Tokens may remain trapped in the contract due to silent failures.

Transferring to address(0) can burn tokens unintentionally.

  • Users or protocol could lose tokens during emergency withdrawals.

Proof of Concept

// Using USDT (non-standard ERC20)
IERC20 usdt = IERC20(USDT_ADDRESS);
withdrawTokens(USDT_ADDRESS, user, 100 * 10**6);
// Buggy code: USDT transfer does not return bool, so it may silently fail
// Fixed code using SafeERC20 ensures proper transfer handling and revert on failure
// Attempt to withdraw to zero address
withdrawTokens(USDT_ADDRESS, address(0), 100);
// Buggy code: tokens burned silently
// Fixed code: reverts with "Cannot withdraw to zero address"
// Attempt to withdraw more than balance
withdrawTokens(USDT_ADDRESS, user, 1_000_000_000);
// Buggy code: may revert or fail silently depending on token
// Fixed code: reverts with "Insufficient balance"

Recommended Mitigation

- IERC20(token).transfer(to, amount);
+ require(to != address(0), "Cannot withdraw to zero address");
+ require(amount > 0, "Amount must be greater than zero");
+ require(IERC20(token).balanceOf(address(this)) >= amount, "Insufficient balance");
+ SafeERC20.safeTransfer(IERC20(token), to, amount);
Updates

Lead Judging Commences

chaossr Lead Judge 12 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Not using safe transfer for ERC20.

Support

FAQs

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

Give us feedback!