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

The value of `blockTimestampDeposit` is incorrectly reset to `uint32(block.number)` in `afterUpdate`

biakia

Summary

The vulnerability present in the code is a logical error due to the mismatch between the expected and actual data being stored in the variable blockTimestampDeposit. Although the variable blockTimestampDeposit is not used now, it can lead to unexpected side effects in future.

Vulnerability Details

In UpliftOnlyExample , the variable blockTimestampDeposit in struct FeeData is intended to store the timestamp of when liquidity was added, which should be represented by block.timestamp:

/// @notice The fee data for a given owner and deposit
struct FeeData {
uint256 tokenID;
uint256 amount;
uint256 lpTokenDepositValue;
uint40 blockTimestampDeposit;
uint64 upliftFeeBps;
}
function addLiquidityProportional(
address pool,
uint256[] memory maxAmountsIn,
uint256 exactBptAmountOut,
bool wethIsEth,
bytes memory userData
) external payable saveSender(msg.sender) returns (uint256[] memory amountsIn) {
if (poolsFeeData[pool][msg.sender].length > 100) {
revert TooManyDeposits(pool, msg.sender);
}
// Do addLiquidity operation - BPT is minted to this contract.
amountsIn = _addLiquidityProportional(
pool,
msg.sender,
address(this),
maxAmountsIn,
exactBptAmountOut,
wethIsEth,
userData
);
​
uint256 tokenID = lpNFT.mint(msg.sender);
​
//this requires the pool to be registered with the QuantAMM update weight runner
//as well as approved with oracles that provide the prices
uint256 depositValue = getPoolLPTokenValue(
IUpdateWeightRunner(_updateWeightRunner).getData(pool),
pool,
MULDIRECTION.MULDOWN
);
​
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;
}

However, later in the afterUpdate function, the value of blockTimestampDeposit is incorrectly reset to uint32(block.number), which is the current block number instead of the timestamp:

feeDataArray[tokenIdIndex].lpTokenDepositValue = lpTokenDepositValueNow;
feeDataArray[tokenIdIndex].blockTimestampDeposit = uint32(block.number);
feeDataArray[tokenIdIndex].upliftFeeBps = upliftFeeBps;

This logical inconsistency can lead to confusion regarding the actual time that liquidity was added and can cause issues in any functionality dependent on blockTimestampDeposit.

In conclusion, the logical error of resetting blockTimestampDeposit to uint32(block.number) not only misrepresents the time of liquidity addition but can have far-reaching implications affecting contract functionality and usage, resulting in exploitable vulnerabilities in dependent logic.

Impact

The impact is LOW, the likelihood is HIGH, so the severity is MEDIUM.

Tools Used

Manual Review

Recommendations

Consider following fix:

feeDataArray[tokenIdIndex].lpTokenDepositValue = lpTokenDepositValueNow;
feeDataArray[tokenIdIndex].blockTimestampDeposit = uint40(block.timestamp);
feeDataArray[tokenIdIndex].upliftFeeBps = upliftFeeBps;
Updates

Lead Judging Commences

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