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

Event Emitted on Failed ERC20 Transfer, Causing Misleading Withdrawal Signals

shashankwcw

The function ignores the boolean return value of transfer(), failing to detect tokens that signal transfer failure by returning false instead of reverting. Because the function does not verify success, it emits a withdrawal event even when no tokens were transferred. This misleads users and off-chain systems, resulting in incorrect assumptions and inaccurate accounting.

Description

  • The contract should only emit a withdrawal event after a successful ERC20 token transfer.

  • The transfer operation should revert or fail cleanly when the underlying token does not execute the transfer correctly.

  • The function calls transfer() without checking its boolean return value, allowing silent failures for tokens that return false instead of reverting.

  • As a result, the withdrawal event is emitted even when no tokens were transferred, creating misleading signals for users, bots, and off-chain systems.

// 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:

  • Occurs whenever interacting with ERC20 tokens or non ERC20 that do not revert on failure but instead return false.

  • Occurs when balance is insufficient, when transfers are blocked by token logic, or when the token implements alternative failure semantics.

Impact:

  • Events indicate successful withdrawals despite failed transfers.

  • Off-chain systems relying on event logs become inconsistent with true on-chain balances.

  • Operators may believe tokens have been withdrawn when they have not, potentially enabling accounting discrepancies or operational mistakes.

Proof of Concept

  • For this PoC to succedd and work as intended you have to deploy a contract which return a bool value upon transfer

function test_withdrawTokens_EmitsEventButDoesNotTransfer() public {
uint256 amount = 20 ether;
​
uint256 beforeUser = badToken.balanceOf(user);
uint256 beforeHook = badToken.balanceOf(address(rebateHook));
​
// expect the event
vm.expectEmit(true, true, true, true);
emit TokensWithdrawn(user,address(badToken), amount);
​
// call withdrawTokens (this test contract is owner)
rebateHook.withdrawTokens(address(badToken), user, amount);
​
// ❌ no actual transfer happened since BadToken.transfer returned false
assertEq(badToken.balanceOf(user), beforeUser, "user balance must NOT increase");
assertEq(badToken.balanceOf(address(rebateHook)), beforeHook, "hook balance must NOT decrease");
}
​
/*
////////////////////////////////////////////// Output /////////////////////////////////////////////////////
*/
​
Ran 1 test for test/Newtest.t.sol:WithdrawTokens_BadToken_Test
[PASS] test_withdrawTokens_EmitsEventButDoesNotTransfer() (gas: 36188)
Traces:
[36188] WithdrawTokens_BadToken_Test::test_withdrawTokens_EmitsEventButDoesNotTransfer()
├─ [2824] BadToken::balanceOf(0x000000000000000000000000000000000000bEEF) [staticcall]
│ └─ ← [Return] 0
├─ [2824] BadToken::balanceOf(ReFiSwapRebateHook: [0x523c7891De963A9A172d2B926fE8BEF33277f080]) [staticcall]
│ └─ ← [Return] 1000000000000000000000 [1e21]
├─ [0] VM::expectEmit(true, true, true, true)
│ └─ ← [Return]
├─ emit TokensWithdrawn(token: 0x000000000000000000000000000000000000bEEF, to: BadToken: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], amount: 20000000000000000000 [2e19])
├─ [6775] ReFiSwapRebateHook::withdrawTokens(BadToken: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 0x000000000000000000000000000000000000bEEF, 20000000000000000000 [2e19])
│ ├─ [876] BadToken::transfer(0x000000000000000000000000000000000000bEEF, 20000000000000000000 [2e19])
│ │ └─ ← [Return] false
│ ├─ emit TokensWithdrawn(token: 0x000000000000000000000000000000000000bEEF, to: BadToken: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], amount: 20000000000000000000 [2e19])
│ └─ ← [Stop]
├─ [824] BadToken::balanceOf(0x000000000000000000000000000000000000bEEF) [staticcall]
│ └─ ← [Return] 0
├─ [824] BadToken::balanceOf(ReFiSwapRebateHook: [0x523c7891De963A9A172d2B926fE8BEF33277f080]) [staticcall]
│ └─ ← [Return] 1000000000000000000000 [1e21]
└─ ← [Stop]
​

Recommended Mitigation

  • Just replace the existing code in withdrawToken with the code given below

function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
bool success = IERC20(token).transfer(to, amount);
if(success){
emit TokensWithdrawn(to, token , amount);
}
}
Updates

Lead Judging Commences

chaossr Lead Judge 12 days ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Appeal created

shashankwcw Submitter
11 days ago
chaossr Lead Judge
10 days ago
chaossr Lead Judge 8 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!