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

Transferring deposit NFT doesn't check if the receiver exceeds the 100 deposit limit

jsondoge

Summary

Transferring doesn't check if the receiver already has 100 deposits and allows exceeding it without limit.

Vulnerability Details

When transferring deposit NFT the functionality doesn't check if the receiver already has reached the limit. Thus allowing for a user address to potentially have unlimited amount of deposits.

Impact

When deposits are unlimited it can create a partial-DOS. If a user has many small amount deposits sent to him he will not be able to withdraw all amounts before removing smaller ones due to gas limits. And also may experience DOS while sending away deposits due to iteration running out of gas while trying to find the tokenID:

for (uint256 i; i < feeDataArrayLength; ++i) {
if (feeDataArray[i].tokenID == _tokenID) {
tokenIdIndex = i;
tokenIdIndexFound = true;
break;
}
}



Because there are no condition requirements this can be abused by malicious actors using dust amount deposit transfers and DOS'ing users (especially on cheaper gas chains - Arbitrum, Base), but since nothing really is gained apart from disruption - Low/Medium.

Tools Used

Manual review + foundry test

function testNftTransferAllowsExceedingDepositLimit() public {
// Add liquidity so bob has BPT to remove liquidity.
uint256[] memory maxAmountsIn = [dai.balanceOf(bob), usdc.balanceOf(bob)].toMemoryArray();
uint256 input = 100;
​
vm.prank(alice);
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, input, false, bytes(""));
​
// Current limitation is max limitation is inaccurate and allows up to 101 deposits
for (uint256 i = 0; i < 101; ++i) {
vm.prank(bob);
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, input, false, bytes(""));
}
​
LPNFT nft = LPNFT(upliftOnlyRouter.lpNFT());
vm.prank(alice);
nft.transferFrom(address(alice), address(bob), 1);
​
// exceeds the 101 limit imposed by the router
assertEq(upliftOnlyRouter.getUserPoolFeeData(pool, bob).length, 102);
}

Recommendations

Add this check

if (poolsFeeData[poolAddress][_to].length >= 100) {
revert TooManyDeposits(poolAddress, _to);
}

to router function afterUpdate(address _from, address _to, uint256 _tokenID) public

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.

Appeal created

bshyuunn Auditor
7 months ago
0xhuntoor Auditor
7 months ago
n0kto Lead Judge
7 months ago
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.