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

The parameters of the TokensWithdrawn event have been swapped.

cryptomv3

The event RebateFiHook::TokensWithdrawn parameters 'to' and 'token' are swapped in the function RebateFiHook::withdrawTokens()

Description

  • It should emit the event TokensWithdrawn(token, to, amount)

  • In reality, it emits TokensWithdrawn(to, token, amount)

// Root cause in the codebase with @> marks to highlight the relevant section
function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
IERC20(token).transfer(to, amount);
emit TokensWithdrawn(to, token, amount); // <@
}

Risk

Likelihood:

  • This will occur, when the owner calls the RebateFiHook::withdrawTokens() function for withdraw tokens from the hook contract.

Impact:

  • The logs will show the addresses swapped: token= recepient address and to= token address. It may:

    • confuse the frontend or backend that listens to the event;

    • cause incorrect display of transaction history in the UI;

    • cause false positives in off-chain monitoring.

Proof of Concept

In this PoC, tokens are first transferred to the hook contract and then the RebateFiHook::withdrawTokens() function is called to withdraw tokens. This function emits the RebateFiHook::TokensWithdrawn event.
To demonstrate the problem, the expected and actual logs are displayed.
The assertions section below verifies that the parameters have been swapped.

function test_PoC_SwappedParamsInTheEvent() public {
// First, send some tokens to the hook contract
uint256 transferAmount = 1 ether;
reFiToken.transfer(address(rebateHook), transferAmount);
​
// Withdraw a reasonable amount
uint256 withdrawAmount = 0.5 ether;
vm.recordLogs();
rebateHook.withdrawTokens(address(reFiToken), address(this), withdrawAmount);
​
console.log("event TokensWithdrawn(address indexed token, address indexed to, uint256 amount), where:");
console.log("token =", address(reFiToken));
console.log("to =", address(this));
console.log("\nExpectation:");
console.log("TokensWithdrawn(%s, %s, %s)", address(reFiToken), address(this), withdrawAmount);
​
uint256 finalBalance = reFiToken.balanceOf(address(this));
uint256 hookBalanceAfter = reFiToken.balanceOf(address(rebateHook));
​
// Check logs
Vm.Log[] memory entries = vm.getRecordedLogs();
bytes32 expectedTopic = keccak256(abi.encodePacked("TokensWithdrawn(address,address,uint256)"));
​
bool found = false;
address tokenIndexed;
address toIndexed;
uint256 amount;
​
for (uint256 i = 0; i < entries.length; i++) {
if (entries[i].topics[0] == expectedTopic) {
found = true;
tokenIndexed = address(uint160(uint256(entries[i].topics[1])));
toIndexed = address(uint160(uint256(entries[i].topics[2])));
amount = abi.decode(entries[i].data, (uint256));
​
console.log("\nReality:");
console.log("TokensWithdrawn(%s, %s, %s)\n", tokenIndexed, toIndexed, amount);
​
break;
}
}
// Checking that the parameters have been swapped
assertEq(tokenIndexed, address(this));
assertEq(toIndexed, address(reFiToken));
}

Recommended Mitigation

You need to swap the 'to' and 'token' event parameters.

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 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!