Summary
Every transfer of LPNFT will set blockTimestampDeposit to incorrect block timestamp value due to the code in
https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol#L610
function afterUpdate(address _from, address _to, uint256 _tokenID) public {
feeDataArray[tokenIdIndex].blockTimestampDeposit = uint32(block.number);
}
Vulnerability Details
When user addLiquidityProportional, a LPNFT is minted for the user and user deposit data is stored in storage array
https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol#L250-L260
function addLiquidityProportional(
address pool,
uint256[] memory maxAmountsIn,
uint256 exactBptAmountOut,
bool wethIsEth,
bytes memory userData
) external payable saveSender(msg.sender) returns (uint256[] memory amountsIn) {
poolsFeeData[pool][msg.sender].push(
FeeData({
tokenID: tokenID,
amount: exactBptAmountOut,
lpTokenDepositValue: depositValue,
blockTimestampDeposit: uint40(block.timestamp),
upliftFeeBps: upliftFeeBps
})
);
}
The poolsFeeData[pool][msg.sender] is an array to keep track of deposit of users and the contract consider this as FILO (First In Last Out).
When Bob transfer the LPNFT tokenID_B to Alice, the tokenID_B is inserted into the last element of poolsFeeData[pool][Alice]
As the variable name blockTimestampDeposit suggests, this should be the block.timestamp of the deposit.
but in afterUpdate, the blockTimestampDeposit is updated to be the block.number
https://github.com/Cyfrin/2024-12-quantamm/blob/main/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol#L610
function afterUpdate(address _from, address _to, uint256 _tokenID) public {
feeDataArray[tokenIdIndex].blockTimestampDeposit = uint32(block.number);
}
So after the transfer of the LPNFT token, the blockTimestampDeposit is replaced with block.number.
So when user or some offchain service call getUserPoolFeeData() to get the data of the tokenID, then the blockTimestampDeposit is wrong and caused inconsistency.
Impact
Impact: Low
Reason: The data return by getUserPoolFeeData() for the tokenID for blockTimestampDeposit is incorrect and inconsitent
Likelyhood: High
It's highly to happen. It happenned with every transfer of LPNFT token
=> Total Impact: Medium
Reference: https://docs.codehawks.com/hawks-auditors/how-to-evaluate-a-finding-severity
Tools Used
Foundry test case
Proof of Concept
Step 0: Setup Precondition that Bob has 1 LPNFT
Step 1: Bob transfer the LPNFT to Alice
Get the getUserPoolFeeData of Alice and check the blockTimestampDeposit.
Now the blockTimestampDeposit is equal to block.number
Add the testTransferLPNFT below into 2024-12-quantamm\pkg\pool-hooks\test\foundry\UpliftExample.t.sol
function testTransferLPNFT() public {
BaseVaultTest.Balances memory balancesBefore = getBalances(bob);
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();
uint256 expectedTokenId = 1;
assertEq(upliftOnlyRouter.getUserPoolFeeData(pool, bob).length, 1, "deposit length incorrect");
assertEq(upliftOnlyRouter.getUserPoolFeeData(pool, bob)[0].amount, bptAmount, "bptAmount incorrect");
uint256 timestamp = uint256(upliftOnlyRouter.getUserPoolFeeData(pool, bob)[0].blockTimestampDeposit);
assertEq(
timestamp,
uint256(block.timestamp),
"blockTimestampDeposit incorrect"
);
console.log("blockTimestampDeposit ", timestamp);
console.log("block.number ", block.number );
vm.roll(100);
console.log("Roll to block.number 100");
console.log("block.number ", block.number );
address Alice = address(0x112233);
console.log("current timestamp ", block.timestamp );
LPNFT lpNft = upliftOnlyRouter.lpNFT();
vm.prank(bob);
lpNft.transferFrom(bob, Alice, expectedTokenId);
vm.stopPrank();
timestamp = uint256(upliftOnlyRouter.getUserPoolFeeData(pool, Alice)[0].blockTimestampDeposit);
console.log("After transfer blockTimestampDeposit ", timestamp);
vm.stopPrank();
}
forge test --match-test testTransferLPNFT -vv
Test Log:
Ran 1 test for test/foundry/UpliftExample.t.sol:UpliftOnlyExampleTest
[PASS] testTransferLPNFT() (gas: 645692)
Logs:
blockTimestampDeposit 1682899200
block.number 1
Roll to block.number 100
block.number 100
current timestamp 1682899200
After transfer blockTimestampDeposit 100
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 25.63ms (1.54ms CPU time)
Recommendations
Change the code in afterUpdate to:
function afterUpdate(address _from, address _to, uint256 _tokenID) public {
feeDataArray[tokenIdIndex].blockTimestampDeposit = uint40(block.timestamp);
}
