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

Missing Fee Validation Allows Excessive Fees

mostafapahlevani93

Description

The normal behavior should validate that fees set by the owner are within reasonable bounds. Uniswap V4 uses basis points where 1,000,000 = 100%, so fees should not exceed this maximum.

The ChangeFee() function allows the owner to set arbitrary fee values without any validation. The owner could accidentally or maliciously set fees exceeding 100% (> 1,000,000 basis points), causing reverts, unexpected behavior, or breaking protocol functionality.

function ChangeFee(
bool _isBuyFee,
uint24 _buyFee,
bool _isSellFee,
uint24 _sellFee
) external onlyOwner {
if(_isBuyFee) buyFee = _buyFee; // ❌ No validation on _buyFee
if(_isSellFee) sellFee = _sellFee; // ❌ No validation on _sellFee
}

Uniswap V4 Fee System:

  • Uses 24-bit unsigned integers (uint24) for fees

  • Maximum value: 16,777,215 (2^24 - 1)

  • Maximum valid fee: 1,000,000 basis points = 100%

  • Fees above 1,000,000 are invalid and break protocol assumptions

Problematic Scenarios:

  1. Owner mistakenly sets fee to 10,000,000 instead of 10,000 (extra zeros)

  2. Owner uses wrong units (percentage instead of basis points)

  3. Owner enters fees in wrong field accidentally

  4. Malicious owner rugpulls by setting 100% fees

Risk

Likelihood:

  • Owner controls fee updates with no restrictions or safeguards

  • Easy to make input errors when dealing with basis points (e.g., adding extra zeros)

  • No warnings or confirmations for extreme fee changes in the contract

  • Owner may not fully understand basis point system

  • Accidental typos can occur during fee updates

  • Frontend may not validate inputs before sending transaction

