RebateFi Hook

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

`ReFiSwapRebateHook::withdrawTokens` function emits `TokensWithdrawn` event with wrong parameters order

Root + Impact

Description

  • TokensWithdrawn event is declared as follows:

event TokensWithdrawn(address indexed token, address indexed to, uint256 amount);
  • ReFiSwapRebateHook::withdrawTokens function emits TokensWithdrawn event with wrong parameters order:

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

Risk

Likelihood:

  • Issue occurs every time the ReFiSwapRebateHook::withdrawTokens is called and when the ERC20 transfer is not reverted.

Impact:

  • Wrong data is provided by the protocol to any concerned third-party, reducing on-chain observability and making it harder for off-chain indexers, UIs, and auditors to track history.

Proof of Concept

Add the following test to TestReFiSwapRebateHook.sol and see the test result as follows:
[FAIL: TokensWithdrawn param mismatch at token: expected=0x212224D2F2d262cd093eE13240ca4873fcCBbA3C, got=0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496, to: expected=0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496, got=0x212224D2F2d262cd093eE13240ca4873fcCBbA3C]

function test_EventParamsMismatch() public {
uint256 dealAmount = 1 ether;
uint256 withdrawAmount = dealAmount / 2;
deal(address(reFiToken), address(rebateHook), dealAmount);
uint256 hookBalanceBefore = IERC20(address(reFiToken)).balanceOf(address(rebateHook));
uint256 ownerBalanceBefore = IERC20(address(reFiToken)).balanceOf(address(this));
vm.expectEmit(true, true, false, false);
emit TokensWithdrawn(address(reFiToken), address(this), withdrawAmount);
rebateHook.withdrawTokens(address(reFiToken), address(this), withdrawAmount);
uint256 hookBalanceAfter = IERC20(address(reFiToken)).balanceOf(address(rebateHook));
uint256 ownerBalanceAfter = IERC20(address(reFiToken)).balanceOf(address(this));
assertEq(hookBalanceBefore - hookBalanceAfter, withdrawAmount);
assertEq(ownerBalanceAfter - ownerBalanceBefore, withdrawAmount);
}

Recommended Mitigation

Fix the order of params in the emitted TokensWithdrawn event to conform the specified event signature:

-emit TokensWithdrawn(to, token, amount);
+emit TokensWithdrawn(token, to, amount);
Updates

Lead Judging Commences

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