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

Event Parameter Order Inconsistency in TokensWithdrawn

mostafapahlevani93

Description

The normal behavior should maintain consistent parameter ordering in event emissions and definitions. Events should follow standard conventions for better code quality and off-chain tracking.

The TokensWithdrawn event definition and its emission have inconsistent parameter ordering. The event is defined with parameters (token, to, amount) but emitted with parameters (to, token, amount), creating confusion and potential issues for off-chain indexers.

// Event definition
@> event TokensWithdrawn(address indexed token, address indexed to, uint256 amount);
​
// Event emission in withdrawTokens function
function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
IERC20(token).transfer(to, amount);
@> emit TokensWithdrawn(to, token , amount); // ❌ Parameters in wrong order: (to, token) instead of (token, to)
}

Expected: emit TokensWithdrawn(token, to, amount);
Actual: emit TokensWithdrawn(to, token, amount);

Risk

Likelihood:

  • Occurs every time withdrawTokens() is called

  • All withdrawal events are affected

  • Off-chain monitoring tools receive incorrect data

  • Event indexers store swapped addresses

Impact:

  • Off-chain systems misinterpret withdrawal events

  • Event listeners receive token address where recipient expected

  • Dashboard displays incorrect withdrawal information

  • Audit trails show wrong token/recipient mapping

  • Analytics tools produce incorrect reports

  • Automated monitoring may trigger false alerts

  • Reduced code maintainability and clarity

  • Developer confusion when reading event logs

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.26;
​
import "forge-std/Test.sol";
import "forge-std/console.sol";
​
contract EventParameterTest is Test {
// Event definition (correct order)
event TokensWithdrawn(address indexed token, address indexed to, uint256 amount);
address constant TOKEN = address(0x1111);
address constant RECIPIENT = address(0x2222);
uint256 constant AMOUNT = 1000 ether;
function test_EventParameterOrderBug() public {
console.log("\n=== Event Parameter Order Bug ===");
console.log("Event Definition: TokensWithdrawn(address indexed token, address indexed to, uint256 amount)");
console.log("");
console.log("Function parameters:");
console.log(" token:", TOKEN);
console.log(" to:", RECIPIENT);
console.log(" amount:", AMOUNT);
// Buggy emission (wrong order)
console.log("\nBuggy emission: emit TokensWithdrawn(to, token, amount);");
console.log(" First indexed param (should be token):", RECIPIENT);
console.log(" Second indexed param (should be to):", TOKEN);
console.log(" Third param (amount):", AMOUNT);
// Record event with buggy order
vm.recordLogs();
emit TokensWithdrawn(RECIPIENT, TOKEN, AMOUNT); // Buggy: swapped order
Vm.Log[] memory entries = vm.getRecordedLogs();
console.log("\n=== Off-Chain Impact ===");
console.log("Event indexer reads:");
console.log(" token field =", RECIPIENT, "(WRONG - this is actually the recipient)");
console.log(" to field =", TOKEN, "(WRONG - this is actually the token)");
console.log(" amount field =", AMOUNT, "(correct)");
// Demonstrate correct emission
console.log("\n=== Correct Emission ===");
console.log("Correct emission: emit TokensWithdrawn(token, to, amount);");
vm.recordLogs();
emit TokensWithdrawn(TOKEN, RECIPIENT, AMOUNT); // Correct order
entries = vm.getRecordedLogs();
console.log("Event indexer reads:");
console.log(" token field =", TOKEN, "(correct)");
console.log(" to field =", RECIPIENT, "(correct)");
console.log(" amount field =", AMOUNT, "(correct)");
}
function test_OffChainQueryImpact() public {
console.log("\n=== Off-Chain Query Impact ===");
// Scenario: Off-chain system queries "all withdrawals of token X"
address targetToken = address(0xABCD);
console.log("\nQuery: Show all withdrawals of token", targetToken);
// With buggy emission
console.log("\nWith BUGGY emission:");
console.log(" Event emitted as: TokensWithdrawn(recipient, token, amount)");
console.log(" Indexer stores: token=recipient, to=token");
console.log(" Query result: MISS (token field contains recipient, not token)");
console.log(" Impact: Cannot find withdrawals by token address");
// With correct emission
console.log("\nWith CORRECT emission:");
console.log(" Event emitted as: TokensWithdrawn(token, recipient, amount)");
console.log(" Indexer stores: token=token, to=recipient");
console.log(" Query result: HIT (token field contains token address)");
console.log(" Impact: Queries work as expected");
}
function test_DashboardDisplayImpact() public {
console.log("\n=== Dashboard Display Impact ===");
address token = address(0x1234);
address recipient = address(0x5678);
uint256 amount = 5000 ether;
console.log("\nActual withdrawal:");
console.log(" Token:", token);
console.log(" Recipient:", recipient);
console.log(" Amount:", amount);
console.log("\nDashboard with BUGGY events:");
console.log(" Displays:");
console.log(" Token: ", recipient, "(WRONG)");
console.log(" Recipient:", token, "(WRONG)");
console.log(" Amount:", amount, "(correct)");
console.log(" Result: Confusing and incorrect information");
console.log("\nDashboard with CORRECT events:");
console.log(" Displays:");
console.log(" Token:", token, "(correct)");
console.log(" Recipient:", recipient, "(correct)");
console.log(" Amount:", amount, "(correct)");
console.log(" Result: Accurate information");
}
}

Running the PoC:

forge test --match-contract EventParameterTest -vv

Recommended Mitigation

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

Alternative: Update Event Definition to Match Emission

If there's a reason to keep the emission order, update the event definition instead (not recommended):

- event TokensWithdrawn(address indexed token, address indexed to, uint256 amount);
+ event TokensWithdrawn(address indexed to, address indexed token, uint256 amount);
​
function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
IERC20(token).transfer(to, amount);
emit TokensWithdrawn(to, token, amount); // Now matches definition
}

Recommended: Fix the emission to match the definition (Option 1) because:

  1. Token-first ordering is more standard

  2. Maintains existing event signature

  3. Aligns with function parameter order

  4. Follows common event naming conventions

Updates

Lead Judging Commences

chaossr Lead Judge 8 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!