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

The inverted logic in the `ReFiSwapRebateHook::_isReFiBuy` function causes the swaps to be performed in the reverse direction.

farnad

Root + Impact

Description

  • Due to the faulty logic in the ReFiSwapRebateHook::_isReFiBuy function, the buy orders are treated as sell orders, and vice versa.

  • If ReFi token is equal to Currency.unwrap(key.currency0) and the zeroForOne argument is true, it means ReFi is being sold, not bought.

  • It actually is so severe that totally breaks the invariants of the system (what the swapper CAN do).

function _isReFiBuy(PoolKey calldata key, bool zeroForOne) internal view returns (bool) {
bool IsReFiCurrency0 = Currency.unwrap(key.currency0) == ReFi;
@> if (IsReFiCurrency0) {
@> return zeroForOne;
@> } else {
@> return !zeroForOne;
}
}

Risk

Likelihood: High

  • It occurs every single time a user wants to swap tokens.

  • It is called by the ReFiSwapRebateHook::_beforeSwap function and is used to determine if the user is going to buy or sell the ReFi tokens.


Impact: High

  • Since this function is used to determine the buy/sell direction of the swap, the wrong return values drastically disrupt the functionality of the protocol.

  • It makes the calling function (_beforeSwap) do the opposite of what the user intends to do.

Proof of Concept

By running the following command in the terminal and paying attention to the emitted events, you will notice that inspite of executing a sell swap, the ReFiBought event is emitted which obviously means a buy order has been executed instead of a desired sell order.

forge test --mt test_SellReFi_AppliesFee -vvvv

​

Recommended Mitigation

In order to solve the issue, please make the following adjustments.

function _isReFiBuy(PoolKey calldata key, bool zeroForOne) internal view returns (bool) {
bool IsReFiCurrency0 = Currency.unwrap(key.currency0) == ReFi;
if (IsReFiCurrency0) {
- return zeroForOne;
+ return !zeroForOne;
} else {
- return !zeroForOne;
+ return zeroForOne;
}
}
Updates

Lead Judging Commences

chaossr Lead Judge 12 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Inverted buy/sell logic when ReFi is currency0, leading to incorrect fee application.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!