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

Removal of liquidity can fail because of the addition of fees does not meet the `minBptAmountOut`

draiakoo

Summary

Rounding errors can lead to DoS of the removal of liquidity

Vulnerability Details

When someone wants to remove liquidity, the protocol accounts a certain amount of fees to the quant admin. These fees are accounted by adding liquidity using the percentage of fee in each token and expecting the same percentage of minimum BPT out:

function onAfterRemoveLiquidity(
address router,
address pool,
RemoveLiquidityKind,
uint256 bptAmountIn,
uint256[] memory,
uint256[] memory amountsOutRaw,
uint256[] memory,
bytes memory userData
) public override onlySelfRouter(router) returns (bool, uint256[] memory hookAdjustedAmountsOutRaw) {
...
​
for (uint256 i = 0; i < localData.amountsOutRaw.length; i++) {
uint256 exitFee = localData.amountsOutRaw[i].mulDown(localData.feePercentage);
​
if (localData.adminFeePercent > 0) {
localData.accruedQuantAMMFees[i] = exitFee.mulDown(localData.adminFeePercent);
}
​
localData.accruedFees[i] = exitFee - localData.accruedQuantAMMFees[i];
hookAdjustedAmountsOutRaw[i] -= exitFee;
// Fees don't need to be transferred to the hook, because donation will redeposit them in the Vault.
// In effect, we will transfer a reduced amount of tokensOut to the caller, and leave the remainder
// in the pool balance.
}
​
if (localData.adminFeePercent > 0) {
_vault.addLiquidity(
AddLiquidityParams({
pool: localData.pool,
to: IUpdateWeightRunner(_updateWeightRunner).getQuantAMMAdmin(),
maxAmountsIn: localData.accruedQuantAMMFees,
minBptAmountOut: localData.feeAmount.mulDown(localData.adminFeePercent) / 1e18,
kind: AddLiquidityKind.PROPORTIONAL,
userData: bytes("")
})
);
emit ExitFeeCharged(
userAddress,
localData.pool,
IERC20(localData.pool),
localData.feeAmount.mulDown(localData.adminFeePercent) / 1e18
);
}
...
}

When it adds this liquidity it passes as maxAmountsIn the accruedQuantAMMFees array that are the amounts of each token that represented the fee. Also this addition expects a minBptAmountOut of the percentage of fee extracted from the total BPT. This can be really problematic because rounding errors in token amount computations can yield to not meet the minBptAmountOut. So if this happens, the user will be unable to remove any of his liquidity.
For example, if the pool require that to receive 1 BPT it needs 10e18 tokenA and 20e18 tokenB, if the computations of the fee tokens yield to provide 10e18 and 19.999999999999999999 of tokenB due to rounding error, the return would be something like 0.999999999999999999 BPT. This would not meet the 1 BPT and the addition of liquidity would revert.
Since this addition of liquidity is done in between a transaction, the minBptAmountOut could be set to 0 and it would have no risk of being sandwitched. This would prevent the addition of liquidity from reverting because of the minBptAmountOut not met.

Impact

Medium, the removal of liquidity would revert

Tools Used

Manual review

Recommendations

Setting the minBptAmountOut would be the best solution to this:

function onAfterRemoveLiquidity(
address router,
address pool,
RemoveLiquidityKind,
uint256 bptAmountIn,
uint256[] memory,
uint256[] memory amountsOutRaw,
uint256[] memory,
bytes memory userData
) public override onlySelfRouter(router) returns (bool, uint256[] memory hookAdjustedAmountsOutRaw) {
...
if (localData.adminFeePercent > 0) {
_vault.addLiquidity(
AddLiquidityParams({
pool: localData.pool,
to: IUpdateWeightRunner(_updateWeightRunner).getQuantAMMAdmin(),
maxAmountsIn: localData.accruedQuantAMMFees,
-- minBptAmountOut: localData.feeAmount.mulDown(localData.adminFeePercent) / 1e18,
++ minBptAmountOut: 0,
kind: AddLiquidityKind.PROPORTIONAL,
userData: bytes("")
})
);
emit ExitFeeCharged(
userAddress,
localData.pool,
IERC20(localData.pool),
localData.feeAmount.mulDown(localData.adminFeePercent) / 1e18
);
}
...
}
Updates

Lead Judging Commences

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

finding_onAfterRemoveLiquidity_vault.addLiquidity_rounding_precision_DoS

Likelihood: High, multiple rounding down and little value can trigger the bug Impact: High, DoS the removal.

Support

FAQs

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