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

L01. Unsafe Token Transfer in withdrawTokens Function

accessdenied

Root + Impact

Description

In normal behavior, the withdrawTokens function allows the contract owner to withdraw ERC20 tokens from the hook contract to a designated address. The function is expected to safely transfer tokens and emit an accurate event reflecting the withdrawal.

The issue is twofold:

  1. The function uses a raw IERC20(token).transfer(to, amount) call. Some non-standard ERC20 tokens do not revert on failure and instead return false. Using raw transfer may silently fail, leaving tokens locked in the contract.

  2. The emitted TokensWithdrawn event has arguments in the wrong order (to, token, amount), which can cause incorrect indexing or misinterpretation of event data.

// Root cause in the codebase with @> marks to highlight the relevant section
​
function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
// @> unsafe raw ERC20 transfer that may fail silently
IERC20(token).transfer(to, amount);
// @> event argument order swapped
emit TokensWithdrawn(to, token, amount);
}
​

Risk

Likelihood:

  • High, because any call to withdrawTokens on non-standard ERC20 tokens that return false on failed transfer will trigger this issue.

  • Medium, as the event mismatch affects monitoring, analytics, and off-chain tracking tools relying on correct event data.

Impact:

  • Tokens may become permanently stuck in the hook contract if transfer fails silently.

  • Event data is unreliable, which can mislead auditing, monitoring, or user interfaces tracking withdrawals.

Proof of Concept

Explanation

A non-standard ERC20 token is deployed that returns false on transfer failure instead of reverting. When the owner calls withdrawTokens, the transfer does not revert, but no tokens are moved. The event emitted does not correctly identify the token and recipient.

function test_WithdrawTokens_UnsafeTransfer() public {
address attackerToken = address(new NonStandardERC20());
uint256 amount = 1000 ether;
​
// Transfer tokens to hook
NonStandardERC20(attackerToken).mint(address(rebateHook), amount);
​
vm.prank(rebateHook.owner());
​
// Call withdrawTokens; transfer silently fails
rebateHook.withdrawTokens(attackerToken, address(this), amount);
​
// Assert tokens are still in the hook contract
assertEq(NonStandardERC20(attackerToken).balanceOf(address(rebateHook)), amount, "Tokens should remain in contract");
​
// Assert event mismatch (optional check depending on event log capture)
}
​

Written explanation:

  • The test shows that withdrawTokens fails to safely transfer non-standard ERC20 tokens.

  • Tokens remain trapped in the contract, demonstrating the vulnerability.

Recommended Mitigation

Use OpenZeppelin SafeERC20 to handle non-standard tokens and fix the event argument order.

function withdrawTokens(address token, address to, uint256 amount)
- IERC20(token).transfer(to, amount);
- emit TokensWithdrawn(to, token, amount);
+ import {SafeERC20, IERC20 as OZIERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
+ using SafeERC20 for OZIERC20;
+ OZIERC20(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!