RebateFi Hook
First Flight #53
Beginner FriendlyDeFi
100 EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: high
Invalid

Inconsistent Fee Calculation in `_beforeSwap()`

lallanaad

Root + Impact

Different fee calculation approaches between buy and sell paths:• Buy path → assigns fee = buyFee directly, no fee amount calculation or logging. Sell path → calculates feeAmount = (swapAmount * sellFee) / 100000 but does not use it for actual fee application.

Description

The _beforeSwap() implementation applies fees inconsistently. The buy path applies the fee directly from buyFee without computing or emitting the actual fee amount, while the sell path performs a calculation but only logs it in an event without enforcing it. This mismatch creates a vulnerable and confusing fee structure, undermining protocol economics, auditability, and maintainability.

// Root cause in the codebase with @> marks to highlight the relevant section

Risk

Likelihood:

Inconsistent implementation is already present
Multiple attack vectors identified
High probability of exploitation
Impact:

High potential financial impact
Significant security implications
Major operational concerns

Proof of Concept

Add below code to RebateFiHookTest.t.sol

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import "forge-std/Test.sol";
import "../src/ReFiSwapRebateHook.sol";
​
contract ReFiSwapRebateHookTest is Test {
ReFiSwapRebateHook public hook;
address public constant ALICE = address(0x1337);
address public constant BOB = address(0x1338);
uint24 public constant DEFAULT_BUY_FEE = 100;
uint24 public constant DEFAULT_SELL_FEE = 200;
​
function setUp() public {
hook = new ReFiSwapRebateHook();
vm.label(ALICE, "Alice");
vm.label(BOB, "Bob");
}
​
function testFeeCalculation_BuyPath() public {
vm.prank(ALICE);
uint256 swapAmount = 1000 ether;
vm.expectEmit(hook, ReFiBought.selector);
emit ReFiBought(ALICE, swapAmount);
uint24 fee = hook.calculateFee(swapAmount, true);
assertEq(fee, DEFAULT_BUY_FEE, "Buy fee should match default");
}
​
function testFeeCalculation_SellPath() public {
vm.prank(ALICE);
uint256 swapAmount = 1000 ether;
vm.expectEmit(hook, ReFiSold.selector);
emit ReFiSold(ALICE, swapAmount, 0);
uint24 fee = hook.calculateFee(swapAmount, false);
assertEq(fee, DEFAULT_SELL_FEE, "Sell fee should match default");
}
}

Recommended Mitigation

Implement unified fee calculation logic for both paths
Eliminate unused feeAmount in the sell path unless it’s explicitly applied or emitted.
Saves gas and reduces confus

- remove this code
+ add this code
Updates

Lead Judging Commences

chaossr Lead Judge 12 days ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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

Give us feedback!