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

The attacker can bypass the 100 deposits limit to dos any user

biakia

Summary

In UpliftOnlyExample contract, the addLiquidityProportional function enforces a limit on the number of deposits (restricting users to a maximum of 100) to avoid dos issues, however, the afterUpdate function lacks this critical check when transferring NFTs, which can lead to the count exceeding the intended limit.

Vulnerability Details

The addLiquidityProportional function includes a check to ensure that a user cannot have more than 100 deposits to avoid dos issues:

/**
* @notice To avoid Ddos issues, a single depositor can only deposit 100 times
* @param pool The pool the depositor is attempting to deposit to
* @param depositor The address of the depositor
*/
error TooManyDeposits(address pool, address depositor);
​
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);
}

In the afterUpdate function, when an NFT is transferred from one user to another, the associated FeeData is pushed into the poolsFeeData array of the recipient (_to):

// Update the deposit value to the current value of the pool in base currency (e.g. USD) and the block index to the current block number
//vault.transferLPTokens(_from, _to, feeDataArray[i].amount);
feeDataArray[tokenIdIndex].lpTokenDepositValue = lpTokenDepositValueNow;
feeDataArray[tokenIdIndex].blockTimestampDeposit = uint32(block.number);
feeDataArray[tokenIdIndex].upliftFeeBps = upliftFeeBps;
​
//actual transfer not a afterTokenTransfer caused by a burn
poolsFeeData[poolAddress][_to].push(feeDataArray[tokenIdIndex]);

However, there is no check to ensure that the recipient does not already have 100 deposits. This could allow a user to accumulate more than 100 deposits by receiving NFTs from others, bypassing the limit enforced in addLiquidityProportional.

The issue arises from the ability to transfer LP tokens to a user without adequately capping the potential size of the poolsFeeData array. The afterUpdate function allows the transfer of NFTs associated with liquidity positions, but there are no checks in place to limit the number of FeeData entries that can be accumulated for each user.

When a user receives a large number of LP tokens (potentially by multiple transfers), they can accumulate corresponding FeeData entries, thereby increasing the poolsFeeData array size indefinitely. This can pose a problem when the user attempts to remove liquidity through the onAfterRemoveLiquidity function. The operation reads the feeDataArray to calculate accrued fees based on past deposits, and if the array has grown too large, it may exceed the block gas limit during execution, leading to transaction failure.

Impact

The impact is HIGH because it will bypass the deposit limit and cause dos issues and the likelihood is HIGH because it is easy to apply, so the severity is HIGH.

Tools Used

Manual Review

Recommendations

To fix this issue, you should add a check in the afterUpdate function to ensure that the recipient (_to) does not already have 100 deposits before pushing the FeeData into their array. Here’s how you can modify the afterUpdate function:

// Ensure the recipient does not already have 100 deposits
if (poolsFeeData[poolAddress][_to].length >= 100) {
revert TooManyDeposits(poolAddress, _to);
}
//actual transfer not a afterTokenTransfer caused by a burn
poolsFeeData[poolAddress][_to].push(feeDataArray[tokenIdIndex]);
Updates

Lead Judging Commences

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

finding_afterUpdate_does_not_check_limit_NFT_per_user

Likelihood: Medium/High, anyone can receive an unlimited NFT number but will cost creation of LP tokens and sending them. Impact: Low/Medium, DoS the afterUpdate and addLiquidityProportional but will be mitigable on-chain because a lot of those NFT can be burn easily in onAfterRemoveLiquidity.

Support

FAQs

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