RebateFi Hook

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

Unchecked ERC20 Transfer in withdrawTokens Can Silently Fail

Root + Impact

Description

  • Normal Behavior: The withdrawTokens function should verify that token transfers succeed and revert if they fail.

  • Specific Issue: The function uses IERC20.transfer() without checking the return value, which can silently fail for non-standard ERC20 tokens:

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

Risk

Likelihood:

  • Only affects non-standard ERC20 tokens that return false instead of reverting

  • Requires owner to attempt withdrawal of such tokens

  • Some tokens (e.g., USDT on some chains) don't return bool values

Impact:

  • Owner believes tokens were withdrawn but they remain in contract

  • Event is emitted indicating success when transfer actually failed

  • Accumulated fees may become stuck in contract

  • Loss of protocol revenue

Proof of Concept

// Non-standard token that returns false on failure
contract BadToken {
function transfer(address to, uint256 amount) external returns (bool) {
// Insufficient balance - returns false instead of reverting
if (balanceOf[msg.sender] < amount) return false;
// ... transfer logic
}
}
// Owner tries to withdraw
rebateHook.withdrawTokens(address(badToken), owner, 1000 ether);
// Transfer fails (returns false) but function doesn't revert
// Event TokensWithdrawn is emitted
// Owner thinks withdrawal succeeded but tokens are still in contract

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 8 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!