QuantAMM

Summary

The UpliftOnlyExample contract is vulnerable to manipulation through NFT transfers, allowing users to potentially reset deposit history (poolsFeeData), which could affect fee calculations. This manipulation could lead to reduced fees or bypassing intended fee structures.

Vulnerability Details

The UpliftOnlyExample contract allows NFTs, representing individual deposits, to be transferred between addresses. Each NFT has associated fee data stored in poolsFeeData, which includes the deposit value, timestamp, and uplift fee basis points.

The UpliftOnlyExample contract follows FILO (First In, Last Out) model for withdrawing the deposits. And the withdrawal fee is calculated based on change in the deposit value after deposits.

However, there is a vulnerability in afterUpdate() function, which will be called upon each NFT transfer. This function updates the lpTokenDepositValue, blockTimestampDeposit, and upliftFeeBps for the transferred NFT, effectively resetting these values to the current state.

https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol#L609

Here we can see that afterUpdate()updating lpTokenDepositValueto current one, which is used for calculating the withdrawal fee in onAfterRemoveLiquidity

https://github.com/Cyfrin/2024-12-quantamm/blob/a775db4273eb36e7b4536c5b60207c9f17541b92/pkg/pool-hooks/contracts/hooks-quantamm/UpliftOnlyExample.sol#L474C12-L476C55

Attack Path:

1. NFT Transfer: A user transfers NFTs to another address, potentially altering the sequence of deposits. Thus users can remove any deposits according to their needs by bypassing FILO

2. Reordering Deposits: By transferring NFTs back and forth, the user can manipulate the order in which deposits are processed.

3. Resetting Fee Context:

   • The afterUpdate() function resets key fee-related values (lpTokenDepositValue, blockTimestampDeposit, and upliftFeeBps) to reflect the current state at the time of transfer.

   • This reset can effectively treat the deposit as new or recent, potentially impacting how fees are calculated upon liquidity removal.

4. Fee Calculation Impact:

   • By manipulating the order and context of deposits through transfers, users can influence the FILO (First In, Last Out) model used for fee calculations.

   • This manipulation might result in lower fees being applied, as the contract may not accurately account for the uplift or changes in value of older deposits.

Impact

• Fee Manipulation: Users could exploit transfer mechanics to minimize fees, bypassing the intended uplift fee structure.

• Protocol Revenue Loss: The protocol could lose revenue due to reduced fees from manipulated deposit histories.

Tools Used

Manual Review

Recommendations

• Consistent Fee Calculation: Ensure that fee calculations are based on the actual value change of each deposit, regardless of transfer history.

  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]);

• Transfer Fee Implementation: Apply a fee to NFT transfers to capture the liquidity removal fees that might be bypassed due to manipulation, ensuring that the protocol collects appropriate fees regardless of transfer activities.