Submission Details
Severity:
User cannot remove liquidity from Vault.
Summary
User cannot remove liquidity from Vault.
https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol#L431-L570
Vulnerability Details
User cannot add liquidity with the amount smaller than _MINIMUM_TRADE_AMOUNT.
We can see this fact from following code snippet.
File: Vault.sol
function _ensureValidTradeAmount(uint256 tradeAmount) internal view {
if (tradeAmount != 0) {
_ensureValidSwapAmount(tradeAmount);
}
}
function _ensureValidSwapAmount(uint256 tradeAmount) internal view {
if (tradeAmount < _MINIMUM_TRADE_AMOUNT) {
revert TradeAmountTooSmall();
}
}
But UpliftOnlyExample.sol#onAfterRemoveLiquidity() function does not consider this aspect.
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) {
address userAddress = address(bytes20(userData));
AfterRemoveLiquidityData memory localData = AfterRemoveLiquidityData({
pool: pool,
bptAmountIn: bptAmountIn,
amountsOutRaw: amountsOutRaw,
minAmountsOut: new uint256[](amountsOutRaw.length),
accruedFees: new uint256[](amountsOutRaw.length),
accruedQuantAMMFees: new uint256[](amountsOutRaw.length),
currentFee: minWithdrawalFeeBps,
feeAmount: 0,
prices: IUpdateWeightRunner(_updateWeightRunner).getData(pool),
lpTokenDepositValueNow: 0,
lpTokenDepositValueChange: 0,
lpTokenDepositValue: 0,
tokens: new IERC20[](amountsOutRaw.length),
feeDataArrayLength: 0,
amountLeft: 0,
feePercentage: 0,
adminFeePercent: 0
});
// We only allow removeLiquidity via the Router/Hook itself so that fee is applied correctly.
hookAdjustedAmountsOutRaw = amountsOutRaw;
//this rounding faxvours the LP
localData.lpTokenDepositValueNow = getPoolLPTokenValue(localData.prices, pool, MULDIRECTION.MULDOWN);
FeeData[] storage feeDataArray = poolsFeeData[pool][userAddress];
localData.feeDataArrayLength = feeDataArray.length;
localData.amountLeft = bptAmountIn;
for (uint256 i = localData.feeDataArrayLength - 1; i >= 0; --i) {
localData.lpTokenDepositValue = feeDataArray[i].lpTokenDepositValue;
localData.lpTokenDepositValueChange =
(int256(localData.lpTokenDepositValueNow) - int256(localData.lpTokenDepositValue)) /
int256(localData.lpTokenDepositValue);
uint256 feePerLP;
// if the pool has increased in value since the deposit, the fee is calculated based on the deposit value
if (localData.lpTokenDepositValueChange > 0) {
feePerLP =
(uint256(localData.lpTokenDepositValueChange) * (uint256(feeDataArray[i].upliftFeeBps) * 1e18)) /
10000;
}
// if the pool has decreased in value since the deposit, the fee is calculated based on the base value - see wp
else {
//in most cases this should be a normal swap fee amount.
//there always myst be at least the swap fee amount to avoid deposit/withdraw attack surgace.
feePerLP = (uint256(minWithdrawalFeeBps) * 1e18) / 10000;
}
// if the deposit is less than the amount left to burn, burn the whole deposit and move on to the next
if (feeDataArray[i].amount <= localData.amountLeft) {
uint256 depositAmount = feeDataArray[i].amount;
localData.feeAmount += (depositAmount * feePerLP);
localData.amountLeft -= feeDataArray[i].amount;
lpNFT.burn(feeDataArray[i].tokenID);
delete feeDataArray[i];
feeDataArray.pop();
if (localData.amountLeft == 0) {
break;
}
} else {
feeDataArray[i].amount -= localData.amountLeft;
localData.feeAmount += (feePerLP * localData.amountLeft);
break;
}
}
localData.feePercentage = (localData.feeAmount) / bptAmountIn;
hookAdjustedAmountsOutRaw = localData.amountsOutRaw;
localData.tokens = _vault.getPoolTokens(localData.pool);
localData.adminFeePercent = IUpdateWeightRunner(_updateWeightRunner).getQuantAMMUpliftFeeTake();
// Charge fees proportional to the `amountOut` of each token.
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, // @audit: this is minted bpt amount when proportional kind.
kind: AddLiquidityKind.PROPORTIONAL,
userData: bytes("")
})
);
emit ExitFeeCharged(
userAddress,
localData.pool,
IERC20(localData.pool),
localData.feeAmount.mulDown(localData.adminFeePercent) / 1e18
);
}
...
return (true, hookAdjustedAmountsOutRaw);
}
As we can see above, there is no check minBptAmount which is the bpt amount minted when kind - AddLiquidityKind.PROPORTIONAL.
File: Vault.sol
Function: _addLiquidity
if (params.kind == AddLiquidityKind.PROPORTIONAL) {
631@> bptAmountOut = params.minBptAmountOut;
// Initializes the swapFeeAmounts empty array (no swap fees on proportional add liquidity).
swapFeeAmounts = new uint256[](locals.numTokens);
amountsInScaled18 = BasePoolMath.computeProportionalAmountsIn(
poolData.balancesLiveScaled18,
_totalSupply(params.pool),
bptAmountOut
);
}
Impact
So in boundary case, user's legitimate call is reverted.
Tools Used
Manual review
Recommendations
Modify UpliftOnlyExample's logic not to add liquidity to admin when minBptAmount < _MINIMUM_TRADE_AMOUNT.
Updates
Lead Judging Commences
n0kto 10 months ago
Submission Judgement Published Assigned finding tags:
finding_onAfterRemoveLiquidity_not_enough_fee_revert
Likelihood: Medium, when fees are below the _MINIMUM_TRADE_AMOUNT. Impact: Low/Medium, DoS of withdrawal but could be mitigated adding funds or waiting for more benefits.
Prize pool breakdown
Total prize
49,600 OP
nSLOC
3,00017
OP / LOC
44,640
4,960
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.