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

The `feeDataArray[tokenIdIndex].blockTimestampDeposit` is recored wrongly when transfer nft.

danielarmstrong

Summary

The feeDataArray[tokenIdIndex].blockTimestampDeposit is recored wrongly when transfer nft.
https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol#L610

Vulnerability Details

UpliftOnlyExample.sol#afterUpdate() function is as follows.

function afterUpdate(address _from, address _to, uint256 _tokenID) public {
...
​
if (tokenIdIndexFound) {
if (_to != address(0)) {
// Update the deposit value to the current value of the pool in base currency (e.g. USD) and the block index to the current block number
//vault.transferLPTokens(_from, _to, feeDataArray[i].amount);
feeDataArray[tokenIdIndex].lpTokenDepositValue = lpTokenDepositValueNow;
@> feeDataArray[tokenIdIndex].blockTimestampDeposit = uint32(block.number);
feeDataArray[tokenIdIndex].upliftFeeBps = upliftFeeBps;
​
//actual transfer not a afterTokenTransfer caused by a burn
poolsFeeData[poolAddress][_to].push(feeDataArray[tokenIdIndex]);
​
if (tokenIdIndex != feeDataArrayLength - 1) {
//Reordering the entire array could be expensive but it is the only way to keep true FILO
for (uint i = tokenIdIndex + 1; i < feeDataArrayLength; i++) {
delete feeDataArray[i - 1];
feeDataArray[i - 1] = feeDataArray[i];
}
}
​
delete feeDataArray[feeDataArrayLength - 1];
feeDataArray.pop();
}
}
}

As we can see above, feeDataArray[tokenIdIndex].blockTimestampDeposit is recorded with block.number.
But UpliftOnlyExample.sol#addLiquidityProportional() function is as follows.

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,
//this rounding favours the LP
lpTokenDepositValue: depositValue,
//known use of timestamp, caveats are known.
@> blockTimestampDeposit: uint40(block.timestamp),
upliftFeeBps: upliftFeeBps
})
);
​
nftPool[tokenID] = pool;
}

This is contradict.

Impact

Users and protocol will suffer from wrong blockTimestampDeposit.

Tools Used

Manual review

Recommendations

Modify UpliftOnlyExample.sol#afterUpdate() as follows.

function afterUpdate(address _from, address _to, uint256 _tokenID) public {
...
​
if (tokenIdIndexFound) {
if (_to != address(0)) {
// Update the deposit value to the current value of the pool in base currency (e.g. USD) and the block index to the current block number
//vault.transferLPTokens(_from, _to, feeDataArray[i].amount);
feeDataArray[tokenIdIndex].lpTokenDepositValue = lpTokenDepositValueNow;
-- feeDataArray[tokenIdIndex].blockTimestampDeposit = uint32(block.number);
++ feeDataArray[tokenIdIndex].blockTimestampDeposit = uint32(block.timestamp);
feeDataArray[tokenIdIndex].upliftFeeBps = upliftFeeBps;
​
//actual transfer not a afterTokenTransfer caused by a burn
...
}
}
}
Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_afterUpdate_blockNumber_instead_of_timestamp

Likelihood: Medium/High, any NFT transfer will change this variable. Impact: Informational/Very Low. This variable is unused and won’t impact anything, but the array is public and its getter will return a variable with inconsistencies.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.