QuantAMM

QuantAMM
49,600 OP
View results
Submission Details
Severity: high
Valid

Users pay less than intended uplift fees by transferring their LPNFT before removing liquidity.

Summary

As the title suggests, users will pay a fee lower than intended or only the minimal fees, by transferring their LPNFT before withdrawing their liquidity.

Vulnerability Details

In onAfterRemoveLiquidity in UpliftOnlyExample, look at this section:

localData.lpTokenDepositValueNow = getPoolLPTokenValue(localData.prices, pool, MULDIRECTION.MULDOWN);
FeeData[] storage feeDataArray = poolsFeeData[pool][userAddress];
localData.feeDataArrayLength = feeDataArray.length;
localData.amountLeft = bptAmountIn;
for (uint256 i = localData.feeDataArrayLength - 1; i >= 0; --i) {
localData.lpTokenDepositValue = feeDataArray[i].lpTokenDepositValue;
localData.lpTokenDepositValueChange =
(int256(localData.lpTokenDepositValueNow) - int256(localData.lpTokenDepositValue)) /
int256(localData.lpTokenDepositValue);

lpTokenDepositValueChangeis dependent on the difference of the current value of LP (lpTokenDepositValueNow) and the previous value of LP (lpTokenDepositValue) when the deposit was initially made or when the LPNFT associated with a deposit was transferred.

It affects how feePerLPis calculated:

uint256 feePerLP;
// if the pool has increased in value since the deposit, the fee is calculated based on the deposit value
if (localData.lpTokenDepositValueChange > 0) {
feePerLP =
(uint256(localData.lpTokenDepositValueChange) * (uint256(feeDataArray[i].upliftFeeBps) * 1e18)) /
10000;
}
// if the pool has decreased in value since the deposit, the fee is calculated based on the base value - see wp
else {
//in most cases this should be a normal swap fee amount.
//there always myst be at least the swap fee amount to avoid deposit/withdraw attack surgace.
feePerLP = (uint256(minWithdrawalFeeBps) * 1e18) / 10000;
}

If lpTokenDepositValueChangeis small, then feePerLPwill also be small and hence the fee amount that the user pays will also be small. Users can abuse this and pay small fees by transferring the LPNFT associated with a deposit to another address and then withdrawing their liquidity. Whenever an LPNFT is transferred, the afterUpdate function updates the feeDataArray :

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

On NFT transfer, the lpTokenDepositValueis updated to the latest value. A user can easily abuse this:

  1. A user makes a deposit (adds liquidity) at time T1. He has an LPNFT corresponding to that deposit.

  2. A year passes by, his position has significantly gained in value.

  3. The user wants to remove his deposits now. However, he observes that feePerLPhe will be charged significantly more.

  4. So, he transfers his LPNFT to another address he owns. This would update the lpTokenDepositValueto the latest value.

  5. Now, using the other address, he withdraws the liquidity associated with that LPNFT.

  6. On removal of liquidity, lpTokenDepositValueChange will be small because the difference between the lpTokenDepositValueNowand lpTokenDepositValuewill be very small, because of the latest update made when NFT was transferred.

  7. The user ends up paying minimal uplift fees or if the lpTokenDepositValueChange == 0, then only the minimum fees.

Impact

User pays much less fees than intended, leading to a loss for the protocol.

Tools Used

Manual review

Recommendations

On transfer of LPNFTs, charge the from address "transfer" fees associated with that LPNFT.

Updates

Lead Judging Commences

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

finding_afterUpdate_bypass_fee_collection_updating_the_deposited_value

Likelihood: High, any transfer will trigger the bug. Impact: High, will update lpTokenDepositValue to the new current value without taking fees on profit.

Support

FAQs

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

Give us feedback!