QuantAMM
QuantAMM
49,600 OP
View results
Previous Next
Submission Details
Severity: high
Valid

Wrong quant fee computation in the `onAfterSwap` hook

draiakoo

Summary

Wrong quant fee computation can take more amount that would belong to the owner

Vulnerability Details

The way that the system accounts fees after each swap is the following:

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);
​
...
​
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);
}

It first computes the total fee to extract from the user by multiplying the hookSwapFeePercentage with the amountCalculatedRaw. After that it fetches the percentage that must go to the quant admin. Both the address and the percentage is fetched from the UpdateWeightRunner. The quantAMMFeeTake is a percentage with 18 decimals, for example 70% fee would be 0.7e18. The admin fee computation is done like this:

uint256 adminFee = hookFee / (1e18 / quantAMMFeeTake)

However, this computation is wrong, it should be like this:

uint256 adminFee = hookFee * quantAMMFeeTake / 1e18

To give an example:

hookFee = 5e18
quantAMMFeeTake = 0.55e18

The quant fee is th 55% of the total fee. 55% of 5e18 are 2.75e18 tokens. However, let's see how much it computes:

uint256 adminFee = 5e18 / (1e18 / 0.55e18) = 5e18 / 1 = 5e18

As we can see, it computes that all the fees should be for the admin, when in fact it was only 55%. This demonstrates that the computation is completely broken.
Let's now see if the proposed formula would work:

uint256 adminFee = 5e18 * 0.55e18 / 1e18 = 2.75e36 / 1e18 = 2.75e18

Impact

Medium

Tools Used

Manual review

Recommendations

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);
​
...
​
uint256 quantAMMFeeTake = IUpdateWeightRunner(_updateWeightRunner).getQuantAMMUpliftFeeTake();
uint256 ownerFee = hookFee;
​
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);
}
​
if (ownerFee > 0) {
_vault.sendTo(feeToken, address(this), ownerFee);
​
emit SwapHookFeeCharged(address(this), feeToken, ownerFee);
}
}
}
return (true, hookAdjustedAmountCalculatedRaw);
}
Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_onAfterSwap_adminFee_overestimated_solidity_rounding_down

Likelyhood: High, quantAMMFeeTake is a percentage on calculated fees. Being between 30-70% is very likely. Impact: High, fees for LP providers will be lower than expected and 0 if the admin fees is above 50%.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.