01. Relevant GitHub Links
02. Summary
The UpliftOnlyExample contract has a critical vulnerability where self-transfers of lpnft are not properly handled. If a user transfers an lpnft to their own address, it causes the liquidity record (FeeData) to be overwritten and partially deleted. This results in a permanent loss of liquidity for the user and locks older deposits due to the FILO mechanism. Self-transfers can occur for legitimate reasons, making this a high-severity issue that requires immediate mitigation.
03. Vulnerability Details
The lpnft contract triggers the UpliftOnlyExample:afterUpdate() function whenever a user transfers an lpnft.
function _update(address to, uint256 tokenId, address auth) internal override returns (address previousOwner) {
previousOwner = super._update(to, tokenId, auth);
if (to != address(0) && previousOwner != address(0)) {
router.afterUpdate(previousOwner, to, tokenId);
}
}
Inside afterUpdate(), the user’s feeDataArray is processed. However, there is no proper handling for the case where the from and to addresses are the same (i.e., a self-transfer).
if (tokenIdIndexFound) {
if (_to != address(0)) {
feeDataArray[tokenIdIndex].lpTokenDepositValue = lpTokenDepositValueNow;
feeDataArray[tokenIdIndex].blockTimestampDeposit = uint32(block.number);
feeDataArray[tokenIdIndex].upliftFeeBps = upliftFeeBps;
poolsFeeData[poolAddress][_to].push(feeDataArray[tokenIdIndex]);
if (tokenIdIndex != feeDataArrayLength - 1) {
for (uint i = tokenIdIndex + 1; i < feeDataArrayLength; i++) {
delete feeDataArray[i - 1];
feeDataArray[i - 1] = feeDataArray[i];
}
}
delete feeDataArray[feeDataArrayLength - 1];
feeDataArray.pop();
}
}
If a user accidentally or intentionally performs a self-transfer, the function will push the same FeeData entry to poolsFeeData[poolAddress][_to] and then proceed to delete the last element of the original array, effectively erasing the record of the user’s LP deposit. This results in:
Permanent Loss of Liquidity: The user’s LP deposit in that NFT is effectively removed from their own record.
Locked Older Deposits: Because liquidity removal proceeds in a FILO manner, any older deposit records remain forever inaccessible—locking user funds.
Self-transfers can happen for various legitimate reasons (e.g., to generate blockchain events, to “cancel” a transaction in certain workflows, etc.), so this vulnerability can be triggered inadvertently. When it does happen, the user loses control over their liquidity.
04. Impact
Permanent Liquidity Loss: The LP deposit is erased, and the user can no longer track or withdraw the funds associated with that NFT.
Fund Lock: If the user had older deposits, those remain in the array but cannot be fully withdrawn due to the FILO pattern. Thus, older liquidity deposits may remain locked indefinitely.
High Severity: Even a single self-transfer will result in irrecoverable loss of liquidity records.
05. Proof of Concept
Below is a simplified PoC demonstrating how a self-transfer erases the liquidity record. Copy the code into pkg/pool-hooks/test/foundry/UpliftExample.t.sol and run with forge test --mt test_poc_lp_sefl_transfer -vvv:
function test_poc_lp_sefl_transfer() public {
uint256[] memory maxAmountsIn = [dai.balanceOf(bob), usdc.balanceOf(bob)].toMemoryArray();
vm.prank(bob);
uint256[] memory amountsIn = upliftOnlyRouter.addLiquidityProportional(
pool,
maxAmountsIn,
bptAmount,
false,
bytes("")
);
vm.stopPrank();
console.log("Bob add liquidity");
uint256 expectedTokenId = 1;
console.log("Before deposit length: ", upliftOnlyRouter.getUserPoolFeeData(pool, bob).length);
console.log("Before bptAmount: ", upliftOnlyRouter.getUserPoolFeeData(pool, bob)[0].amount);
console.log("Before timestamp: ", upliftOnlyRouter.getUserPoolFeeData(pool, bob)[0].blockTimestampDeposit);
console.log("Before lpTokenDepositValue: ", upliftOnlyRouter.getUserPoolFeeData(pool, bob)[0].lpTokenDepositValue);
console.log("Before fee: ", upliftOnlyRouter.getUserPoolFeeData(pool, bob)[0].upliftFeeBps);
console.log("Before pool: ", upliftOnlyRouter.nftPool(expectedTokenId));
console.log("Before pool, bptAmount: ", BalancerPoolToken(pool).balanceOf(address(upliftOnlyRouter)));
LPNFT lpNft = upliftOnlyRouter.lpNFT();
vm.prank(bob);
lpNft.transferFrom(bob, bob, expectedTokenId);
console.log("Bob self transfer nft..");
console.log("After deposit length: ", upliftOnlyRouter.getUserPoolFeeData(pool, bob).length);
console.log("After bptAmount: ", upliftOnlyRouter.getUserPoolFeeData(pool, bob)[0].amount);
console.log("After timestamp: ", upliftOnlyRouter.getUserPoolFeeData(pool, bob)[0].blockTimestampDeposit);
console.log("After lpTokenDepositValue: ", upliftOnlyRouter.getUserPoolFeeData(pool, bob)[0].lpTokenDepositValue);
console.log("After fee: ", upliftOnlyRouter.getUserPoolFeeData(pool, bob)[0].upliftFeeBps);
console.log("After pool: ", upliftOnlyRouter.nftPool(expectedTokenId));
console.log("After pool, bptAmount: ", BalancerPoolToken(pool).balanceOf(address(upliftOnlyRouter)));
vm.expectRevert();
vm.prank(bob);
uint256[] memory minAmountsOut = [uint256(0), uint256(0)].toMemoryArray();
upliftOnlyRouter.removeLiquidityProportional(bptAmount, minAmountsOut, false, pool);
console.log("Bob can't remove liquidity");
}
bshyuunn@hyuunn-MacBook-Air pool-hooks % forge test --mt test_poc_lp_sefl_transfer -vvv
[⠘] Compiling...
[⠒] Compiling 17 files with Solc 0.8.26
[⠘] Solc 0.8.26 finished in 132.55s
Ran 1 test for test/foundry/UpliftExample.t.sol:UpliftOnlyExampleTest
[PASS] test_poc_lp_sefl_transfer() (gas: 613360)
Logs:
Bob add liquidity
Before deposit length: 1
Before bptAmount: 2000000000000000000000
Before timestamp: 1682899200
Before lpTokenDepositValue: 500000000000000000
Before fee: 200
Before pool: 0xDB25A7b768311dE128BBDa7B8426c3f9C74f3240
Before pool, bptAmount: 2000000000000000000000
Bob self transfer nft..
After deposit length: 1
After bptAmount: 0
After timestamp: 0
After lpTokenDepositValue: 0
After fee: 0
After pool: 0xDB25A7b768311dE128BBDa7B8426c3f9C74f3240
After pool, bptAmount: 2000000000000000000000
Bob can't remove liquidity
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 39.12ms (3.98ms CPU time)
Ran 1 test suite in 295.61ms (39.12ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
06. Tools Used
Manual Code Review and Foundry
07. Recommended Mitigation
Check for Self-Transfer:
Add a simple conditional in afterUpdate() that reverts or skips the array reordering logic if _from == _to. For example:
+ if (_from == _to) {
+ revert("Self-transfers are not allowed");
+ }
