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

Missing Fee Validation Allows Pool Bricking via Invalid Fee Setting

muzammilmohammed678

Root + Impact

Description

  • Describe the normal behavior in one or more sentences

  • Explain the specific issue or problem in one or more sentences

The `ChangeFee` function does not validate that fees are within acceptable bounds, allowing the owner to set fees that exceed the maximum allowed by Uniswap V4, potentially causing swaps to fail or unexpected behavior.
​
**Description**:
* The normal behavior expects fee values to be validated against the maximum allowed LP fee (1,000,000 representing 100%) before being set.
* The specific issue is that `ChangeFee` accepts any `uint24` value without validation, which could lead to fees exceeding `MAX_LP_FEE` (1,000,000), causing the pool to reject swaps or behave unexpectedly.
​
**Root cause in the codebase**:
```solidity
// @> src/RebateFiHook.sol:84-92
function ChangeFee(
bool _isBuyFee,
uint24 _buyFee,
bool _isSellFee,
uint24 _sellFee
) external onlyOwner {
if(_isBuyFee) buyFee = _buyFee;
if(_isSellFee) sellFee = _sellFee;
}
```
​
No validation is performed to ensure `_buyFee` and `_sellFee` are <= `MAX_LP_FEE` (1,000,000) as required by `LPFeeLibrary.validate()`.
​

Risk

Likelihood:

  • * This occurs when the owner calls `ChangeFee` with invalid fee values

    * Accidental misconfiguration or malicious owner action can trigger this

Impact:

  • * Swaps will revert when fees exceed the maximum, breaking pool functionality

    * The hook's `_beforeSwap` will return invalid fees that cause `LPFeeLibrary.removeOverrideFlagAndValidate()` to revert

    * Pool becomes unusable until fees are corrected

Proof of Concept

The following demonstrates how an invalid fee can brick the pool:
​
```solidity
// Step 1: Owner accidentally or maliciously sets invalid fee
// MAX_LP_FEE = 1,000,000 (represents 100%)
// Setting 2,000,000 exceeds the maximum allowed
​
rebateHook.ChangeFee(false, 0, true, 2000000); // 200% fee (invalid, exceeds MAX_LP_FEE)
​
// Step 2: User attempts to swap
SwapParams memory params = SwapParams({
zeroForOne: false, // Selling ReFi
amountSpecified: -int256(1 ether),
sqrtPriceLimitX96: TickMath.MAX_SQRT_PRICE - 1
});
​
// Step 3: Hook's _beforeSwap() returns invalid fee
function _beforeSwap(...) returns (..., uint24) {
uint24 fee = sellFee; // fee = 2000000
return (..., fee | LPFeeLibrary.OVERRIDE_FEE_FLAG);
// Returns: 2000000 | 0x400000 = invalid fee with override flag
}
​
// Step 4: Pool.swap() calls LPFeeLibrary.removeOverrideFlagAndValidate()
// In Pool.sol:
uint24 lpFee = params.lpFeeOverride.removeOverrideFlagAndValidate();
// removeOverrideFlag(): 2000000 & 0xBFFFFF = 2000000
// validate(): if (2000000 > MAX_LP_FEE) revert LPFeeTooLarge(2000000)
// MAX_LP_FEE = 1,000,000
// 2000000 > 1000000 = true
// Result: Transaction reverts with LPFeeTooLarge(2000000)
​
// Step 5: All subsequent swaps fail
// Every swap attempt reverts at validation
// Pool becomes completely unusable until owner fixes the fee
```
​
**Step-by-step execution:**
1. Owner calls `ChangeFee(false, 0, true, 2000000)` - no validation occurs
2. `sellFee` is set to `2000000` (exceeds `MAX_LP_FEE = 1000000`)
3. User attempts a swap, triggering `_beforeSwap()`
4. Hook returns fee `2000000 | OVERRIDE_FEE_FLAG`
5. Pool's `swap()` calls `removeOverrideFlagAndValidate(2000000)`
6. Validation checks: `2000000 > 1000000` → true
7. Reverts with `LPFeeTooLarge(2000000)`
8. All swaps fail, pool is permanently bricked until fee is corrected

Recommended Mitigation

Add validation before setting fees using `LPFeeLibrary.validate()` to ensure fees don't exceed `MAX_LP_FEE` (1,000,000). This prevents invalid fees from being stored and protects the pool from being bricked.

```diff

// src/RebateFiHook.sol:23

using LPFeeLibrary for uint24;

// src/RebateFiHook.sol:84-92

function ChangeFee(

bool _isBuyFee,

uint24 _buyFee,

bool _isSellFee,

uint24 _sellFee

) external onlyOwner {

+ if(_isBuyFee) {

+ _buyFee.validate(); // Reverts with LPFeeTooLarge if > MAX_LP_FEE

+ }

+ if(_isSellFee) {

+ _sellFee.validate(); // Reverts with LPFeeTooLarge if > MAX_LP_FEE

+ }

if(_isBuyFee) buyFee = _buyFee;

if(_isSellFee) sellFee = _sellFee;

}

```

**Explanation:**

The fix validates fees before state changes using `LPFeeLibrary.validate()`, which checks if fees are <= `MAX_LP_FEE` (1,000,000). Invalid fees revert with `LPFeeTooLarge(fee)` before being stored, preventing pool bricking. This ensures fees always comply with Uniswap V4's requirements and are caught at configuration time rather than during swaps.

Updates

Lead Judging Commences

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