QuantAMM
QuantAMM
49,600 OP
View results
Previous Next
Submission Details
Severity: low
Invalid

Sender can add liquidity to the pools more than 100 times. Oversight makes the contract vulnerable to DDoS attacks by 1%.

theirrationalone

Summary

The UpliftOnlyExample contract allows the sender to add liquidity to the pools more than 100 times. As it can be seen in the code snippet below, the addLiquidityProportional function of the contract allows the sender to add liquidity to the pools. Indeed the function does check that sender can add liquidity to the pools only limited number of time. The expectation was to limit senders upto 100 times to add liquidity to the pool. However, due to an oversight, the contract allows the sender to add liquidity to the pools more than 100 times at least one more time. This could make the contract vulnerable to DDoS attacks by 1%. The attacker could add liquidity to the pools more than 100 times and disrupt the contract's intended functionality.

Vulnerability Details

see the code snippet below:

UpliftOnlyExample.sol::addLiquidityProportional:

function addLiquidityProportional(
address pool,
uint256[] memory maxAmountsIn,
uint256 exactBptAmountOut,
bool wethIsEth,
bytes memory userData
) external payable saveSender(msg.sender) returns (uint256[] memory amountsIn) {
// @info: using magic number 100
// @info: if a user has already 100 deposits in a pool, then
// then the check below will be skipped and the user will be able to deposit at least 1 more time
// and the user will have 101 deposits in the pool.
// Moreover, the chances of a ddos attack will rise by 1%.
@> if (poolsFeeData[pool][msg.sender].length > 100) {
--------------------------------------------------^
revert TooManyDeposits(pool, msg.sender);
}
// 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;
}

The issue is technical and the code snippet is self-explanatory.

PoC

The following code snippet demonstrates how the contract could be exploited to add liquidity to the pool more than 100 times:

Steps to reproduce:

  1. Go to the test folder inside pool-hooks folder then -> foundry folder -> UpliftExample.t.sol

  2. Add the following test snippet just below the setUp function in the UpliftExample.t.sol file:

function testAddLiquidityBypassDepositLimitByOne() public {
uint256[] memory maxAmountsIn = [dai.balanceOf(bob), usdc.balanceOf(bob)].toMemoryArray();
vm.startPrank(bob);
uint256 bptAmountDeposit = bptAmount / 150;
// 0-100 inclusively is 101 deposits
for (uint256 i = 0; i <= 100; i++) {
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, bptAmountDeposit, false, bytes(""));
skip(1 days);
}
​
// 102nd deposit should fail
vm.expectRevert(abi.encodeWithSelector(UpliftOnlyExample.TooManyDeposits.selector, pool, bob));
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, bptAmountDeposit, false, bytes(""));
​
UpliftOnlyExample.FeeData[] memory poolFeeData = upliftOnlyRouter.getUserPoolFeeData(pool, bob);
​
console.log("bob's poolFeeData length: ", poolFeeData.length);
assertEq(poolFeeData.length, 101, "deposit length incorrect");
vm.stopPrank();
}

  1. Now run the test cases using the following command: (Make sure you are in the pool-hooks directory)

forge test --mt testAddLiquidityBypassDepositLimitByOne -vv

  1. As we can see in the output, the contract allows the sender to add liquidity to the pool more than 100 times.

[⠢] Compiling...
No files changed, compilation skipped
​
Ran 1 test for test/foundry/UpliftExample.t.sol:UpliftOnlyExampleTest
[PASS] testAddLiquidityBypassDepositLimitByOne() (gas: 26795469)
Logs:
bob's poolFeeData length: 101
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 101.53ms (73.71ms CPU time)
​
Ran 1 test suite in 104.48ms (101.53ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

  1. The sender can add liquidity to the pool more than 100 times.

  2. The contract is vulnerable to DDoS attacks by 1%.

  3. A malicious actor who've knowledge to this vulnerability could add liquidity to the pool more than 100 times exactly 101 times and could gain the advantage of 1% DDoS attack in all the pools, that's why it's should be considered as a Medium issue.

  4. Another reason to consider this issue as a Medium issue is that, each time a user adds liquidity to the pool, the contract will mint a new NFT token for the user, and if the user adds liquidity to the pool more than 100 times, the contract will mint more than 100 NFT tokens for the user and afterwards user could transfer the excess NFT token to another user.

  5. Moreover, if that user leaks the oversight to the other users, then there would be chain of corrupt users i.e.,

    • User A adds liquidity to the pool more than 100 times.

    • User A transfers the excess NFT tokens to User B.

    • User B adds liquidity to the pool more than 100 times.

    • User B transfers the excess NFT tokens to User C.

    • User C adds liquidity to the pool more than 100 times.

    • User C transfers the excess NFT tokens to User D.

    • And so on.

Tools Used

  1. Foundry (Framework)

  2. Manual Review

Recommendations

The solution is simple and straightforward. The contract should limit the sender to add liquidity to the pools only 100 times. The following code snippet demonstrates the solution:

function addLiquidityProportional(
address pool,
uint256[] memory maxAmountsIn,
uint256 exactBptAmountOut,
bool wethIsEth,
bytes memory userData
) external payable saveSender(msg.sender) returns (uint256[] memory amountsIn) {
// @info: using magic number 100
- if (poolsFeeData[pool][msg.sender].length > 100) {
+ if (poolsFeeData[pool][msg.sender].length >= 100) {
revert TooManyDeposits(pool, msg.sender);
}
// 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;
}

The above solution limits the sender to add liquidity to the pools only 100 times. Therefore, enhancing the security of the contract.

Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

invalid_Uplift_101_deposit_strict_equal

Only 1 more NFT won’t have any impact. Informational.

Appeal created

theirrationalone Submitter
10 months ago
n0kto Lead Judge
10 months ago
n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

invalid_Uplift_101_deposit_strict_equal

Only 1 more NFT won’t have any impact. Informational.

Support

FAQs

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

Give us feedback!