Lines of code

Vulnerability details

Summary

Fees are incorrectly snapshotted when a new lien is created, potentially leading to a fee overpay.

Impact

The Particle LAMM protocol tracks fees using the same internal tracking built in Uniswap V3. Positions in Uniswap V3 contain a couple of variables feeGrowthInside0LastX128 and feeGrowthInside1LastX128 that track the last state in which fees were accrued to the position.

These values are flushed every time the NonfungiblePositionManager updates the position, for example in increaseLiquidity(), decreaseLiquidity() or collect(). The Uniswap implementation calculates the current values for feeGrowthInside0LastX128 and feeGrowthInside1LastX128, applies the difference using the last stored values to accrue the pending fees, and updates the latest values in position.feeGrowthInside0LastX128 and position.feeGrowthInside1LastX128. Using increaseLiquidity() as an example:

<https://github.com/Uniswap/v3-periphery/blob/697c2474757ea89fec12a4e6db16a574fe259610/contracts/NonfungiblePositionManager.sol#L232-L250&gt;

232:         (, uin256 feeGrowthInside0LastX128, uint256 feeGrowthInside1LastX128, , ) = pool.positions(positionKey);
233: 
234:         position.tokensOwed0 += uint128(
235:             FullMath.mulDiv(
236:                 feeGrowthInside0LastX128 - position.feeGrowthInside0LastX128,
237:                 position.liquidity,
238:                 FixedPoint128.Q128
239:             )
240:         );
241:         position.tokensOwed1 += uint128(
242:             FullMath.mulDiv(
243:                 feeGrowthInside1LastX128 - position.feeGrowthInside1LastX128,
244:                 position.liquidity,
245:                 FixedPoint128.Q128
246:             )
247:         );
248: 
249:         position.feeGrowthInside0LastX128 = feeGrowthInside0LastX128;
250:         position.feeGrowthInside1LastX128 = feeGrowthInside1LastX128;

This means that NonfungiblePositionManager doesn’t always hold the up-to-date values for these variables: if the position isn’t updated, these variables indicate the values at the time of the last action performed over the position.

When a user opens a new position in Particle, the implementation grabs these two variables and stores them in the Lien structure, so that when the position is closed it can calculate the owed fees by taking the difference between the actual state of feeGrowthInside0LastX128 and feeGrowthInside1LastX128.

<https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/protocol/ParticlePositionManager.sol#L151-L179&gt;

