Submission Details
Severity:
Incorrect Event Parameter Ordering in `withdrawTokens` function, Leading to Misleading On-Chain Logs.
Incorrect Event Parameter Ordering in withdrawTokens function, Leading to Misleading On-Chain Logs.
Description
The
withdrawTokensfunction event emits its parameters in the wrong order (tois logged astoken, andtokenis logged asto). This causes incorrect on-chain logs, misleads indexers/subgraphs, and breaks any off-chain system relying on event data for accounting, monitoring, or withdrawals tracking.
event TokensWithdrawn(address indexed token, address indexed to, uint256 amount);
/// @notice Withdraws tokens from the hook contract
/// @param token Address of the token to withdraw
/// @param to Address to send the tokens to
/// @param amount Amount of tokens to withdraw
/// @dev Only callable by owner
function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
IERC20(token).transfer(to, amount);
@> emit TokensWithdrawn(to, token , amount);
}
Risk
Likelihood:
It's highly probable to happen.
Impact:
There's some level of disruption to the protocol's functionality or availability.
Proof of Concept
<details>
<summary>POC</summary>
1. Import it in `RebateFiHookTest.t.sol` test file.
import {Vm} from "forge-std/Vm.sol";
2. Add this test to `RebateFiHookTest.t.sol` test file.
```javascript
function test_TokensWithdrawn_EventMisordered() public {
address tokenAddr = address(reFiToken);
address recipient = user1;
uint256 amount = 1 ether;
// Fund hook
reFiToken.mint(address(rebateHook), amount);
// Start capturing EVM logs
vm.recordLogs();
// Call function (this emits the WRONG event order)
rebateHook.withdrawTokens(tokenAddr, recipient, amount);
// Read logs from EVM
Vm.Log[] memory logs = vm.getRecordedLogs();
// TokensWithdrawn signature
bytes32 sig = keccak256("TokensWithdrawn(address,address,uint256)");
// Variables to store decoded values
address loggedParam1;
address loggedParam2;
uint256 loggedAmount;
bool found;
for (uint256 i = 0; i < logs.length; i++) {
if (logs[i].topics[0] == sig) {
found = true;
// Decode actual emitted parameters
loggedParam1 = address(uint160(uint256(logs[i].topics[1])));
loggedParam2 = address(uint160(uint256(logs[i].topics[2])));
loggedAmount = abi.decode(logs[i].data, (uint256));
}
}
require(found, "Event not emitted");
// ----------- EXPECTED CORRECT ORDER --------------
// tokenAddr = token
// recipient = to
//
// ----------- ACTUAL WRONG ORDER ------------------
// loggedParam1 = recipient (WRONG)
// loggedParam2 = token (WRONG)
// This comparison proves the flaw clearly:
assertEq(loggedParam1, recipient, "BUG: event first param == to (should be token)");
assertEq(loggedParam2, tokenAddr, "BUG: event second param == token (should be to)");
assertEq(loggedAmount, amount);
}
```
</details>
Recommended Mitigation
1.Modify the `withdrawTokens` function to work correctly.
```diff
/// @notice Withdraws tokens from the hook contract
/// @param token Address of the token to withdraw
/// @param to Address to send the tokens to
/// @param amount Amount of tokens to withdraw
/// @dev Only callable by owner
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);
}
```
Updates
Lead Judging Commences
chaossr 12 days ago
Submission Judgement Published Assigned finding tags:
Swapped token and to parameters in TokensWithdrawn event.
Rewards breakdown
nSLOC
150This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.