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

lpNFT transfer to self can cause token owner to lose their balance.

inh3l

Summary

Users who transfer the lpNFT to themselves stand the risk of losing their entire balance which can lead to fund loss and accounting issues.

Vulnerability Details

lpNFT's _update function doesn't check that the from and to parameters are different. Neither that the base ERC721's transferFrom and safeTransferFrom functions. As a result, a user can successfully transfer their lpNFT to themselves. However, in doing this, the user stands the risk of losing the FeeData.amount associated with the lpNFT which is the expected amountOut for the user when liquidity is added.

To prove this, the following test can be added to UpliftExample.sol and ran. It shows that the bob loses their balance when he transfers the lpNFT to himself.

function testTransferToSelf(
uint256,
uint256 depositLength
)
public {
uint256 depositBound = bound(depositLength, 1, 10);
​
uint256[] memory maxAmountsIn = [
dai.balanceOf(bob),
usdc.balanceOf(bob)
].toMemoryArray();
​
vm.startPrank(bob);
​
uint256 bptAmountDeposit = bptAmount / 150;
​
uint256[] memory tokenIndexArray = new uint256[]();
​
for (uint256 i = 0; i < depositBound; i++) {
tokenIndexArray[i] = i + 1;
upliftOnlyRouter.addLiquidityProportional(
pool,
maxAmountsIn,
bptAmountDeposit,
false,
bytes("")
);
skip(1 days);
}
​
LPNFT lpNft = upliftOnlyRouter.lpNFT();
​
UpliftOnlyExample.FeeData[] memory bobFees = upliftOnlyRouter.getUserPoolFeeData(pool, bob);
​
uint256 nftTokenId = bobFees[bobFees.length - 1].tokenID;
uint256 amt = bobFees[bobFees.length - 1].amount;
​
lpNft.transferFrom(bob, bob, nftTokenId);
​
UpliftOnlyExample.FeeData[] memory bobFees2 = upliftOnlyRouter.getUserPoolFeeData(pool, bob);
​
uint256 amt2 = bobFees2[bobFees2.length - 1].amount;
​
assertNotEq(amt, amt2);
assertEq(amt2, 0);
}

Impact

Loss of feeData amount

Tools Used

Manual Review

Recommendations

Revert if from == to when transferring lpNFT.

Updates

Lead Judging Commences

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

finding_afterUpdate_erase_self_transfer

Likelihood: Low, when users wants to transfer tokens they already own to themselves. Impact: High, funds loss.

Support

FAQs

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