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

Permanent Loss of User BPT Shares due to Self Transfer Vulnerability.

0xwsecteam

Summary

This issue has a critical impact on users' BPT (deposit shares), as their BPT amount can be reset to 0 if they accidentally transfer a token to themselves.
This transfer cause the deletion poolsFeeData index which contains (BPT amount, deposited Value, etc..) and permanently removes all BPT tied to it.
This results in a irreversible loss of the user's BPT amount, leaving no way for recovery or remediation.

Vulnerability Details

In the QuantAMM system, users can transfer their tokens, but there is a serious issue if a user accidentally transfers a token to themselves.
When this happens, the token gets deleted from the poolsFeeData array, which holds all the important data about the user’s deposits and balances.
This means the user loses all the BPT (their deposit shares) tied to that token ID, and it is reset to 0.

See below PoC there is more explenation

PoC: add this to https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol

function testLoseAllBPTs() public {
uint256[] memory maxAmountsInBob = [dai.balanceOf(bob), dai.balanceOf(bob)].toMemoryArray();
uint256[] memory minAmountsOut = [uint256(0), uint256(0)].toMemoryArray();
​
LPNFT lpNft = upliftOnlyRouter.lpNFT();
uint256 bptAmountDepositBob = 1_000_000 ether;
​
// Bob deposit 2 times.
vm.startPrank(bob);
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsInBob, bptAmountDepositBob, false, bytes(""));
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsInBob, bptAmountDepositBob * 2, false, bytes(""));
vm.stopPrank();
​
UpliftOnlyExample.FeeData[] memory bobFees = upliftOnlyRouter.getUserPoolFeeData(pool, bob);
​
// check that feeData array has lenght of 2 tokenID:1 tokenID: 2
assertEq(bobFees.length, 2);
​
// Check deposit informations.
for(uint256 i; i < bobFees.length; i++) {
console.log("TokenID :", bobFees[i].tokenID);
console.log("BPT Amount :", bobFees[i].amount);
console.log("deposit Value:", bobFees[i].lpTokenDepositValue);
console.log("timestamp :", bobFees[i].blockTimestampDeposit);
console.log("upliftFeeBps :", bobFees[i].upliftFeeBps);
console.log("");
}
​
// bob transfer token by mistake to himself.
vm.prank(bob);
lpNft.transferFrom(bob, bob, 1 /* transfer tokenID 1 */);
​
bobFees = upliftOnlyRouter.getUserPoolFeeData(pool, bob);
​
console.log("After Bob transfer tokenID 1\n");
​
// Check deposit informations.
for(uint256 i; i < bobFees.length; i++) {
console.log("TokenID :", bobFees[i].tokenID);
console.log("BPT Amount :", bobFees[i].amount);
console.log("deposit Value:", bobFees[i].lpTokenDepositValue);
console.log("timestamp :", bobFees[i].blockTimestampDeposit);
console.log("upliftFeeBps :", bobFees[i].upliftFeeBps);
console.log("");
}
}

Result:

Logs:
TokenID : 1
BPT Amount : 1000000000000000000000000
deposit Value: 500000000000000000
timestamp : 1682899200
upliftFeeBps : 200
TokenID : 2
BPT Amount : 2000000000000000000000000
deposit Value: 500000000000000000
timestamp : 1682899200
upliftFeeBps : 200
After Bob transfer tokenID 1
​
TokenID : 2
BPT Amount : 2000000000000000000000000
deposit Value: 500000000000000000
timestamp : 1682899200
upliftFeeBps : 200
TokenID : 0
BPT Amount : 0
deposit Value: 0
timestamp : 0
upliftFeeBps : 0
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 33.45ms (2.46ms CPU time)

This is Step by Step how Transfer works with FILO system in QuantAMM:

Action poolsFeeData Before poolsFeeData After Self Transfer
Self Transfer {tokenID: 1, BPT: 1M}, {tokenID: 2, BPT: 2M}, tokenID 2 shifted to left
{tokenID: 2, BPT: 2M} {tokenID: 0, BPT: 0} data is poped and lost

Bob want to transfer token he calls transfer/transferFrom then LPNFT contract override _update function
and it is transfer it calls afterUpdate which is the responsible of transfering BPT amount and deposit value etc...
and then it push feeDataArray[tokenIdIndex] to new owner (_to) so _to poolsFeeData array will increase and then FILO is applied.

original bob's dataArray:

feeDataArray = [
{tokenID: 1, BPT: 1_000_000},
{tokenID: 2, BPT: 2_000_000},
]

After poolsFeeData[_to].push(feeDataArray[tokenIdIndex]);:

feeDataArray = [
{tokenID: 1, BPT: 1_000_000},
{tokenID: 2, BPT: 2_000_000},
{tokenID: 1, BPT: 1_000_000}, // Pushed duplicate entry
]

After FILO Loop Execution:

  1. The loop starts shifting entries to the left:

    • {tokenID: 2, BPT: 2_000_000} moves to index 0.

    • {tokenID: 1, BPT: 1_000_000} (duplicate) moves to index 1.

    Result:

    feeDataArray = [
    {tokenID: 2, BPT: 2_000_000},
    {tokenID: 1, BPT: 1_000_000},
    {tokenID: 1, BPT: 1_000_000}, // Duplicate at the end
    ]

  2. After loop ends it deletes and pops;

Notice that feeDataArrayLength is 2 because it was stored before transfered tokenID 1 is pushed to \_to address:

delete feeDataArray[feeDataArrayLength - 1]; // this deletes index number 1.
feeDataArray.pop(); // this pops index number 2

Result:

feeDataArray = [
{tokenID: 2, BPT: 2_000_000}, // index number 0.
{tokenID: 0, BPT: 0}, // index number 1.
]

As we see second index entry is reset to 00 and all BPT, deposit value, timestamp, tokenID are deleted and reset to 0.

Impact

  1. Users lose all associated BPT tokens for the token ID, which cannot be recovered due to the deletion of the poolsFeeData.

  2. Once the poolsFeeData and associated BPT are deleted, there is no mechanism to revert the operation or recover the funds.

impact: High likelihood: low ~= Severity: Medium

Recommendations

File: https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-hooks/contracts/hooks-quantamm/LPNFT.sol#L49-L56

Add this in the override _update to prevent self transfer by mistake:

function _update(address to, uint256 tokenId, address auth) internal override returns (address previousOwner) {
+ require(_from != _to, "self transfers are not allowed");
previousOwner = super._update(to, tokenId, auth);
//_update is called during mint, burn and transfer. This functionality is only for transfer
if (to != address(0) && previousOwner != address(0)) {
//if transfering the record in the vault needs to be changed to reflect the change in ownership
router.afterUpdate(previousOwner, to, tokenId);
}
}
Updates

Lead Judging Commences

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