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

Unrestricted Fee Configuration Allows DoS and 100%+ Fees

ashutoshcshah

Root + Impact

Description

  • The ChangeFee function allows the contract owner to update the buyFee and sellFee state variables. However, the function lacks input validation to ensure the new fee values are within a reasonable or mathematically valid range.

  • The uint24 data type used for fees has a maximum value of 16,777,215. In the context of Uniswap V4 (and most DeFi protocols), fees are typically denominated in pips (where 1,000,000 equals 100%).

function ChangeFee(
bool _isBuyFee,
uint24 _buyFee,
bool _isSellFee,
uint24 _sellFee
) external onlyOwner {
// @> Root Cause: Missing require check ensures _buyFee <= MAX_FEE (e.g. 100%)
@> if(_isBuyFee) buyFee = _buyFee;
// @> Root Cause: Missing require check ensures _sellFee <= MAX_FEE
@> if(_isSellFee) sellFee = _sellFee;
}

Risk

Impact:

  • Denial of Service (DoS): If the fee is set > 100%, the PoolManager or the Hook logic will attempt to deduct more tokens than the user provided. This will cause the transaction to revert due to underflow or insufficient balance errors, rendering the pool unusable

  • Griefing: A malicious or compromised owner key could front-run a large swap by setting the fee to 100%, effectively confiscating the user's entire input for zero output (depending on how the fee revenue is routed).

Proof of Concept

The following Foundry test demonstrates that the owner can set an invalid fee (200%), causing subsequent swaps to revert.

function DOS() public {
// 1. Initialize Pool and Add Liquidity
(PoolKey memory key, ) = initPool(
currency0, currency1, rebateHook,
LPFeeLibrary.DYNAMIC_FEE_FLAG, SQRT_PRICE_1_1
);
// 2. Owner sets Buy Fee to 200% (2,000,000 pips)
// There is no validation to stop this.
uint24 abusiveFee = 2000000;
rebateHook.ChangeFee(true, abusiveFee, false, 0);
​
// 3. User attempts to swap
vm.deal(address(this), 10 ether);
// The swap will fail because the fee exceeds the input amount
vm.expectRevert();
swapRouter.swap{value: 1 ether}(
key,
SwapParams({
zeroForOne: true,
amountSpecified: -1 ether,
sqrtPriceLimitX96: TickMath.MIN_SQRT_PRICE + 1
}),
PoolSwapTest.TestSettings({takeClaims: false, settleUsingBurn: false}),
ZERO_BYTES
);
}

Recommended Mitigation

Introduce a constant representing the maximum allowable fee (e.g., 10% or 100,000 pips) and require that any new fee set via ChangeFee falls within this limit.

+ uint24 public constant MAX_FEE = 100000; // 10% Max Fee
​
function ChangeFee(
bool _isBuyFee,
uint24 _buyFee,
bool _isSellFee,
uint24 _sellFee
) external onlyOwner {
if(_isBuyFee) {
+ require(_buyFee <= MAX_FEE, "Fee exceeds maximum limit");
buyFee = _buyFee;
}
if(_isSellFee) {
+ require(_sellFee <= MAX_FEE, "Fee exceeds maximum limit");
sellFee = _sellFee;
}
+ emit FeeChanged(_isBuyFee, _buyFee, _isSellFee, _sellFee);
}
Updates

Lead Judging Commences

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