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 about 1 month 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!