RebateFi Hook
First Flight #53
Beginner FriendlyDeFi
100 EXP
View results
Previous Next
Submission Details
Impact: low
Likelihood: medium
Invalid

Unchecked Return Value in Token Transfer

uandersonricardo

Root + Impact

Description

  • The withdrawTokens function is designed to allow the owner to withdraw ERC20 tokens from the hook contract to a specified address. The function calls IERC20(token).transfer(to, amount) to perform the transfer and emits an event to log the withdrawal.

  • However, the function does not check the return value of the transfer call. Some ERC20 tokens (notably USDT) return false instead of reverting when a transfer fails. This means the withdrawal could silently fail while the contract emits a TokensWithdrawn event, creating a false record of a successful transfer when no tokens were actually moved.

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

Risk

Likelihood:

  • The owner attempts to withdraw tokens using a non-standard ERC20 implementation that returns false on failure rather than reverting

The transfer fails due to insufficient balance or other conditions, but the function continues execution

Impact:

  • The TokensWithdrawn event is emitted even though no tokens were transferred, creating misleading on-chain logs

The owner believes tokens were successfully withdrawn when they remain locked in the contract

  • Accounting discrepancies between expected and actual token balances could lead to operational issues

Proof of Concept

function test_WithdrawTokens_UncheckedReturnValue() public {
// Deploy a mock ERC20 that returns false on transfer failure
MockERC20Failing transferFailingToken = new MockERC20Failing();
transferFailingToken.mint(address(rebateHook), 1 ether);
​
uint256 initialBalance = transferFailingToken.balanceOf(address(this));
uint256 hookBalance = transferFailingToken.balanceOf(address(rebateHook));
​
// Attempt to withdraw -> this should fail but won't revert
vm.expectEmit(true, true, true, true);
emit ReFiSwapRebateHook.TokensWithdrawn(address(this), address(transferFailingToken), 0.5 ether);
rebateHook.withdrawTokens(address(transferFailingToken), address(this), 0.5 ether);
​
// Tokens were NOT transferred despite event emission
assertEq(transferFailingToken.balanceOf(address(this)), initialBalance, "Recipient balance unchanged");
assertEq(transferFailingToken.balanceOf(address(rebateHook)), hookBalance, "Hook balance unchanged");
}
​
...
​
contract MockERC20Failing is MockERC20 {
constructor() MockERC20("Failing", "FAIL", 18) {}
​
function transfer(address, uint256) public pure override returns (bool) {
return false; // Simulates non-reverting failure
}
}

Recommended Mitigation

function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
- IERC20(token).transfer(to, amount);
+ require(IERC20(token).transfer(to, amount), "Transfer failed");
emit TokensWithdrawn(to, token , amount);
}

Alternatively, use OpenZeppelin's SafeERC20 library for more robust handling:

+ import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
​
...
​
+ 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
15 days ago
chaossr Lead Judge 12 days ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!