151:     function openPosition(
152:         DataStruct.OpenPositionParams calldata params
153:     ) public override nonReentrant returns (uint96 lienId, uint256 collateralTo) {
154:         if (params.liquidity == 0) revert Errors.InsufficientBorrow();
155: 
156:         // local cache to avoid stack too deep
157:         DataCache.OpenPositionCache memory cache;
158: 
159:         // prepare data for swap
160:         (
161:             cache.tokenFrom,
162:             cache.tokenTo,
163:             cache.feeGrowthInside0LastX128,
164:             cache.feeGrowthInside1LastX128,
165:             cache.collateralFrom,
166:             collateralTo
167:         ) = Base.prepareLeverage(params.tokenId, params.liquidity, params.zeroForOne);
168: 
169:         // decrease liquidity from LP position, pull the amount to this contract
170:         (cache.amountFromBorrowed, cache.amountToBorrowed) = LiquidityPosition.decreaseLiquidity(
171:             params.tokenId,
172:             params.liquidity
173:         );
174:         LiquidityPosition.collectLiquidity(
175:             params.tokenId,
176:             uint128(cache.amountFromBorrowed),
177:             uint128(cache.amountToBorrowed),
178:             address(this)
179:         );
...
247:         liens[keccak256(abi.encodePacked(msg.sender, lienId = _nextRecordId++))] = Lien.Info({
248:             tokenId: uint40(params.tokenId),
249:             liquidity: params.liquidity,
250:             token0PremiumPortion: cache.token0PremiumPortion,
251:             token1PremiumPortion: cache.token1PremiumPortion,
252:             startTime: uint32(block.timestamp),
253:             feeGrowthInside0LastX128: cache.feeGrowthInside0LastX128,
254:             feeGrowthInside1LastX128: cache.feeGrowthInside1LastX128,
255:             zeroForOne: params.zeroForOne
256:         });

As we can see in the previous snippet of code, the implementation fetches the values using Base.prepareLeverage(), which simply retrieves them from the positions() function of Uniswap NonfungiblePositionManager.

    function prepareLeverage(
        uint256 tokenId,
        uint128 liquidity,
        bool zeroForOne
    )
        internal
        view
        returns (
            address tokenFrom,
            address tokenTo,
            uint256 feeGrowthInside0LastX128,
            uint256 feeGrowthInside1LastX128,
            uint256 collateralFrom,
            uint256 collateralTo
        )
    {
        int24 tickLower;
        int24 tickUpper;
        (
            ,
            ,
            tokenFrom,
            tokenTo,
            ,
            tickLower,
            tickUpper,
            ,
            feeGrowthInside0LastX128,
            feeGrowthInside1LastX128,
            ,

        ) = UNI_POSITION_MANAGER.positions(tokenId);

However, these values are fetched before doing any modification to the position. The Uniswap position is modifiedafter fetching the values for feeGrowthInside0LastX128 and feeGrowthInside1LastX128 by the calls to decreaseLiquidity() and collectLiquidity(). The values here belong to the current values at the time of the last action over the position, prior to opening the position in Particle.

This means that the borrower will overpay for any fee pending to be accrued in the LP prior to opening the position. When the borrower closes the position (or is liquidated) the protocol will calculate the fees by taking the difference between the current values and the snapshotted values in the Lien, which also include any pending fees present before opening the position.

1. LP is updated at t1, by any call to increaseLiquidity(), decreaseLiquidity(), etc.
2. Swaps are executed in the pool, earning fees for the LP.
3. A user opens a new position in Particle at t2.
4. The user closes the position at t3.

In this scenario, the user will not only pay for any accrued fees between t2 and t3, but also from t1 and t2.

Proof of Concept

The following test reproduces the issue. Here, we simulate several large swaps in the pool to earn fees for the LP. Then, right after opening a new position, we can see there are already owed fees in the newly created position.

Note: the snippet shows only the relevant code for the test. Full test file can be found here.

function test_ParticlePositionMananger_FeeOverpay() public {
    // artificially generate fees in the pool so the minted position gets fees
    address feeGenerator = makeAddr("feeGenerator");

    // mint 1M USDC to feeGenerator
    deal(address(USDC), feeGenerator, 1_000_000 * 1e6);

    for (uint256 i = 0; i < 10; i++) {
        uint256 usdcAmount = USDC.balanceOf(feeGenerator);
        _swap(feeGenerator, address(USDC), address(WETH), FEE, usdcAmount);

        uint256 wethAmount = WETH.balanceOf(feeGenerator);
        _swap(feeGenerator, address(WETH), address(USDC), FEE, wethAmount);
    }

    // update state
    IUniswapV3Pool pool = IUniswapV3Pool(uniswapV3Factory.getPool(address(USDC), address(WETH), FEE));
    (_sqrtRatioX96, _tick, , , , , ) = pool.slot0();

    // open position in particle
    uint128 borrowerLiquidity = _liquidity / _borrowerLiquidityPorition;
    (uint256 amount0ToBorrow, uint256 amount1ToBorrow) = LiquidityAmounts.getAmountsForLiquidity(
        _sqrtRatioX96,
        _sqrtRatioAX96,
        _sqrtRatioBX96,
        borrowerLiquidity
    );
    (, uint256 requiredEth) = particleInfoReader.getRequiredCollateral(borrowerLiquidity, _tickLower, _tickUpper);
    uint256 amountNeeded = QUOTER.quoteExactOutputSingle(
        address(USDC),
        address(WETH),
        FEE,
        requiredEth - amount1ToBorrow,
        0
    );
    uint256 amountIn = amountNeeded + amountNeeded / 1e6 - amount0ToBorrow; // 1e-6 tolerance

    _borrowToLong(SWAPPER, address(USDC), _tokenId, amountIn, amount0ToBorrow, borrowerLiquidity);

    // Get state of owed fees
    (uint256 token0Owed, uint256 token1Owed, , , , ) = particleInfoReader.getOwedInfo(SWAPPER, 0);

    // Fees are non-zero right after opening the position!
    assertGt(token0Owed, 0);
    assertGt(token1Owed, 0);

    console.log("Owed token0:", token0Owed);
    console.log("Owed token1:", token1Owed);
}

Recommendation

Fetch both feeGrowthInside0LastX128 and feeGrowthInside1LastX128 after the calls to withdraw the liquidity from the Uniswap position. This will ensure the values are correctly up to date, as the calls to decreaseLiquidity() or collectLiquidity() will update the Uniswap position.

    // prepare data for swap
    (
        cache.tokenFrom,
        cache.tokenTo,
-       cache.feeGrowthInside0LastX128,
-       cache.feeGrowthInside1LastX128,
+       ,
+       ,
        cache.collateralFrom,
        collateralTo
    ) = Base.prepareLeverage(params.tokenId, params.liquidity, params.zeroForOne);
    
    // decrease liquidity from LP position, pull the amount to this contract
    (cache.amountFromBorrowed, cache.amountToBorrowed) = LiquidityPosition.decreaseLiquidity(
        params.tokenId,
        params.liquidity
    );
    LiquidityPosition.collectLiquidity(
        params.tokenId,
        uint128(cache.amountFromBorrowed),
        uint128(cache.amountToBorrowed),
        address(this)
    );
    if (!params.zeroForOne)
        (cache.amountFromBorrowed, cache.amountToBorrowed) = (cache.amountToBorrowed, cache.amountFromBorrowed);
        
+   (, , , , , , , , cache.feeGrowthInside0LastX128, cache.feeGrowthInside1LastX128, , ) = Base
+       .UNI_POSITION_MANAGER
+       .positions(params.tokenId);

Assessed type

Other

---

The text was updated successfully, but these errors were encountered:

All reactions