Summary
The onAfterSwap function in the UpliftOnlyExample contract incorrectly calculates the adminFee by dividing hookFee using 1e18 / quantAMMFeeTake. This calculation causes precision loss in scenarios involving small or large values for quantAMMFeeTake, leading to incorrect distribution of fees between the admin and owner.
Vulnerability Details
-
Issue:
The division in (1e18 / quantAMMFeeTake) introduces rounding errors when quantAMMFeeTake is a small fraction or very large. This results in an inaccurate calculation of both adminFee and ownerFee, leading to discrepancies in the fees allocated.
-
Root Cause:
Division operations with large or small numbers in Solidity can cause loss of precision due to truncation in integer arithmetic.
UpliftOnlyExample.sol Github link
contract UpliftOnlyExample is MinimalRouter, BaseHooks, Ownable {
...
uint64 private constant _MIN_SWAP_FEE_PERCENTAGE = 0.001e16;
uint64 private constant _MAX_SWAP_FEE_PERCENTAGE = 10e16;
...
function setHookSwapFeePercentage(uint64 hookFeePercentage) external onlyOwner {
require(hookFeePercentage >= _MIN_SWAP_FEE_PERCENTAGE, "Below _MIN_SWAP_FEE_PERCENTAGE");
require(hookFeePercentage <= _MAX_SWAP_FEE_PERCENTAGE, "Above _MAX_SWAP_FEE_PERCENTAGE");
hookSwapFeePercentage = hookFeePercentage;
emit HookSwapFeePercentageChanged(address(this), hookFeePercentage);
}
...
function onAfterSwap(
AfterSwapParams calldata params
) public override onlyVault returns (bool success, uint256 hookAdjustedAmountCalculatedRaw) {
hookAdjustedAmountCalculatedRaw = params.amountCalculatedRaw;
if (hookSwapFeePercentage > 0) {
uint256 hookFee = params.amountCalculatedRaw.mulUp(hookSwapFeePercentage);
if (hookFee > 0) {
IERC20 feeToken;
if (params.kind == SwapKind.EXACT_IN) {
feeToken = params.tokenOut;
hookAdjustedAmountCalculatedRaw -= hookFee;
} else {
feeToken = params.tokenIn;
hookAdjustedAmountCalculatedRaw += hookFee;
}
uint256 quantAMMFeeTake = IUpdateWeightRunner(_updateWeightRunner).getQuantAMMUpliftFeeTake();
uint256 ownerFee = hookFee;
if (quantAMMFeeTake > 0) {
@@ uint256 adminFee = hookFee / (1e18 / quantAMMFeeTake);
ownerFee = hookFee - adminFee;
address quantAMMAdmin = IUpdateWeightRunner(_updateWeightRunner).getQuantAMMAdmin();
_vault.sendTo(feeToken, quantAMMAdmin, adminFee);
emit SwapHookFeeCharged(quantAMMAdmin, feeToken, adminFee);
}
if (ownerFee > 0) {
_vault.sendTo(feeToken, address(this), ownerFee);
emit SwapHookFeeCharged(address(this), feeToken, ownerFee);
}
}
}
return (true, hookAdjustedAmountCalculatedRaw);
}
}
UpdateWeightRunner.sol Github link
contract UpdateWeightRunner is Ownable2Step, IUpdateWeightRunner {
...
function setQuantAMMUpliftFeeTake(uint256 _quantAMMUpliftFeeTake) external{
require(msg.sender == quantammAdmin, "ONLYADMIN");
require(_quantAMMUpliftFeeTake <= 1e18, "Uplift fee must be less than 100%");
uint256 oldSwapFee = quantAMMSwapFeeTake;
quantAMMSwapFeeTake = _quantAMMUpliftFeeTake;
emit UpliftFeeTakeSet(oldSwapFee, _quantAMMUpliftFeeTake);
}
function getQuantAMMUpliftFeeTake() external view returns (uint256){
return quantAMMSwapFeeTake;
}
}
Impact
-
Incorrect Fee Distribution:
The admin and owner receive inaccurate portions of the fees, potentially leading to financial inconsistencies.
-
Trust Issues:
A misallocation of fees could erode trust in the protocol's fee calculation mechanisms.
-
Precision Loss:
The discrepancy can grow significant in edge cases with extreme values for quantAMMFeeTake.
Tools Used
Proof of Concept (POC)
pkg\pool-hooks\test\foundry\UpliftExample.t.sol
function testFeeSwapExactIn_Has_quantAMMFeeTake__Fuzz(
uint256 swapAmount,
uint64 hookFeePercentage,
uint256 quantAMMUpliftFeeTak
) public {
swapAmount = bound(swapAmount, POOL_MINIMUM_TOTAL_SUPPLY, poolInitAmount);
hookFeePercentage = uint64(bound(hookFeePercentage, _MIN_SWAP_FEE_PERCENTAGE, _MAX_SWAP_FEE_PERCENTAGE));
vm.expectEmit();
emit UpliftOnlyExample.HookSwapFeePercentageChanged(poolHooksContract, hookFeePercentage);
vm.assume(quantAMMUpliftFeeTak != 0);
quantAMMUpliftFeeTak = uint256(bound(quantAMMUpliftFeeTak, 0, 1e18));
vm.prank(owner);
UpliftOnlyExample(payable(poolHooksContract)).setHookSwapFeePercentage(hookFeePercentage);
vm.prank(address(vaultAdmin));
updateWeightRunner.setQuantAMMUpliftFeeTake(quantAMMUpliftFeeTak);
vm.stopPrank();
uint256 hookFee = swapAmount.mulUp(hookFeePercentage);
BaseVaultTest.Balances memory balancesBefore = getBalances(bob);
if (hookFee > 0) {
uint256 quantAMMFeeTake = updateWeightRunner.getQuantAMMUpliftFeeTake();
console.log("hookFee: ", hookFee);
console.log("quantAMMFeeTake: ", quantAMMFeeTake);
if (quantAMMFeeTake > 0) {
uint256 adminFee_old_formula = hookFee / (1e18 / quantAMMFeeTake);
uint256 adminFee_new_formula = (hookFee * quantAMMFeeTake) / 1e18;
uint256 ownerFee_old_formula = hookFee - adminFee_old_formula;
uint256 ownerFee_new_formula = hookFee - adminFee_new_formula;
console.log("adminFee_old_formula: ", adminFee_old_formula);
console.log("adminFee_new_formula: ", adminFee_new_formula);
console.log("ownerFee_old_formula: ", ownerFee_old_formula);
console.log("ownerFee_new_formula: ", ownerFee_new_formula);
assertEq(adminFee_old_formula, adminFee_new_formula);
}
}
}
Run the test using Foundry:
forge test --mt "testFeeSwapExactIn_Has_quantAMMFeeTake__Fuzz" -vv
Test Result:
Ran 1 test for test/foundry/UpliftExample.t.sol:UpliftOnlyExampleTest
[FAIL: assertion failed: 6190697509195062332 != 6016070015519375489; counterexample: calldata=0xe2b2fe6500000000000000000000009eba3a035bc839a503bdafe21cb84116eca6d9131e000000000000000000000000000000000000000000000000000000a6bd0c2fed00000000000000000000000000000000000003aae1a33ff66dbb8fcf31189b8a args=[231980424717446108095889811563625021149466649498398 [2.319e50], 716136263661 [7.161e11], 74385847922553243022373800156042 [7.438e31]]] testFeeSwapExactIn_Has_quantAMMFeeTake__Fuzz(uint256,uint64,uint256) (runs: 0, μ: 0, ~: 0)
Logs:
hookFee: 24762790036780249328
quantAMMFeeTake: 242947987952233489
adminFee_old_formula: 6190697509195062332
adminFee_new_formula: 6016070015519375489
ownerFee_old_formula: 18572092527585186996
ownerFee_new_formula: 18746720021260873839
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 73.55ms (11.59ms CPU time)
Recommendations
Update Fee Calculation Logic:
Replace the existing division formula with a multiplication-based calculation to eliminate precision loss:
if (quantAMMFeeTake > 0) {
- uint256 adminFee = hookFee / (1e18 / quantAMMFeeTake);
+ uint256 adminFee = (hookFee * quantAMMFeeTake) / 1e18;
ownerFee = hookFee - adminFee;
address quantAMMAdmin = IUpdateWeightRunner(_updateWeightRunner).getQuantAMMAdmin();
_vault.sendTo(feeToken, quantAMMAdmin, adminFee);
emit SwapHookFeeCharged(quantAMMAdmin, feeToken, adminFee);
}
