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

An unintended self-transfer of an NFT will result in a DoS, preventing the user from removing liquidity.

TheKhans

Summary

A user can perform a uninteded self-transfer of their liquidity-position NFT, leading to a state inconsistency that prevents them from subsequently withdrawing their liquidity.

Vulnerability Details

If a user mistakenly transfers the NFT to themselves, they will not be able to remove liquidity. This happens because the initial addition and removal of FeeData in afterUpdate deletes their record from the array but does not pop the array entry, leading to an inconsistency while processing the liquidity removal.

function testSelfTransfer() public {
uint256[] memory maxAmountsIn = [uint256(2e18), uint256(2e18)].toMemoryArray();
vm.startPrank(bob);
uint256 bptAmountDeposit = 2e18;
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, bptAmountDeposit, false, bytes(""));
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, bptAmountDeposit, false, bytes(""));
vm.stopPrank();
​
LPNFT lpNft = upliftOnlyRouter.lpNFT();
​
vm.startPrank(bob);
lpNft.transferFrom(bob, bob, 1);
vm.stopPrank();
​
uint256[] memory minAmountsOut = [uint256(0), uint256(0)].toMemoryArray();
​
vm.expectRevert();
vm.startPrank(bob);
upliftOnlyRouter.removeLiquidityProportional(bptAmountDeposit, minAmountsOut, false, pool);
vm.stopPrank();
​
vm.expectRevert();
vm.startPrank(bob);
upliftOnlyRouter.removeLiquidityProportional(bptAmountDeposit, minAmountsOut, false, pool);
vm.stopPrank();
}

Impact

create DoS Preventing User from Removing Liquidity.

Tools Used

Manual Review

Recommendations

  1. Add a check to revert or disallow transfers if the from and to addresses are same

diff --git a/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol b/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol
index 66c0a5e..7da60d0 100644
--- a/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol
+++ b/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol
@@ -189,6 +189,8 @@ contract UpliftOnlyExample is MinimalRouter, BaseHooks, Ownable {
error TransferUpdateTokenIDInvaid(address from, address to, uint256 tokenId);
error ZeroAmountProvided(uint256 amount);
+
+ error SelfTransfer();
modifier onlySelfRouter(address router) {
_ensureSelfRouter(router);
@@ -585,7 +587,9 @@ contract UpliftOnlyExample is MinimalRouter, BaseHooks, Ownable {
if (msg.sender != address(lpNFT)) {
revert TransferUpdateNonNft(_from, _to, msg.sender, _tokenID);
}
-
+ if (_from == _to){
+ revert SelfTransfer();
+ }

  1. If self-transfers must be allowed, ensure the contract logic correctly updates ownership and internal state even when the NFT is transferred to the same address.

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.

Give us feedback!