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

_isRefiBuy() wrong logic leads to inverted fees applied

howiecht

_isRefiBuy() wrong logic leads to inverted fees applied

Description

  • The _isReFiBuy function is intended to identify whether a swap is a "buy" or a "sell" of the ReFi token, allowing the _beforeSwap hook to apply the appropriate buyFee or sellFee.

  • The function's logic is inverted, causing it to misidentify buys as sells and sells as buys. Consequently, when a user buys the ReFi token, the sellFee is applied and a ReFiSold event is emitted. Conversely, when a user sells the ReFi token, the buyFee is applied and a ReFiBought event is emitted.

unction _isReFiBuy(PoolKey calldata key, bool zeroForOne) internal view returns (bool) {
bool IsReFiCurrency0 = Currency.unwrap(key.currency0) == ReFi;
​
// @audit applied logic is inverted
if (IsReFiCurrency0) {
@> return zeroForOne;
} else {
@> return !zeroForOne;
}
}

Risk

Likelihood:

  • This logical error occurs on every swap executed through a pool that integrates the ReFiSwapRebateHook.

Impact:

  • The intended fee structure is reversed. Buying the ReFi token is charged a 0.3% fee when they should be charged 0%. This is an unexpected and incorrect charge that results in a loss for the user on every single buy transaction.

Proof of Concept

Running the existing test case test_BuyReFi_ZeroFee with verbose logging (forge test --mt test_BuyReFi_ZeroFee -vvvv) demonstrates the issue. The test executes a "buy" of ReFi by swapping ETH fo ReFi (zeroForOne: true). The trace clearly shows that instead of a ReFiBought event, the hook incorrectly emits a ReFiSold event, proving the logic is inverted.

function test_BuyReFi_ZeroFee() public {
uint256 ethAmount = 0.01 ether;
// Fund user and record initial balances
vm.deal(user1, 1 ether);
// Record initial balances after funding
uint256 initialEthBalance = user1.balance;
uint256 initialReFiBalance = reFiToken.balanceOf(user1);
vm.startPrank(user1);
​
SwapParams memory params = SwapParams({
zeroForOne: true, // ETH -> ReFi
amountSpecified: -int256(ethAmount),
sqrtPriceLimitX96: TickMath.MIN_SQRT_PRICE + 1
});
​
PoolSwapTest.TestSettings memory testSettings =
PoolSwapTest.TestSettings({takeClaims: false, settleUsingBurn: false});
​
// Perform swap
swapRouter.swap{value: ethAmount}(key, params, testSettings, ZERO_BYTES);
vm.stopPrank();
uint256 finalEthBalance = user1.balance;
uint256 finalReFiBalance = reFiToken.balanceOf(user1);
​
// User should have less ETH and more ReFi
assertEq(finalEthBalance, initialEthBalance - ethAmount, "ETH balance should decrease by swap amount");
assertGt(finalReFiBalance, initialReFiBalance, "ReFi balance should increase after buy");
​
// Verify fee configuration remains zero for buys
(uint24 currentBuyFee,) = rebateHook.getFeeConfig();
// assertEq(currentBuyFee, 0, "Buy fee should remain 0");
}
│ │ │ │ ├─ [7225] ReFiSwapRebateHook::beforeSwap(PoolSwapTest: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], PoolKey({ currency0: 0x0000000000000000000000000000000000000000, currency1: 0x212224D2F2d262cd093eE13240ca4873fcCBbA3C, fee: 8388608 [8.388e6], tickSpacing: 60, hooks: 0xF398DeAF12Ba47830ECA5a881175a3690A90F080 }), SwapParams({ zeroForOne: true, amountSpecified: -10000000000000000 [-1e16], sqrtPriceLimitX96: 4295128740 [4.295e9] }), 0x)
│ │ │ │ │ ├─ emit ReFiSold(seller: PoolSwapTest: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], amount: 10000000000000000 [1e16], fee: 300000000000000 [3e14])
│ │ │ │ │ └─ ← [Return] 0x575e24b4, 0, 4197304 [4.197e6]

Recommended Mitigation

function _isReFiBuy(PoolKey calldata key, bool zeroForOne) internal view returns (bool) {
bool IsReFiCurrency0 = Currency.unwrap(key.currency0) == ReFi;
​
// @audit applied logic is inverted
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!