Summary
In the UpliftOnlyExample contract, block.timestamp is used to track liquidity additions and is recorded in the FeeData struct's state variable poolsFeeData, a mapping keyed by the pool address and owner address. While the potential caveats of using block.timestamp are well-understood and handled with care, an oversight exists in the UpliftOnlyExample::afterUpdate function. This function is responsible for tracking and updating data when transferring LPNFT. Instead of block.timestamp, block.number is used for the FeeData member feeDataArray[tokenIdIndex].blockTimestampDeposit. This design choice complicates the tracking of data, particularly in monitoring and updating intervals for liquidity deposits and transfers in FeeData.
Vulnerability Details
Refer to the code snippet below. The block.timestamp is utilized in the addLiquidityProportional function to track liquidity deposits.
UpliftOnlyExample::addLiquidityProportional:
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);
}
amountsIn = _addLiquidityProportional(
pool, msg.sender, address(this), maxAmountsIn, exactBptAmountOut, wethIsEth, userData
);
uint256 tokenID = lpNFT.mint(msg.sender);
uint256 depositValue =
getPoolLPTokenValue(IUpdateWeightRunner(_updateWeightRunner).getData(pool), pool, MULDIRECTION.MULDOWN);
poolsFeeData[pool][msg.sender].push(
FeeData({
tokenID: tokenID,
amount: exactBptAmountOut,
lpTokenDepositValue: depositValue,
@>
@> blockTimestampDeposit: uint40(block.timestamp),
upliftFeeBps: upliftFeeBps
})
);
nftPool[tokenID] = pool;
}
As mentioned above, the comment indicates that all the caveats associated with using timestamps are well-understood.
Now, let’s take a closer look at updating or transferring liquidity (LPNFT)...
UpliftOnlyExample::afterUpdate:
function afterUpdate(address _from, address _to, uint256 _tokenID) public {
if (msg.sender != address(lpNFT)) {
revert TransferUpdateNonNft(_from, _to, msg.sender, _tokenID);
}
address poolAddress = nftPool[_tokenID];
if (poolAddress == address(0)) {
revert TransferUpdateTokenIDInvaid(_from, _to, _tokenID);
}
int256[] memory prices = IUpdateWeightRunner(_updateWeightRunner).getData(poolAddress);
uint256 lpTokenDepositValueNow = getPoolLPTokenValue(prices, poolAddress, MULDIRECTION.MULDOWN);
FeeData[] storage feeDataArray = poolsFeeData[poolAddress][_from];
uint256 feeDataArrayLength = feeDataArray.length;
uint256 tokenIdIndex;
bool tokenIdIndexFound = false;
for (uint256 i; i < feeDataArrayLength; ++i) {
if (feeDataArray[i].tokenID == _tokenID) {
tokenIdIndex = i;
tokenIdIndexFound = true;
break;
}
}
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 (uint256 i = tokenIdIndex + 1; i < feeDataArrayLength; i++) {
delete feeDataArray[i - 1];
feeDataArray[i - 1] = feeDataArray[i];
}
}
delete feeDataArray[feeDataArrayLength - 1];
feeDataArray.pop();
}
}
}
read the comments above indicated by @>.
PoC
UpliftOnlyExample::addLiquidityProportional:
poolsFeeData[pool][msg.sender].push(
FeeData({
tokenID: tokenID,
amount: exactBptAmountOut,
lpTokenDepositValue: depositValue,
@>
@> blockTimestampDeposit: uint40(block.timestamp),
upliftFeeBps: upliftFeeBps
})
);
UpliftOnlyExample::afterUpdate:
@>
@> feeDataArray[tokenIdIndex].blockTimestampDeposit = uint32(block.number);
Impact
Loss of Tracking: Unable to effectively track LPNFT transfers.
Lack of Interval Clarity: No clear record of liquidity addition time intervals.
Unpredictable User Actions: Inability to determine when users transferred their LPNFTs.
Unrecorded Activity: User actions may go undocumented.
Strategic Challenges: Hinders the development of effective strategies.
Non-Linear Progression: Block numbers do not correspond linearly to time, complicating time estimation.
Difficulty Bomb Impact: Sudden jumps in block numbers due to the upcoming difficulty bomb.
Hard Fork Disruptions: Major network upgrades could reset block numbers.
Network Splits: Block numbers may become discontinuous across chains during network splits.
Tools Used
Manual Review
Phind (AI Buddy)
Recommendations
The mitigation for this issue is straightforward. Since the protocol is already aware of the prominent caveats associated with block.timestamp, we should use block.timestamp instead of block.number. The corrected code is provided below.
function afterUpdate(address _from, address _to, uint256 _tokenID) public {
if (msg.sender != address(lpNFT)) {
revert TransferUpdateNonNft(_from, _to, msg.sender, _tokenID);
}
address poolAddress = nftPool[_tokenID];
if (poolAddress == address(0)) {
revert TransferUpdateTokenIDInvaid(_from, _to, _tokenID);
}
int256[] memory prices = IUpdateWeightRunner(_updateWeightRunner).getData(poolAddress);
uint256 lpTokenDepositValueNow = getPoolLPTokenValue(prices, poolAddress, MULDIRECTION.MULDOWN);
FeeData[] storage feeDataArray = poolsFeeData[poolAddress][_from];
uint256 feeDataArrayLength = feeDataArray.length;
uint256 tokenIdIndex;
bool tokenIdIndexFound = false;
//find the tokenID index in the array
for (uint256 i; i < feeDataArrayLength; ++i) {
if (feeDataArray[i].tokenID == _tokenID) {
tokenIdIndex = i;
tokenIdIndexFound = true;
break;
}
}
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 = uint40(block.timestamp);
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 (uint256 i = tokenIdIndex + 1; i < feeDataArrayLength; i++) {
delete feeDataArray[i - 1];
feeDataArray[i - 1] = feeDataArray[i];
}
}
delete feeDataArray[feeDataArrayLength - 1];
feeDataArray.pop();
}
}
}
