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

Unchecked transfer in the `ReFiSwapRebateHook::withdrawTokens`, does not check return value

alexscherbatyuk

Unchecked ERC20 transfer in withdrawTokens → Silent failure + incorrect event emission on failed withdrawals

Description

  • The withdrawTokens function is intended to allow the owner to withdraw any ERC20 tokens collected by the hook (e.g., fees or rebates).

  • The current implementation calls IERC20(token).transfer(to, amount) directly without checking the returned bool. Many ERC20 tokens (USDT, older tokens, some fee-on-transfer tokens) return false instead of reverting on failure, or have non-standard behavior.

  • As a result, a failed transfer is treated as successful and the TokensWithdrawn event is still emitted, creating an inconsistent on-chain state.

// Root cause in the codebase with @> marks to highlight the relevant section
function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
@> IERC20(token).transfer(to, amount);
emit TokensWithdrawn(to, token, amount);
}

Risk

Likelihood:

  • Any non-standard or legacy ERC20 token deposited into the hook will trigger the issue

  • Tokens that return false on failure (e.g., USDT before CPI upgrade, certain proxy tokens) are still widely used

  • Fee-on-transfer or rebasing tokens may also cause transfer to return false

Impact:

  • Tokens can become permanently stuck in the hook if the transfer silently fails

  • The TokensWithdrawn event lies about a successful withdrawal, breaking off-chain accounting and monitoring

  • Owner may believe funds were sent when they were not, leading to loss of funds or delayed recovery attempts

Proof of Concept

Add the following code snippet to the RebateFiHookTest.t.sol test file.

This is a mock ERC20 token that will revert on the first transfer. Add this contract to the RebateFiHookTest.t.sol file.

contract FailedMockERC20 is MockERC20 {
uint256 public countTransfers;
​
constructor(string memory name, string memory symbol, uint8 decimals) MockERC20(name, symbol, decimals) {}
​
function mint(address to, uint256 amount) public override {
_mint(to, amount);
}
​
function transfer(address to, uint256 amount) public override returns (bool) {
if (countTransfers == 0) {
countTransfers++;
return super.transfer(to, amount);
}
return false;
}
}

This is a test function that will test the ReFiSwapRebateHook::withdrawTokens function with a failed token. Add this test to the RebateFiHookTest.t.sol file.

function test_WithdrawTokens_Failed() public {
FailedMockERC20 failedToken = new FailedMockERC20("Failed", "FAIL", 18);
uint256 transferAmount = 1 ether;
​
failedToken.mint(address(this), transferAmount);
failedToken.approve(address(rebateHook), transferAmount);
​
failedToken.transfer(address(rebateHook), transferAmount);
​
uint256 initialBalance = failedToken.balanceOf(address(this));
uint256 hookBalanceBefore = failedToken.balanceOf(address(rebateHook));
​
console.log("initialBalance", initialBalance);
console.log("hookBalanceBefore", hookBalanceBefore);
​
uint256 withdrawAmount = 0.5 ether;
rebateHook.withdrawTokens(address(failedToken), address(this), withdrawAmount);
​
uint256 finalBalance = failedToken.balanceOf(address(this));
​
console.log("finalBalance", finalBalance);
​
assertEq(finalBalance, initialBalance + withdrawAmount, "Should receive withdrawn tokens");
}

Recommended mitigation

ERC20 functions may not behave as expected. For example: return values are not always meaningful. It is recommended to use OpenZeppelin's SafeERC20 library.

+ 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(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!