apegurus/unsafe-erc20-transfers

<!-- Source: BailSec audit reports (CC0) --> <!-- Extracted via audit-ingest pipeline from 4 independent protocol audits -->

Unsafe ERC20 Transfer and Approve Calls

Overview

The standard ERC20 interface specifies that transfer(), transferFrom(), and approve() return a bool indicating success. However, many widely-used tokens deviate from this standard:

• USDT does not return a boolean on transfer/approve
• BNB, OMG have missing return values
• Some tokens return false on failure instead of reverting

Contracts that call these functions directly (without SafeERC20) either:

1. Ignore the return value → silent failure, tokens not actually transferred
2. Expect a boolean return → revert on tokens that don't return one (like USDT)

Severity: Low to Medium

Prevalence: Found in 4 independent BailSec audits: Hypertrade V3 Core, Meuna, Robinos, SwapX Exchange.

---

Vulnerable Pattern

// VULNERABLE: Direct transfer — no return value check
function withdraw(address token, uint256 amount) external {
    IERC20(token).transfer(msg.sender, amount);
    // If token returns false instead of reverting, this silently fails
    // If token doesn't return bool (USDT), this reverts unexpectedly
    balances[msg.sender] -= amount;  // State updated even if transfer failed!
}

// VULNERABLE: Direct approve — breaks with USDT
function approveSpender(address token, address spender, uint256 amount) external {
    IERC20(token).approve(spender, amount);
    // USDT requires setting allowance to 0 before changing to non-zero
    // Direct approve also doesn't handle missing return values
}

Secure Pattern

import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

using SafeERC20 for IERC20;

// SECURE: SafeERC20 handles all non-standard token behaviors
function withdraw(address token, uint256 amount) external {
    IERC20(token).safeTransfer(msg.sender, amount);
    // Reverts on failure for ALL token types
    balances[msg.sender] -= amount;
}

// SECURE: forceApprove handles USDT's approve quirk
function approveSpender(address token, address spender, uint256 amount) external {
    IERC20(token).forceApprove(spender, amount);
    // Sets to 0 first if needed (USDT), handles missing return values
}

Impact

• Silent failure: Token transfer returns false but contract proceeds as if successful — leads to accounting mismatch
• Unexpected revert: Contract fails on widely-used tokens (USDT, BNB) that don't conform to standard return types
• Stuck funds: Approve fails on USDT when changing non-zero allowance without zeroing first
• Loss of funds: State changes applied after a silently failed transfer result in fund loss

Affected Token Examples

| Token | Issue | Consequence | |-------|-------|-------------| | USDT | No bool return on transfer/approve | Reverts if caller expects bool return | | USDT | Requires approve(0) before approve(N) | Approve fails for non-zero to non-zero | | BNB | Missing return value | Reverts on standard interface call | | OMG | Missing return value | Reverts on standard interface call | | ZRX | Returns false on failure (no revert) | Silent failure if return unchecked |

Detection Checklist

1. Does the contract use IERC20.transfer() or IERC20.transferFrom() directly?
2. Is OpenZeppelin's SafeERC20 imported and applied via using SafeERC20 for IERC20?
3. Are safeTransfer, safeTransferFrom, and forceApprove used instead of raw calls?
4. Does the contract need to support USDT or other non-standard tokens?

Relationship to Other Patterns

• unchecked-return-values: Covers low-level .call(), .send(), .delegatecall() return values — different from ERC20 interface returns
• weird-tokens: Broader reference covering all non-standard token behaviors — this skill focuses specifically on the transfer/approve safety wrapper pattern
• fee-on-transfer-tokens: Covers amount mismatch due to transfer fees — complementary to this pattern

Remediation

1. Use SafeERC20: Import and apply using SafeERC20 for IERC20 for all ERC20 interactions
2. Use forceApprove: Replace approve() with forceApprove() to handle USDT
3. Audit token list: Verify which tokens the protocol supports and test with non-standard ones
4. Add integration tests: Test deposit/withdraw flows with USDT, USDC, and at least one missing-return-value token

References

• OpenZeppelin SafeERC20
• Weird ERC20 — Missing Return Values
• BailSec audit reports: Hypertrade V3 Core, Meuna, Robinos, SwapX Exchange