Impact:

  • Fees exceeding 100% cause undefined behavior in Uniswap V4

  • Protocol functionality breaks completely with invalid fees

  • All swaps may revert if fees are excessive

  • Users lose trust if ridiculous fees are set

  • Emergency situation if fees are set incorrectly and can't be swapped

  • No way to recover from mistake except deploying new pool

  • Liquidity providers cannot remove liquidity if swaps are broken

  • Pool becomes permanently unusable with extreme fees

  • Economic attack vector for malicious owner to extract all value

  • Reputation damage from fee manipulation

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.26;
​
import "forge-std/Test.sol";
import "forge-std/console.sol";
​
contract FeeValidationTest is Test {
uint24 public buyFee;
uint24 public sellFee;
// Uniswap V4 constants
uint24 constant MAX_VALID_FEE = 1000000; // 100% in basis points
uint24 constant MAX_UINT24 = type(uint24).max; // 16,777,215
// Buggy implementation - no validation
function changeFee_BUGGY(
bool _isBuyFee,
uint24 _buyFee,
bool _isSellFee,
uint24 _sellFee
) public {
if (_isBuyFee) buyFee = _buyFee;
if (_isSellFee) sellFee = _sellFee;
}
// Safe implementation - with validation
function changeFee_SAFE(
bool _isBuyFee,
uint24 _buyFee,
bool _isSellFee,
uint24 _sellFee
) public {
if (_isBuyFee) {
require(_buyFee <= MAX_VALID_FEE, "Buy fee exceeds maximum");
buyFee = _buyFee;
}
if (_isSellFee) {
require(_sellFee <= MAX_VALID_FEE, "Sell fee exceeds maximum");
sellFee = _sellFee;
}
}
function test_BuggyAllowsExcessiveFees() public {
console.log("\n=== Testing Buggy Implementation ===");
console.log("Maximum valid fee (100%):", MAX_VALID_FEE);
console.log("Maximum uint24 value:", MAX_UINT24);
// Owner accidentally adds extra zero
uint24 wrongFee = 30000; // Meant to set 3000 (0.3%) but typed 30000 (3%)
changeFee_BUGGY(false, 0, true, wrongFee);
console.log("\nScenario 1: Typo with extra zero");
console.log("Intended fee: 3000 (0.3%)");
console.log("Actual fee set:", sellFee);
console.log("Fee percentage:", uint256(sellFee) * 100 / MAX_VALID_FEE, "%");
assertEq(sellFee, 30000, "Wrong fee accepted");
// Owner uses wrong units (thinks it's percentage not basis points)
uint24 percentageMistake = 30; // Thinks 30%, actually 0.003%
changeFee_BUGGY(false, 0, true, percentageMistake);
console.log("\nScenario 2: Unit confusion");
console.log("Owner thinks: 30%");
console.log("Actual fee set:", sellFee);
console.log("Actual percentage:", uint256(sellFee) * 100 / MAX_VALID_FEE, "%");
// Malicious owner sets 100% fee
changeFee_BUGGY(false, 0, true, MAX_VALID_FEE);
console.log("\nScenario 3: Maximum valid fee");
console.log("Fee set:", sellFee, "(100%)");
assertEq(sellFee, MAX_VALID_FEE, "100% fee allowed");
// Owner sets fee beyond 100%
uint24 excessiveFee = MAX_VALID_FEE + 1;
changeFee_BUGGY(false, 0, true, excessiveFee);
console.log("\nScenario 4: Exceeds 100%");
console.log("Fee set:", sellFee);
console.log("Percentage:", uint256(sellFee) * 100 / MAX_VALID_FEE, "%");
assertGt(sellFee, MAX_VALID_FEE, "Excessive fee accepted");
// Owner sets maximum possible uint24 value
changeFee_BUGGY(false, 0, true, MAX_UINT24);
console.log("\nScenario 5: Maximum uint24 value");
console.log("Fee set:", sellFee);
console.log("Percentage:", uint256(sellFee) * 100 / MAX_VALID_FEE, "%");
assertEq(sellFee, MAX_UINT24, "Maximum uint24 accepted");
console.log("\n❌ BUG CONFIRMED: All excessive fees were accepted without validation!");
}
function test_SafeRejectsExcessiveFees() public {
console.log("\n=== Testing Safe Implementation ===");
// Test fee at boundary (should succeed)
changeFee_SAFE(false, 0, true, MAX_VALID_FEE);
console.log("Boundary test: MAX_VALID_FEE accepted");
assertEq(sellFee, MAX_VALID_FEE, "Boundary fee accepted");
// Test fee beyond boundary (should revert)
console.log("\nTesting fees beyond 100%...");
vm.expectRevert("Sell fee exceeds maximum");
changeFee_SAFE(false, 0, true, MAX_VALID_FEE + 1);
console.log("✅ Correctly rejected fee:", MAX_VALID_FEE + 1);
vm.expectRevert("Sell fee exceeds maximum");
changeFee_SAFE(false, 0, true, MAX_UINT24);
console.log("✅ Correctly rejected max uint24:", MAX_UINT24);
vm.expectRevert("Buy fee exceeds maximum");
changeFee_SAFE(true, MAX_VALID_FEE + 1, false, 0);
console.log("✅ Correctly rejected excessive buy fee");
}
function test_RealWorldScenarios() public {
console.log("\n=== Real-World Scenarios ===");
// Scenario: Owner wants to set 0.3% sell fee
console.log("\nOwner wants: 0.3% sell fee");
console.log("Correct value: 3000 basis points");
// Mistake 1: Typed 30000 instead of 3000
changeFee_BUGGY(false, 0, true, 30000);
console.log("Typo (30000):", sellFee, "basis points =", uint256(sellFee) / 100, "% ❌");
// Mistake 2: Used percentage directly
changeFee_BUGGY(false, 0, true, 3); // Thinks "3%"
console.log("Unit confusion (3):", sellFee, "basis points = 0.003% ❌");
// Correct usage
changeFee_BUGGY(false, 0, true, 3000);
console.log("Correct (3000):", sellFee, "basis points = 0.3% ✅");
console.log("\n=== Potential Exploit Scenario ===");
console.log("Malicious owner sets 99% sell fee to extract value");
changeFee_BUGGY(false, 0, true, 990000);
console.log("Sell fee set to:", sellFee, "basis points");
console.log("Effective fee: 99%");
console.log("Impact: Users selling 1000 tokens receive only 10 tokens");
console.log("LPs receive 990 tokens as fees (potential rugpull vector)");
}
function test_ImpactOnSwaps() public {
console.log("\n=== Impact on Swap Calculations ===");
uint256 swapAmount = 1000 ether;
console.log("Swap amount:", swapAmount / 1 ether, "tokens");
// Normal 0.3% fee
sellFee = 3000;
uint256 normalFeeAmount = (swapAmount * sellFee) / MAX_VALID_FEE;
console.log("\nNormal fee (0.3%):", normalFeeAmount / 1 ether, "tokens");
console.log("User receives:", (swapAmount - normalFeeAmount) / 1 ether, "tokens");
// Excessive 10% fee
sellFee = 100000;
uint256 excessiveFeeAmount = (swapAmount * sellFee) / MAX_VALID_FEE;
console.log("\nExcessive fee (10%):", excessiveFeeAmount / 1 ether, "tokens");
console.log("User receives:", (swapAmount - excessiveFeeAmount) / 1 ether, "tokens");
// Extreme 100% fee
sellFee = MAX_VALID_FEE;
uint256 extremeFeeAmount = (swapAmount * sellFee) / MAX_VALID_FEE;
console.log("\nExtreme fee (100%):", extremeFeeAmount / 1 ether, "tokens");
console.log("User receives:", (swapAmount - extremeFeeAmount) / 1 ether, "tokens (nothing!)");
// Beyond 100% - undefined behavior
sellFee = MAX_VALID_FEE + 500000;
uint256 invalidFeeAmount = (swapAmount * sellFee) / MAX_VALID_FEE;
console.log("\nInvalid fee (>100%):", invalidFeeAmount / 1 ether, "tokens");
console.log("Fee amount exceeds swap amount - undefined behavior!");
}
}

