RebateFi Hook

First Flight #53
Beginner FriendlyDeFi
100 EXP
View results
Submission Details
Impact: high
Likelihood: medium
Invalid

Unbounded dynamic fees

The ReFiSwapRebateHook allows the owner to set the 100% fee, that will consume the entire swap amount.

Description

  • The ReFiSwapRebateHook allows the owner to set the sellFee == 100%, that will consume the entire swap amount. The user loses the entire swap amount.

// Root cause in the codebase with @> marks to highlight the relevant section
function ChangeFee(bool _isBuyFee, uint24 _buyFee, bool _isSellFee, uint24 _sellFee) external onlyOwner {
@> if (_isBuyFee) buyFee = _buyFee;
@> if (_isSellFee) sellFee = _sellFee;
}

Risk

Likelihood:

  • The owner may accidentally or intentionally set the commission at 100%, causing users to lose the entire transaction amount.

Impact:

  • The user сomplete loss of funds in a single transaction.

Proof of Concept

1) Set extreme sell fee == 100%;
2) Make swap (sell);
3) Looking for the ReFiSold(address indexed seller, uint256 amount, uint256 fee) event in the logs and, if the event is found, get params amount and fee ;
4) Print params:
swapAmount - swap amount,
sellFee - swap fee,
amountOut - the number of tokens received by the user as a result of the swap.

In the reusult you will see, that sellFee > swapAmount and the number of tokens received by the user as a result of the swap == 0

function test_PoC_UnboundedSellFee() public {
// set extreme sell fee
uint24 extremeSellFee = 1_000_000; // 100%
rebateHook.ChangeFee(false, 0, true, extremeSellFee);
// Check it was updated
(, uint24 currentSellFee) = rebateHook.getFeeConfig();
assertEq(currentSellFee, extremeSellFee, "Sell fee should be updated to extreme value");
uint256 reFiAmount = 0.001 ether;
vm.startPrank(user1);
reFiToken.approve(address(swapRouter), type(uint256).max);
vm.recordLogs();
// Make sell
SwapParams memory params = SwapParams({
zeroForOne: true, amountSpecified: -int256(reFiAmount), sqrtPriceLimitX96: TickMath.MIN_SQRT_PRICE + 1
});
PoolSwapTest.TestSettings memory testSettings =
PoolSwapTest.TestSettings({takeClaims: false, settleUsingBurn: false});
vm.deal(address(swapRouter), 10 ether);
BalanceDelta delta = swapRouter.swap(key, params, testSettings, ZERO_BYTES);
vm.stopPrank();
// Looking for the ReFiSold event in the logs
Vm.Log[] memory entries = vm.getRecordedLogs();
bytes32 topic = keccak256("ReFiSold(address,uint256,uint256)");
bool found = false;
uint256 eventAmount;
uint256 eventFee;
for (uint256 i = 0; i < entries.length; i++) {
if (entries[i].topics[0] == topic && entries[i].topics[0] == topic) {
(eventAmount, eventFee) = abi.decode(entries[i].data, (uint256, uint256));
found = true;
break;
}
}
require(found, "ReFiSold event not found");
int128 amountOut = params.zeroForOne ? delta.amount1() : delta.amount0();
// Should be print: sellFee > swapAmount and amountOut == 0
console.log("swapAmount =\t", eventAmount);
console.log("sellFee =\t", eventFee);
console.log("amountOut =\t", amountOut);
// Assert that sellFee > swapAmount and amountOut == 0
assertGt(eventFee, eventAmount, "Fee should exceed swap amount with extreme sellFee");
assertEq(amountOut, 0, "No output should be received with 100% fee");
}

Recommended Mitigation

Limit the hook to the safest possible fee range, for example ≤ 50% depending on the tokenomics, and check the limit inside ReFiSwapRebateHook::ChangeFee().

+uint24 constant MAX_FEE = 500_000; // 50%
function ChangeFee(bool _isBuyFee, uint24 _buyFee, bool _isSellFee, uint24 _sellFee) external onlyOwner {
- if (_isBuyFee) buyFee = _buyFee;
+ if (_isBuyFee) {
+ require(_buyFee <= MAX_FEE, "BuyFeeTooHigh");
+ buyFee = _buyFee;
+ }
- if (_isSellFee) sellFee = _sellFee;
+ if (_isSellFee) {
+ require(_sellFee <= MAX_FEE, "SellFeeTooHigh");
+ sellFee = _sellFee;
+ }
}
Updates

Lead Judging Commences

chaossr Lead Judge 11 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!