Submission Details
Impact:
Likelihood:
No Bounds Validation on Fee Updates
Root + Impact
The ChangeFee function accepts any uint24 value without bounds checking, allowing fees up to 1677% which would break swaps or cause massive unexpected costs.
Description
-
Uniswap V4 has a maximum fee of 1,000,000 (100%).
-
The function allows values up to
type(uint24).max(16,777,215) without validation.
// Root cause in RebateFiHook.sol lines 84-92
function ChangeFee(
bool _isBuyFee,
uint24 _buyFee,
bool _isSellFee,
uint24 _sellFee
) external onlyOwner {
@> if(_isBuyFee) buyFee = _buyFee; // @> No validation
@> if(_isSellFee) sellFee = _sellFee; // @> No validation
}
Risk
Likelihood:
-
Requires owner mistake or compromise
-
Single transaction can set catastrophic fees
Impact:
-
Fees >100% would make all swaps fail or drain users
-
No recovery mechanism without another owner transaction
Proof of Concept
The following test demonstrates that the owner can set arbitrarily high fees without any validation. An accidental typo or malicious owner could set fees that would drain users or break swaps entirely.
// SPDX-License-Identifier: MIT
import {Test, console} from "forge-std/Test.sol";
import {ReFiSwapRebateHook} from "../src/RebateFiHook.sol";
import {Deployers} from "@uniswap/v4-core/test/utils/Deployers.sol";
import {MockERC20} from "solmate/src/test/utils/mocks/MockERC20.sol";
contract UnboundedFeePoCTest is Test, Deployers {
ReFiSwapRebateHook public hook;
MockERC20 public reFiToken;
address owner;
function setUp() public {
deployFreshManagerAndRouters();
reFiToken = new MockERC20("ReFi", "REFI", 18);
hook = new ReFiSwapRebateHook(manager, address(reFiToken));
owner = hook.owner();
}
function test_PoC_CanSet100PercentFee() public {
// Uniswap V4 max fee is 1,000,000 (100%)
uint24 maxUniswapFee = 1_000_000;
(uint24 buyFeeBefore, uint24 sellFeeBefore) = hook.getFeeConfig();
console.log("=== Before ===");
console.log("Buy Fee:", buyFeeBefore);
console.log("Sell Fee:", sellFeeBefore);
// Owner sets 100% fee - this should be rejected but isn't
vm.prank(owner);
hook.ChangeFee(true, maxUniswapFee, true, maxUniswapFee);
(uint24 buyFeeAfter, uint24 sellFeeAfter) = hook.getFeeConfig();
console.log("");
console.log("=== After Setting 100% Fee ===");
console.log("Buy Fee:", buyFeeAfter, "(100%)");
console.log("Sell Fee:", sellFeeAfter, "(100%)");
console.log("");
console.log("DANGER: Users would lose ALL tokens in fees!");
assertEq(buyFeeAfter, maxUniswapFee, "100% buy fee accepted");
assertEq(sellFeeAfter, maxUniswapFee, "100% sell fee accepted");
}
function test_PoC_CanSetExtremelyHighFee() public {
// uint24 max is 16,777,215 (1677% fee!)
uint24 extremeFee = type(uint24).max;
console.log("=== Setting Extreme Fee ===");
console.log("Fee value:", extremeFee);
console.log("As percentage:", extremeFee / 10000, "%"); // 1677%
// This succeeds - no validation!
vm.prank(owner);
hook.ChangeFee(false, 0, true, extremeFee);
(, uint24 sellFee) = hook.getFeeConfig();
console.log("");
console.log("Sell Fee set to:", sellFee);
console.log("This is", sellFee / 1_000_000, "x (", sellFee * 100 / 1_000_000, "%) the swap amount");
console.log("");
console.log("Result: Swaps would fail or drain users completely");
assertEq(sellFee, extremeFee, "Extreme fee accepted without validation");
}
function test_PoC_AccidentalTypo() public {
// Owner wants 0.5% (5000), but accidentally adds extra zero
uint24 intendedFee = 5000; // 0.5%
uint24 accidentalFee = 500000; // 50% (typo!)
console.log("=== Accidental Typo Scenario ===");
console.log("Owner intends to set:", intendedFee, "(0.5%)");
console.log("Owner accidentally types:", accidentalFee, "(50%)");
vm.prank(owner);
hook.ChangeFee(false, 0, true, accidentalFee);
(, uint24 sellFee) = hook.getFeeConfig();
console.log("");
console.log("Fee actually set:", sellFee);
console.log("Users now pay 50% fee instead of 0.5%");
console.log("100x more than intended!");
console.log("");
console.log("With MAX_FEE validation, this would revert.");
assertEq(sellFee, accidentalFee, "No safeguard against typos");
}
function test_PoC_NoEventEmittedOnChange() public {
console.log("=== No Event Emitted ===");
console.log("Fee changes are invisible to off-chain monitoring");
// No event to expect - ChangeFee doesn't emit any
vm.prank(owner);
hook.ChangeFee(true, 10000, true, 20000);
console.log("Fee changed from 0/3000 to 10000/20000");
console.log("But no event was emitted!");
console.log("Users cannot be notified through standard monitoring.");
}
}
Recommended Mitigation
Add maximum fee validation in the method ChangeFee to mitigate the issue
+ uint24 public constant MAX_FEE = 100000; // 10% maximum
+ error FeeTooHigh();
function ChangeFee(
bool _isBuyFee,
uint24 _buyFee,
bool _isSellFee,
uint24 _sellFee
) external onlyOwner {
+ if (_isBuyFee && _buyFee > MAX_FEE) revert FeeTooHigh();
+ if (_isSellFee && _sellFee > MAX_FEE) revert FeeTooHigh();
if(_isBuyFee) buyFee = _buyFee;
if(_isSellFee) sellFee = _sellFee;
+ emit FeeChanged(buyFee, sellFee);
}
Rewards breakdown
nSLOC
150This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.