RebateFi Hook

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

Incorrect Parameter Ordering When Emitting TokensWithdrawn Event

The TokensWithdrawn event is declared as (token, to, amount) but the function emits the parameters in a different order. This causes event logs to map indexed parameters incorrectly, misleading all off-chain systems that rely on topic order for tracking withdrawals. The impact is significant because event data is immutable and heavily relied upon for monitoring and accounting.

Description

  • Describe the normal behavior in one or more sentences

  • Explain the specific issue or problem in one or more sentences

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

Risk

Likelihood:

  • Very likely to occur because the incorrect order is hard-coded, meaning every emission produces corrupted logs.

Impact:

  • Event listeners will misidentify the token address and recipient address, breaking indexing pipelines, dashboards, analytics, and audit trails.

  • Data consumers may assume withdrawals occurred to incorrect addresses or involving incorrect tokens.

Proof of Concept

  • In the test output we can clearly see that the order of the emittion of the event is wrong in context of the event declaration,

function test_WithdrawTokens() public {
// First, send some tokens to the hook contract
uint256 transferAmount = 1 ether;
reFiToken.transfer(address(rebateHook), transferAmount);
uint256 initialBalance = reFiToken.balanceOf(address(this));
uint256 hookBalanceBefore = reFiToken.balanceOf(address(rebateHook));
// Withdraw a reasonable amount
uint256 withdrawAmount = 0.5 ether;
vm.expectEmit();
emit TokensWithdrawn(address(this), address(reFiToken), withdrawAmount);
rebateHook.withdrawTokens(address(reFiToken), address(this), withdrawAmount);
}
/*
///////////////////////////////////////////////// Output ////////////////////////////////////////
*/
[PASS] test_WithdrawTokens_Success() (gas: 58711)
Traces:
[58711] TestReFiSwapRebateHook::test_WithdrawTokens_Success()
├─ [30329] MockERC20::transfer(ReFiSwapRebateHook: [0x5Ea76dced871d32C594E967FA4D07df7832AF080], 1000000000000000000 [1e18])
│ ├─ emit Transfer(from: TestReFiSwapRebateHook: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], to: ReFiSwapRebateHook: [0x5Ea76dced871d32C594E967FA4D07df7832AF080], value: 1000000000000000000 [1e18])
│ └─ ← [Return] true
├─ [825] MockERC20::balanceOf(TestReFiSwapRebateHook: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496]) [staticcall]
│ └─ ← [Return] 998900000000000000000 [9.989e20]
├─ [825] MockERC20::balanceOf(ReFiSwapRebateHook: [0x5Ea76dced871d32C594E967FA4D07df7832AF080]) [staticcall]
│ └─ ← [Return] 1000000000000000000 [1e18]
├─ [9528] ReFiSwapRebateHook::withdrawTokens(MockERC20: [0x2a07706473244BC757E10F2a9E86fB532828afe3], TestReFiSwapRebateHook: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], 500000000000000000 [5e17])
│ ├─ [3629] MockERC20::transfer(TestReFiSwapRebateHook: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], 500000000000000000 [5e17])
│ │ ├─ emit Transfer(from: ReFiSwapRebateHook: [0x5Ea76dced871d32C594E967FA4D07df7832AF080], to: TestReFiSwapRebateHook: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], value: 500000000000000000 [5e17])
│ │ └─ ← [Return] true
│ ├─ emit TokensWithdrawn(token: TestReFiSwapRebateHook: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], to: MockERC20: [0x2a07706473244BC757E10F2a9E86fB532828afe3], amount: 500000000000000000 [5e17])
│ └─ ← [Stop]
├─ [825] MockERC20::balanceOf(TestReFiSwapRebateHook: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496]) [staticcall]
│ └─ ← [Return] 999400000000000000000 [9.994e20]
├─ [825] MockERC20::balanceOf(ReFiSwapRebateHook: [0x5Ea76dced871d32C594E967FA4D07df7832AF080]) [staticcall]
│ └─ ← [Return] 500000000000000000 [5e17]
└─ ← [Stop]

Recommended Mitigation

  • Either change the parameter while event declaration or change the parameters while emittion of event.

  • I am changing the event emission in the TokensWithdrawn function

- emit TokensWithdrawn(to, token , amount);
+ emit TokensWithdrawn(token,to ,amount);
Updates

Lead Judging Commences

chaossr Lead Judge 12 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Swapped token and to parameters in TokensWithdrawn event.

Support

FAQs

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

Give us feedback!