Running the PoC:

forge test --match-contract FeeValidationTest -vv

Expected Output:

[PASS] test_BuggyAllowsExcessiveFees()
=== Testing Buggy Implementation ===
Maximum valid fee (100%): 1000000
Maximum uint24 value: 16777215
Scenario 1: Typo with extra zero
Intended fee: 3000 (0.3%)
Actual fee set: 30000
Fee percentage: 3 %
Scenario 2: Unit confusion
Owner thinks: 30%
Actual fee set: 30
Actual percentage: 0 %
Scenario 3: Maximum valid fee
Fee set: 1000000 (100%)
Scenario 4: Exceeds 100%
Fee set: 1000001
Percentage: 100 %
Scenario 5: Maximum uint24 value
Fee set: 16777215
Percentage: 1677 %
❌ BUG CONFIRMED: All excessive fees were accepted without validation!
​
[PASS] test_SafeRejectsExcessiveFees()
=== Testing Safe Implementation ===
Boundary test: MAX_VALID_FEE accepted
Testing fees beyond 100%...
✅ Correctly rejected fee: 1000001
✅ Correctly rejected max uint24: 16777215
✅ Correctly rejected excessive buy fee

Recommended Mitigation

function ChangeFee(
bool _isBuyFee,
uint24 _buyFee,
bool _isSellFee,
uint24 _sellFee
) external onlyOwner {
+ // Uniswap V4 maximum fee is 100% (1,000,000 basis points)
+ uint24 constant MAX_FEE = 1000000;
+
+ if (_isBuyFee) {
+ require(_buyFee <= MAX_FEE, "Buy fee exceeds 100%");
+ }
+ if (_isSellFee) {
+ require(_sellFee <= MAX_FEE, "Sell fee exceeds 100%");
+ }
+
if(_isBuyFee) buyFee = _buyFee;
if(_isSellFee) sellFee = _sellFee;
}

Enhanced Implementation with Additional Safety:

/// @notice Maximum allowed fee (100% in basis points)
uint24 public constant MAX_FEE = 1000000;
​
/// @notice Reasonable maximum fee for normal operations (10% in basis points)
uint24 public constant REASONABLE_MAX_FEE = 100000;
​
/// @notice Custom error for excessive fees
error FeeExceedsMaximum(uint24 fee, uint24 maximum);
​
/// @notice Updates the buy and/or sell fee percentages
/// @param _isBuyFee Whether to update the buy fee
/// @param _buyFee New buy fee value (if _isBuyFee is true)
/// @param _isSellFee Whether to update the sell fee
/// @param _sellFee New sell fee value (if _isSellFee is true)
/// @dev Only callable by owner. Fees cannot exceed 100% (1,000,000 basis points)
function ChangeFee(
bool _isBuyFee,
uint24 _buyFee,
bool _isSellFee,
uint24 _sellFee
) external onlyOwner {
if (_isBuyFee) {
if (_buyFee > MAX_FEE) {
revert FeeExceedsMaximum(_buyFee, MAX_FEE);
}
buyFee = _buyFee;
emit FeeChanged("buy", _buyFee);
}
if (_isSellFee) {
if (_sellFee > MAX_FEE) {
revert FeeExceedsMaximum(_sellFee, MAX_FEE);
}
sellFee = _sellFee;
emit FeeChanged("sell", _sellFee);
}
}
​
/// @notice Event emitted when fees are changed
event FeeChanged(string feeType, uint24 newFee);

Alternative: Add Warning for Unusual Fees

function ChangeFee(
bool _isBuyFee,
uint24 _buyFee,
bool _isSellFee,
uint24 _sellFee
) external onlyOwner {
if (_isBuyFee) {
require(_buyFee <= MAX_FEE, "Buy fee exceeds maximum (100%)");
if (_buyFee > REASONABLE_MAX_FEE) {
emit UnusualFeeWarning("buy", _buyFee, REASONABLE_MAX_FEE);
}
buyFee = _buyFee;
}
if (_isSellFee) {
require(_sellFee <= MAX_FEE, "Sell fee exceeds maximum (100%)");
if (_sellFee > REASONABLE_MAX_FEE) {
emit UnusualFeeWarning("sell", _sellFee, REASONABLE_MAX_FEE);
}
sellFee = _sellFee;
}
}
​
event UnusualFeeWarning(string feeType, uint24 fee, uint24 reasonableMax);

Testing Recommendations:

Add test cases for:

  1. Setting fee at maximum boundary (1,000,000) - should succeed

  2. Setting fee beyond maximum (1,000,001) - should revert

  3. Setting fee at maximum uint24 (16,777,215) - should revert

  4. Setting normal fees (< 100,000) - should succeed

  5. Attempting to set both fees simultaneously with one invalid - should revert both

Updates

Lead Judging Commences

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