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

[L-1] Missing return value check on ERC20 transfer in `withdrawTokens()`

x0jgsleepy

Root + Impact

Description

The ERC20 standard specifies that transfer() should return a boolean value indicating success or failure. While most tokens revert on failure, some non-standard tokens (like USDT) return false instead of reverting.

The withdrawTokens() function calls IERC20(token).transfer() without checking the return value. If the transfer fails silently (returns false), the function will continue execution and emit the TokensWithdrawn event, incorrectly indicating success.

function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
@> IERC20(token).transfer(to, amount); // @audit unchecked return value
emit TokensWithdrawn(to, token , amount);
}

Risk

Likelihood:

  • Tokens like USDT on mainnet return false on failure instead of reverting

  • The issue only manifests when the hook has insufficient token balance or when dealing with non-standard ERC20 tokens

  • Owner might not notice failed withdrawals if they only check events

Impact:

  • Silent failures where tokens are not actually transferred

  • Events incorrectly indicate successful withdrawal

  • Owner believes funds were withdrawn when they weren't

  • Loss of trust in protocol accounting

  • Difficulty debugging when withdrawals don't appear in destination wallet

Proof of Concept

// Mock ERC20 that returns false on transfer failure
contract BadERC20 {
mapping(address => uint256) public balanceOf;
function transfer(address to, uint256 amount) external returns (bool) {
if (balanceOf[msg.sender] < amount) {
return false; // Fails silently instead of reverting
}
balanceOf[msg.sender] -= amount;
balanceOf[to] += amount;
return true;
}
}
​
function test_UncheckedTransferFailsSilently() public {
BadERC20 badToken = new BadERC20();
// Hook has 0 balance of badToken
assertEq(badToken.balanceOf(address(hook)), 0);
// Owner tries to withdraw 100 tokens
vm.prank(owner);
hook.withdrawTokens(address(badToken), owner, 100);
// Event was emitted despite transfer failure
// Owner has 0 balance (transfer failed)
assertEq(badToken.balanceOf(owner), 0);
// But the function didn't revert, creating a false success indication
}

Recommended Mitigation

+import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
​
contract ReFiSwapRebateHook is BaseHook, Ownable {
+ using SafeERC20 for IERC20;
// ...
​
function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
- IERC20(token).transfer(to, amount);
+ IERC20(token).safeTransfer(to, amount);
emit TokensWithdrawn(to, token , 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!