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

Incorrect Event Parameter Order in withdrawTokens

samuelsmith442

Scope

Affected Files:

  • RebateFiHook.sol: Lines 72-75

Affected Functions:

  • RebateFiHook.withdrawTokens()

Description

The withdrawTokens() function emits the TokensWithdrawn event with parameters in the wrong order, causing event logs to be misleading and breaking off-chain indexing/monitoring systems.

Expected Behavior

The event should emit in the order: (token, to, amount) to match the function parameters and be intuitive for off-chain systems.

Actual Behavior

The event emits as: (to, token, amount), swapping the first two parameters. This causes:

  • Off-chain indexers to misinterpret which token was withdrawn

  • Monitoring systems to log incorrect data

  • Frontend dashboards to display wrong information

  • Difficulty in auditing token flows

Root Cause

// VULNERABLE CODE (lines 72-75)
function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
IERC20(token).transfer(to, amount);
emit TokensWithdrawn(to, token , amount); // ← BUG: Parameters in wrong order
}

The event definition (line 43) expects the token first:

event TokensWithdrawn(address indexed token, address indexed to, uint256 amount);

But the emit statement passes to first and token second.

Risk Assessment

Impact

Medium - While this doesn't directly affect on-chain functionality, it causes:

  1. Off-chain system failures: Blockchain indexers (The Graph, Etherscan, etc.) will misinterpret events

  2. Audit trail corruption: Historical records will show wrong token addresses

  3. Monitoring alerts: Systems tracking token withdrawals will trigger on wrong tokens

  4. User confusion: Users checking transaction logs will see incorrect token information

Likelihood

High - This is a simple parameter ordering bug that would be caught by:

  • Any off-chain monitoring system

  • Manual inspection of transaction logs

  • Integration testing with event listeners

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.26;
​
import "forge-std/Test.sol";
import {ReFiSwapRebateHook} from "../src/RebateFiHook.sol";
import {ReFi} from "../src/ReFi.sol";
import {MockERC20} from "solmate/src/test/utils/mocks/MockERC20.sol";
import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol";
​
contract Bug2Test is Test {
ReFiSwapRebateHook hook;
ReFi reFiToken;
MockERC20 testToken;
IPoolManager manager;
event TokensWithdrawn(address indexed token, address indexed to, uint256 amount);
function setUp() public {
manager = IPoolManager(address(0x1)); // Mock
reFiToken = new ReFi();
testToken = new MockERC20("TEST", "TEST", 18);
hook = new ReFiSwapRebateHook(manager, address(reFiToken));
// Fund the hook with test tokens
testToken.mint(address(hook), 1000e18);
}
function testEventParameterOrderBug() public {
address recipient = address(0x123);
uint256 withdrawAmount = 100e18;
// Listen for the event
vm.recordLogs();
// Call withdrawTokens
vm.prank(hook.owner());
hook.withdrawTokens(address(testToken), recipient, withdrawAmount);
// Get the emitted logs
Vm.Log[] memory logs = vm.getRecordedLogs();
// Find the TokensWithdrawn event
for (uint i = 0; i < logs.length; i++) {
if (logs[i].topics[0] == keccak256("TokensWithdrawn(address,address,uint256)")) {
// Decode the event
address emittedToken = address(uint160(uint256(logs[i].topics[1])));
address emittedTo = address(uint160(uint256(logs[i].topics[2])));
console.log("Expected token:", address(testToken));
console.log("Emitted token (topics[1]):", emittedToken);
console.log("");
console.log("Expected recipient:", recipient);
console.log("Emitted recipient (topics[2]):", emittedTo);
// BUG: The emitted values are swapped!
// emittedToken will be the recipient address
// emittedTo will be the token address
assertEq(emittedToken, recipient, "BUG: First indexed param is recipient, not token!");
assertEq(emittedTo, address(testToken), "BUG: Second indexed param is token, not recipient!");
}
}
}
function testCorrectEventEmission() public {
address recipient = address(0x123);
uint256 withdrawAmount = 100e18;
// This test shows what the correct event should look like
vm.expectEmit(true, true, true, true);
emit TokensWithdrawn(address(testToken), recipient, withdrawAmount);
// The actual call emits with wrong order
vm.prank(hook.owner());
hook.withdrawTokens(address(testToken), recipient, withdrawAmount);
}
}

Console Output

Expected token: 0x...testToken...
Emitted token (topics[1]): 0x...recipient...
​
Expected recipient: 0x...recipient...
Emitted recipient (topics[2]): 0x...testToken...
​
BUG: First indexed param is recipient, not token!
BUG: Second indexed param is token, not recipient!

Recommended Mitigation

Before: Vulnerable Code

function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
IERC20(token).transfer(to, amount);
emit TokensWithdrawn(to, token , amount); // ← Wrong order
}

After: Fixed Code

function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
IERC20(token).transfer(to, amount);
emit TokensWithdrawn(token, to, amount); // ← Correct order
}

Explanation

Simply swap the order of to and token in the emit statement to match:

  1. The event definition signature

  2. The function parameter order

  3. Intuitive semantics (token first, then recipient)

This ensures off-chain systems correctly interpret which token was withdrawn and to which address.

Impact on Uniswap V4 Context

In Uniswap V4 hooks, event emissions are critical for:

  • Monitoring accumulated fees: Off-chain systems track token accumulation in the hook

  • Governance tracking: DAOs need accurate records of treasury movements

  • Compliance: Regulatory systems need correct audit trails

  • User notifications: Users need to verify their token withdrawals

Incorrect event parameter order breaks all these systems, making it a significant issue despite not affecting on-chain logic.

Additional Notes

This is a common mistake in Solidity development where developers copy-paste event emissions without carefully checking parameter order. The fix is trivial but the impact on system reliability is significant.

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!