Particle Leverage AMM Protocol Invitational - said's results

A permissionless protocol to leverage trade any ERC20 tokens.

General Information

Platform: Code4rena

Start Date: 11/12/2023

Pot Size: $24,150 USDC

Total HM: 19

Participants: 5

Period: 10 days

Judge: 0xLeastwood

Total Solo HM: 6

Id: 312

League: ETH

Particle Protocol

Findings Distribution

Researcher Performance

Rank: 1/5

Findings: 5

Award: $0.00

QA:
grade-a

🌟 Selected for report: 2

🚀 Solo Findings: 2

Findings Information

🌟 Selected for report: bin2chen

Also found by: ladboy233, said

Labels

bug
3 (High Risk)
satisfactory
upgraded by judge
duplicate-31

Awards

Data not available

External Links

Lines of code

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/protocol/ParticlePositionManager.sol#L460-L490

Vulnerability details

Impact

If a trader is blacklisted from a blacklistable ERC20 token while has an open position, it may not be possible to liquidate the position.

Proof of Concept

When liquidate position, it will eventually calculate the amount of token that need to be send to borrower, the amount will be non-zero if the position is profitable or the borrower has excess premium.

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/protocol/ParticlePositionManager.sol#L374

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/protocol/ParticlePositionManager.sol#L460-L490

    function _closePosition(
        DataStruct.ClosePositionParams calldata params,
        DataCache.ClosePositionCache memory cache,
        Lien.Info memory lien,
        address borrower
    ) internal {
        ...
        // calculate the the amounts owed to LP up to the premium in the lien
        // must ensure enough amount is left to pay for interest first, then send gains and fund left to borrower
        ///@dev refundWithCheck ensures actual cannot be more than expected, since amount owed to LP is in actual,
        ///     it ensures (1) on the collateralFrom part of refund, tokenOwed is covered, and (2) on the amountReceived
        ///      part, received is no less than liquidity addback + token owed.
        if (lien.zeroForOne) {
            cache.token0Owed = cache.token0Owed < cache.tokenToPremium ? cache.token0Owed : cache.tokenToPremium;
            cache.token1Owed = cache.token1Owed < cache.tokenFromPremium ? cache.token1Owed : cache.tokenFromPremium;
>>>         Base.refundWithCheck(
                borrower,
                cache.tokenFrom,
                cache.collateralFrom + cache.tokenFromPremium,
                cache.amountSpent + cache.amountFromAdd + cache.token1Owed
            );
>>>         Base.refundWithCheck(
                borrower,
                cache.tokenTo,
                cache.amountReceived + cache.tokenToPremium,
                cache.amountToAdd + cache.token0Owed
            );
        } else {
            cache.token0Owed = cache.token0Owed < cache.tokenFromPremium ? cache.token0Owed : cache.tokenFromPremium;
            cache.token1Owed = cache.token1Owed < cache.tokenToPremium ? cache.token1Owed : cache.tokenToPremium;
>>>         Base.refundWithCheck(
                borrower,
                cache.tokenFrom,
                cache.collateralFrom + cache.tokenFromPremium,
                cache.amountSpent + cache.amountFromAdd + cache.token0Owed
            );
>>>         Base.refundWithCheck(
                borrower,
                cache.tokenTo,
                cache.amountReceived + cache.tokenToPremium,
                cache.amountToAdd + cache.token1Owed
            );
        }

        // pay for interest
        lps.addTokensOwed(lien.tokenId, cache.token0Owed, cache.token1Owed);
    }

The issue with this design is that if a borrower is blacklisted from a blacklistable token, such as USDC, it will not be possible to send profits or excess premium back to the borrower.

This will not only impact the borrower but also the liquidity provider, as the token cannot be added back to the Uniswap V3 LP position.

Tools Used

Manual review

use the "Pull over Push" design for sending profit/excess premium to the borrower.

Assessed type

Token-Transfer

#0 - c4-judge

2023-12-21T22:14:18Z

0xleastwood marked the issue as duplicate of #31

#1 - c4-judge

2023-12-24T12:32:51Z

0xleastwood marked the issue as satisfactory

#2 - c4-judge

2023-12-26T11:33:25Z

0xleastwood changed the severity to 3 (High Risk)

Findings Information

🌟 Selected for report: said

Labels

bug
3 (High Risk)
primary issue
selected for report
sponsor confirmed
H-04

Awards

Data not available

External Links

Lines of code

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/libraries/Base.sol#L318-L342

Vulnerability details

Impact

When operations need to calculate Uniswap V3 position's fee growth, it used similar function implemented by uniswap v3. However, according to this known issue : https://github.com/Uniswap/v3-core/issues/573. The contract is implicitly relies on underflow/overflow when calculating the fee growth, if underflow is prevented, some operations that rely on fee growth will revert.

Proof of Concept

It can be observed that current implementation of getFeeGrowthInside not allow underflow/overflow to happen when calculating feeGrowthInside0X128 and feeGrowthInside1X128, because the contract used solidity 0.8.23.

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/libraries/Base.sol#L318-L342

    function getFeeGrowthInside(
        address token0,
        address token1,
        uint24 fee,
        int24 tickLower,
        int24 tickUpper
    ) internal view returns (uint256 feeGrowthInside0X128, uint256 feeGrowthInside1X128) {
        IUniswapV3Pool pool = IUniswapV3Pool(UNI_FACTORY.getPool(token0, token1, fee));
        (, int24 tickCurrent, , , , , ) = pool.slot0();
        (, , uint256 lowerFeeGrowthOutside0X128, uint256 lowerFeeGrowthOutside1X128, , , , ) = pool.ticks(tickLower);
        (, , uint256 upperFeeGrowthOutside0X128, uint256 upperFeeGrowthOutside1X128, , , , ) = pool.ticks(tickUpper);

        if (tickCurrent < tickLower) {
            feeGrowthInside0X128 = lowerFeeGrowthOutside0X128 - upperFeeGrowthOutside0X128;
            feeGrowthInside1X128 = lowerFeeGrowthOutside1X128 - upperFeeGrowthOutside1X128;
        } else if (tickCurrent < tickUpper) {
            uint256 feeGrowthGlobal0X128 = pool.feeGrowthGlobal0X128();
            uint256 feeGrowthGlobal1X128 = pool.feeGrowthGlobal1X128();
            feeGrowthInside0X128 = feeGrowthGlobal0X128 - lowerFeeGrowthOutside0X128 - upperFeeGrowthOutside0X128;
            feeGrowthInside1X128 = feeGrowthGlobal1X128 - lowerFeeGrowthOutside1X128 - upperFeeGrowthOutside1X128;
        } else {
            feeGrowthInside0X128 = upperFeeGrowthOutside0X128 - lowerFeeGrowthOutside0X128;
            feeGrowthInside1X128 = upperFeeGrowthOutside1X128 - lowerFeeGrowthOutside1X128;
        }
    }

This could impact crucial operation that rely on this call, such as liquidation, could revert unexpectedly. This behavior is quite often especially for pools that use lower fee.

Coded PoC :

Add the following test to /test/OpenPosition.t.sol :

  function testLiquidationRevert() public {
        address LIQUIDATOR = payable(address(0x7777));
        uint128 REPAY_LIQUIDITY_PORTION = 1000;
        _setupLowerOutOfRange();
        testBaseOpenLongPosition();
        // get lien info
        (, uint128 liquidityInside, , , , , , ) = particlePositionManager.liens(
            keccak256(abi.encodePacked(SWAPPER, uint96(0)))
        );
        // start reclaim
        vm.startPrank(LP);
        vm.warp(block.timestamp + 1);
        particlePositionManager.reclaimLiquidity(_tokenId);
        vm.stopPrank();
        // add back liquidity requirement
        vm.warp(block.timestamp + 7 days);
        IUniswapV3Pool _pool = IUniswapV3Pool(uniswapV3Factory.getPool(address(USDC), address(WETH), FEE));
        (uint160 currSqrtRatioX96, , , , , , ) = _pool.slot0();
        (uint256 amount0ToReturn, uint256 amount1ToReturn) = LiquidityAmounts.getAmountsForLiquidity(
            currSqrtRatioX96,
            _sqrtRatioAX96,
            _sqrtRatioBX96,
            liquidityInside
        );
        (uint256 usdCollateral, uint256 ethCollateral) = particleInfoReader.getRequiredCollateral(liquidityInside, _tickLower, _tickUpper);

        // get swap data
        uint160 currentPrice = particleInfoReader.getCurrentPrice(address(USDC), address(WETH), FEE);
        uint256 amountSwap = ethCollateral - amount1ToReturn;

        ISwapRouter.ExactInputSingleParams memory params = ISwapRouter.ExactInputSingleParams({
            tokenIn: address(WETH),
            tokenOut: address(USDC),
            fee: FEE,
            recipient: address(particlePositionManager),
            deadline: block.timestamp,
            amountIn: amountSwap,
            amountOutMinimum: 0,
            sqrtPriceLimitX96: currentPrice + currentPrice / SLIPPAGE_FACTOR
        });
        bytes memory data = abi.encodeWithSelector(ISwapRouter.exactInputSingle.selector, params);
        // liquidate position
        vm.startPrank(LIQUIDATOR);
        vm.expectRevert(abi.encodeWithSelector(Errors.InsufficientRepay.selector));
        particlePositionManager.liquidatePosition(
            DataStruct.ClosePositionParams({lienId: uint96(0), amountSwap: amountSwap, data: data}),
            SWAPPER
        );
        vm.stopPrank();
    }

Also modify FEE inside /test/Base.t.sol to 500 :

contract ParticlePositionManagerTestBase is Test {
    using Lien for mapping(bytes32 => Lien.Info);

    IERC20 public constant WETH = IERC20(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2);
    IERC20 public constant USDC = IERC20(0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48);
    IERC20 public constant DAI = IERC20(0x6B175474E89094C44Da98b954EedeAC495271d0F);
    uint256 public constant USDC_AMOUNT = 50000000 * 1e6;
    uint256 public constant DAI_AMOUNT = 50000000 * 1e18;
    uint256 public constant WETH_AMOUNT = 50000 * 1e18;
    address payable public constant ADMIN = payable(address(0x4269));
    address payable public constant LP = payable(address(0x1001));
    address payable public constant SWAPPER = payable(address(0x1002));
    address payable public constant WHALE = payable(address(0x6666));
    IQuoter public constant QUOTER = IQuoter(0xb27308f9F90D607463bb33eA1BeBb41C27CE5AB6);
    int24 public constant TICK_SPACING = 60;
    uint256 public constant BASIS_POINT = 1_000_000;
-    uint24 public constant FEE = 3000; // uniswap swap fee
+    uint24 public constant FEE = 500; // uniswap swap fee
   ..
}

Run the test :

forge test --fork-url $MAINNET_RPC_URL --fork-block-number 18750931 --match-contract OpenPositionTest --match-test testRevertUnderflow -vvvv

Log output :

image

It can be observed that the liquidation revert due to the underflow.

Tools Used

Manual review.

Use unchecked when calculating feeGrowthInside0X128 and feeGrowthInside1X128.

Assessed type

Error

#0 - c4-judge

2023-12-21T21:45:35Z

0xleastwood marked the issue as primary issue

#1 - romeroadrian

2023-12-22T16:26:58Z

Really interesting side effect of upgrading code from solidity < 0.8 👍

#2 - wukong-particle

2023-12-23T02:49:44Z

Oh this one is great. Will update the code to uncheck feeGrowthInside0X128 and feeGrowthInside1X128 calculations.

#3 - c4-judge

2023-12-23T19:41:37Z

0xleastwood marked the issue as selected for report

#4 - c4-sponsor

2023-12-24T08:58:55Z

wukong-particle (sponsor) confirmed

Findings Information

🌟 Selected for report: adriro

Also found by: said

Labels

bug
2 (Med Risk)
satisfactory
duplicate-51

Awards

Data not available

External Links

Lines of code

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/protocol/ParticlePositionManager.sol#L349-L368

Vulnerability details

Impact

The liquidation eligibility check is performed after the premiums provided by the trader are decreased by liquidation rewards. This poses a risk for traders and could lead to a condition where a trader's position unexpectedly becomes eligible for liquidation.

Proof of Concept

It can be observed that currently, the liquidation eligibility check is performed after decreasing closeCache.tokenFromPremium and closeCache.tokenToPremium by liquidateCache.liquidationRewardFrom and liquidateCache.liquidationRewardTo.

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/protocol/ParticlePositionManager.sol#L349-L368

    /// @inheritdoc IParticlePositionManager
    function liquidatePosition(
        DataStruct.ClosePositionParams calldata params,
        address borrower
    ) external override nonReentrant {
        bytes32 lienKey = keccak256(abi.encodePacked(borrower, params.lienId));
        Lien.Info memory lien = liens.getInfo(lienKey);

        // check lien is valid
        if (lien.liquidity == 0) revert Errors.RecordEmpty();

        // local cache to avoid stack too deep
        DataCache.ClosePositionCache memory closeCache;
        DataCache.LiquidatePositionCache memory liquidateCache;

        // get liquidation parameters
        ///@dev calculate premium outside of _closePosition to allow liquidatePosition to take reward from premium
        (
            closeCache.tokenFrom,
            closeCache.tokenTo,
            liquidateCache.tokenFromOwed,
            liquidateCache.tokenToOwed,
            closeCache.tokenFromPremium,
            closeCache.tokenToPremium,
            closeCache.collateralFrom,

        ) = Base.getOwedInfoConverted(
            DataStruct.OwedInfoParams({
                tokenId: lien.tokenId,
                liquidity: lien.liquidity,
                feeGrowthInside0LastX128: lien.feeGrowthInside0LastX128,
                feeGrowthInside1LastX128: lien.feeGrowthInside1LastX128,
                token0PremiumPortion: lien.token0PremiumPortion,
                token1PremiumPortion: lien.token1PremiumPortion
            }),
            lien.zeroForOne
        );

        // calculate liquidation reward
        liquidateCache.liquidationRewardFrom =
            ((closeCache.tokenFromPremium) * LIQUIDATION_REWARD_FACTOR) /
            uint128(Base.BASIS_POINT);
        liquidateCache.liquidationRewardTo =
            ((closeCache.tokenToPremium) * LIQUIDATION_REWARD_FACTOR) /
            uint128(Base.BASIS_POINT);
>>>     closeCache.tokenFromPremium -= liquidateCache.liquidationRewardFrom;
>>>     closeCache.tokenToPremium -= liquidateCache.liquidationRewardTo;

        // check for liquidation condition
        ///@dev the liquidation condition is that
        ///     (EITHER premium is not enough) OR (cutOffTime > startTime AND currentTime > startTime + LOAN_TERM)
>>>     if (
            !((closeCache.tokenFromPremium < liquidateCache.tokenFromOwed ||
                closeCache.tokenToPremium < liquidateCache.tokenToOwed) ||
                (lien.startTime < lps.getRenewalCutoffTime(lien.tokenId) &&
                    lien.startTime + LOAN_TERM < block.timestamp))
        ) {
            revert Errors.LiquidationNotMet();
        }

       ...

With this design, when the protocol decides to increase the LIQUIDATION_REWARD_FACTOR, it could directly impact traders' positions and potentially cause them to become liquidatable.

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/protocol/ParticlePositionManager.sol#L567-L571

    function updateLiquidationRewardFactor(uint128 liquidationRewardFactor) external override onlyOwner {
        if (liquidationRewardFactor > _LIQUIDATION_REWARD_FACTOR_MAX) revert Errors.InvalidValue();
        LIQUIDATION_REWARD_FACTOR = liquidationRewardFactor;
        emit UpdateLiquidationRewardFactor(liquidationRewardFactor);
    }

Tools Used

Manual review

Perform the check before decreasing closeCache.tokenFromPremium and closeCache.tokenToPremium so that traders' position condition is easier to predict and maintain.

Assessed type

Context

#0 - c4-judge

2023-12-21T22:25:30Z

0xleastwood marked the issue as primary issue

#1 - romeroadrian

2023-12-22T16:22:30Z

Duplicate #51

#2 - c4-judge

2023-12-22T19:22:09Z

0xleastwood marked the issue as duplicate of #51

#3 - c4-judge

2023-12-24T12:33:16Z

0xleastwood marked the issue as satisfactory

Findings Information

🌟 Selected for report: said

Labels

bug
2 (Med Risk)
primary issue
selected for report
sponsor acknowledged
M-14

Awards

Data not available

External Links

Lines of code

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/protocol/ParticlePositionManager.sol#L218-L244

Vulnerability details

Impact

Excess tokens from margins provided by traders when opening position could become stuck inside the ParticlePositionManager due to precision loss when calculating the token premium portion.

Proof of Concept

When traders call openPosition, eventually it will calculate cache.token0PremiumPortion and cache.token1PremiumPortion before put it into lien data.

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/protocol/ParticlePositionManager.sol#L218-L244

    /// @inheritdoc IParticlePositionManager
    function openPosition(
        DataStruct.OpenPositionParams calldata params
    ) public override nonReentrant returns (uint96 lienId, uint256 collateralTo) {
       ...
        if (params.zeroForOne) {
>>>         cache.token0PremiumPortion = Base.uint256ToUint24(
                ((params.marginFrom + cache.amountFromBorrowed - cache.feeAmount - cache.amountSpent) *
                    Base.BASIS_POINT) / cache.collateralFrom
            );
>>>         cache.token1PremiumPortion = Base.uint256ToUint24(
                ((cache.amountReceived + cache.amountToBorrowed + params.marginTo - collateralTo) * Base.BASIS_POINT) /
                    collateralTo
            );
            if (
                cache.token0PremiumPortion < params.tokenFromPremiumPortionMin ||
                cache.token1PremiumPortion < params.tokenToPremiumPortionMin
            ) revert Errors.InsufficientPremium();
        } else {
>>>         cache.token1PremiumPortion = Base.uint256ToUint24(
                ((params.marginFrom + cache.amountFromBorrowed - cache.feeAmount - cache.amountSpent) *
                    Base.BASIS_POINT) / cache.collateralFrom
            );
>>>         cache.token0PremiumPortion = Base.uint256ToUint24(
                ((cache.amountReceived + cache.amountToBorrowed + params.marginTo - collateralTo) * Base.BASIS_POINT) /
                    collateralTo
            );
            if (
                cache.token0PremiumPortion < params.tokenToPremiumPortionMin ||
                cache.token1PremiumPortion < params.tokenFromPremiumPortionMin
            ) revert Errors.InsufficientPremium();
        }

        // create a new lien
        liens[keccak256(abi.encodePacked(msg.sender, lienId = _nextRecordId++))] = Lien.Info({
            tokenId: uint40(params.tokenId),
            liquidity: params.liquidity,
            token0PremiumPortion: cache.token0PremiumPortion,
            token1PremiumPortion: cache.token1PremiumPortion,
            startTime: uint32(block.timestamp),
            feeGrowthInside0LastX128: cache.feeGrowthInside0LastX128,
            feeGrowthInside1LastX128: cache.feeGrowthInside1LastX128,
            zeroForOne: params.zeroForOne
        });

        emit OpenPosition(msg.sender, lienId, collateralTo);
    }

It can be observed that when calculating cache.token0PremiumPortion and cache.token1PremiumPortion, precision loss could occur, especially when the token used has a high decimals (e.g. 18) while BASIS_POINT used only 1_000_000. When this happens, the token could become stuck inside the ParticlePositionManager.

Coded PoC :

The PoC goal is to check the amount of tokens inside the ParticlePositionManager after the position is opened, premium is added, liquidated, the LP withdraws the tokens and collects fees, and the admin collects the treasury.

Add the following test inside test/OpenPosition.t.sol and also add import "forge-std/console.sol"; to the test contract.

   function testLiquidateAndClearLiquidity() public {
        address LIQUIDATOR = payable(address(0x7777));
        uint128 REPAY_LIQUIDITY_PORTION = 1000;
        testBaseOpenLongPosition();
        // add enough premium
        uint128 premium0 = 500 * 1e6;
        uint128 premium1 = 0.5 ether;
        vm.startPrank(WHALE);
        USDC.transfer(SWAPPER, premium0);
        WETH.transfer(SWAPPER, premium1);
        vm.stopPrank();

        vm.startPrank(SWAPPER);
        TransferHelper.safeApprove(address(USDC), address(particlePositionManager), premium0);
        TransferHelper.safeApprove(address(WETH), address(particlePositionManager), premium1);
        particlePositionManager.addPremium(0, premium0, premium1);
        vm.stopPrank();
        // get lien info
        (, uint128 liquidityInside, , , , , , ) = particlePositionManager.liens(
            keccak256(abi.encodePacked(SWAPPER, uint96(0)))
        );
        // start reclaim
        vm.startPrank(LP);
        vm.warp(block.timestamp + 1);
        particlePositionManager.reclaimLiquidity(_tokenId);
        vm.stopPrank();
        // add back liquidity requirement
        vm.warp(block.timestamp + 7 days);
        IUniswapV3Pool _pool = IUniswapV3Pool(uniswapV3Factory.getPool(address(USDC), address(WETH), FEE));
        (uint160 currSqrtRatioX96, , , , , , ) = _pool.slot0();
        (uint256 amount0ToReturn, uint256 amount1ToReturn) = LiquidityAmounts.getAmountsForLiquidity(
            currSqrtRatioX96,
            _sqrtRatioAX96,
            _sqrtRatioBX96,
            liquidityInside + liquidityInside / REPAY_LIQUIDITY_PORTION
        );
        (, uint256 ethCollateral) = particleInfoReader.getRequiredCollateral(liquidityInside, _tickLower, _tickUpper);

        // get swap data
        uint160 currentPrice = particleInfoReader.getCurrentPrice(address(USDC), address(WETH), FEE);
        uint256 amountSwap = ethCollateral - amount1ToReturn;
        ISwapRouter.ExactInputSingleParams memory params = ISwapRouter.ExactInputSingleParams({
            tokenIn: address(WETH),
            tokenOut: address(USDC),
            fee: FEE,
            recipient: address(particlePositionManager),
            deadline: block.timestamp,
            amountIn: amountSwap,
            amountOutMinimum: 0,
            sqrtPriceLimitX96: currentPrice + currentPrice / SLIPPAGE_FACTOR
        });
        bytes memory data = abi.encodeWithSelector(ISwapRouter.exactInputSingle.selector, params);
        // liquidate position
        vm.startPrank(LIQUIDATOR);
        particlePositionManager.liquidatePosition(
            DataStruct.ClosePositionParams({lienId: uint96(0), amountSwap: amountSwap, data: data}),
            SWAPPER
        );
        vm.stopPrank();
        vm.startPrank(LP);
        // console.log("liquidity before : ");
        // console.log(_liquidity);
        (, , , , , , , _liquidity, , , , ) = nonfungiblePositionManager.positions(_tokenId);
        (uint256 amount0Decreased, uint256 amount1Decreased) = particlePositionManager.decreaseLiquidity(
            _tokenId,
            _liquidity
        );
        (uint256 amount0Returned, uint256 amount1Returned) = particlePositionManager.collectLiquidity(_tokenId);
        vm.stopPrank();
        vm.startPrank(ADMIN);
        particlePositionManager.withdrawTreasury(address(USDC), ADMIN);
        particlePositionManager.withdrawTreasury(address(WETH), ADMIN);
        vm.stopPrank();
        uint256 usdcBalanceAfter = USDC.balanceOf(LP);
        uint256 wethBalanceAfter = WETH.balanceOf(LP);
        console.log("balance LP after - USDC:");
        console.log(usdcBalanceAfter);
        console.log("balance LP after -  WETH:");
        console.log(wethBalanceAfter);
        console.log("balance inside manager -  USDC:");
        console.log(USDC.balanceOf(address(particlePositionManager)));
        console.log("balance inside manager - WETH:");
        console.log(WETH.balanceOf(address(particlePositionManager)));

    }

Run the test :

forge test -vv --fork-url $MAINNET_RPC_URL --fork-block-number 18750931 --match-contract OpenPositionTest --match-test testLiquidateAndClearLiquidity -vvv

Output log :

  balance LP after - USDC:
  15000228083

  balance LP after -  WETH:
  9999997057795940602

  balance inside manager -  USDC:
  1123

  balance inside manager - WETH:
  516472404479

It can be observed that some tokens stuck inside the ParticlePositionManager contract.

Tools Used

Manual review + foundry

Use a higher scaling for BASIS_POINT, considering the fact that tokens commonly have a high number of decimals

Assessed type

Math

#0 - c4-judge

2023-12-21T22:15:30Z

0xleastwood marked the issue as primary issue

#1 - wukong-particle

2023-12-23T00:12:48Z

Good point. Appreciated the detailed PoC. On our end, I think we should have documented the use of tokenPremiumPortion more clearly. It's a compromise we took to ensure the lien structure stored on chain is less than 3 uint256. We compressed a lot and were only left with uint24.

We think a dust of ~1000 USDC when a user is spending 15B USDC is acceptable. We could add a function to collect dust if needed (might be complicated though).

If there can be serious attack around this dust, please do raise it. Thanks!

#2 - c4-judge

2023-12-23T23:05:08Z

0xleastwood marked the issue as selected for report

#3 - c4-sponsor

2023-12-24T08:58:37Z

wukong-particle (sponsor) acknowledged

#4 - romeroadrian

2023-12-30T14:34:18Z

These are leftover amounts that are caused by the rounding of the premium as a portion (premium -> portion -> premium). It looks more on the QA side to me.

#5 - 0xleastwood

2023-12-31T13:21:50Z

These are leftover amounts that are caused by the rounding of the premium as a portion (premium -> portion -> premium). It looks more on the QA side to me.

This seems considerable and will accrue significantly over the lifetime of the protocol. I think the severity is justified as per the judging criteria.

2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.

Findings Information

🌟 Selected for report: ladboy233

Also found by: adriro, bin2chen, immeas, said

Labels

bug
2 (Med Risk)
satisfactory
duplicate-2

Awards

Data not available

External Links

Lines of code

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/libraries/LiquidityPosition.sol#L253-L263 https://github.com/code-423n4/2023-12-particle/blob/main/contracts/libraries/LiquidityPosition.sol#L178-L204

Vulnerability details

Impact

According to uniswap V3 docs for increase and decrease liquidity:

In production, amount0Min and amount1Min should be adjusted to create slippage protections.

However, current implementation inside Particle doesn't allow users to provide the amount minimum.

Proof of Concept

This are the current implementation for increasing and decreasing liquidity, it can be observed that amount0Min and amount1Min provided are 0 :

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/libraries/LiquidityPosition.sol#L178-L204

    function increaseLiquidity(
        address token0,
        address token1,
        uint256 tokenId,
        uint256 amount0,
        uint256 amount1
    ) internal returns (uint128 liquidity, uint256 amount0Added, uint256 amount1Added) {
        // approve spending for uniswap's position manager
        TransferHelper.safeApprove(token0, Base.UNI_POSITION_MANAGER_ADDR, amount0);
        TransferHelper.safeApprove(token1, Base.UNI_POSITION_MANAGER_ADDR, amount1);

        // increase liquidity via position manager
        (liquidity, amount0Added, amount1Added) = Base.UNI_POSITION_MANAGER.increaseLiquidity(
            INonfungiblePositionManager.IncreaseLiquidityParams({
                tokenId: tokenId,
                amount0Desired: amount0,
                amount1Desired: amount1,
                amount0Min: 0,
                amount1Min: 0,
                deadline: block.timestamp
            })
        );

        // reset approval
        TransferHelper.safeApprove(token0, Base.UNI_POSITION_MANAGER_ADDR, 0);
        TransferHelper.safeApprove(token1, Base.UNI_POSITION_MANAGER_ADDR, 0);
    }

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/libraries/LiquidityPosition.sol#L253-L263

    function decreaseLiquidity(uint256 tokenId, uint128 liquidity) internal returns (uint256 amount0, uint256 amount1) {
        (amount0, amount1) = Base.UNI_POSITION_MANAGER.decreaseLiquidity(
            INonfungiblePositionManager.DecreaseLiquidityParams({
                tokenId: tokenId,
                liquidity: liquidity,
                amount0Min: 0,
                amount1Min: 0,
                deadline: block.timestamp
            })
        );
    }

There are several instances where increasing and decreasing liquidity are called :

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/protocol/ParticlePositionManager.sol#L118-L124

    function increaseLiquidity(
        uint256 tokenId,
        uint256 amount0,
        uint256 amount1
    ) external override nonReentrant returns (uint128 liquidity, uint256 amount0Added, uint256 amount1Added) {
        (liquidity, amount0Added, amount1Added) = lps.increaseLiquidity(tokenId, amount0, amount1);
    }

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/protocol/ParticlePositionManager.sol#L127-L132

    function decreaseLiquidity(
        uint256 tokenId,
        uint128 liquidity
    ) external override nonReentrant returns (uint256 amount0Decreased, uint256 amount1Decreased) {
        (amount0Decreased, amount1Decreased) = lps.decreaseLiquidity(tokenId, liquidity);
    }

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/protocol/ParticlePositionManager.sol#L423-L439

    function _closePosition(
        DataStruct.ClosePositionParams calldata params,
        DataCache.ClosePositionCache memory cache,
        Lien.Info memory lien,
        address borrower
    ) internal {
       ...

        // add liquidity back to borrower
        if (lien.zeroForOne) {
>>>         (cache.liquidityAdded, cache.amountToAdd, cache.amountFromAdd) = LiquidityPosition.increaseLiquidity(
                cache.tokenTo,
                cache.tokenFrom,
                lien.tokenId,
                cache.amountToAdd,
                cache.amountFromAdd
            );
        } else {
>>>         (cache.liquidityAdded, cache.amountFromAdd, cache.amountToAdd) = LiquidityPosition.increaseLiquidity(
                cache.tokenFrom,
                cache.tokenTo,
                lien.tokenId,
                cache.amountFromAdd,
                cache.amountToAdd
            );
        }
   ...
}

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/protocol/ParticlePositionManager.sol#L170-L173

    function openPosition(
        DataStruct.OpenPositionParams calldata params
    ) public override nonReentrant returns (uint96 lienId, uint256 collateralTo) {
        if (params.liquidity == 0) revert Errors.InsufficientBorrow();

        // local cache to avoid stack too deep
        DataCache.OpenPositionCache memory cache;

        // 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
        );
      ...
   }

This causes the operations to have minimal slippage protection when involving decreasing or increasing liquidity

Tools Used

Manual review

When it is possible, allow user to provide amount0Min and amount1Min

Assessed type

Uniswap

#0 - c4-judge

2023-12-21T21:38:58Z

0xleastwood marked the issue as duplicate of #2

#1 - c4-judge

2023-12-23T23:01:21Z

0xleastwood marked the issue as satisfactory

Findings Information

🌟 Selected for report: immeas

Also found by: adriro, bin2chen, ladboy233, said

Labels

bug
downgraded by judge
grade-a
QA (Quality Assurance)
duplicate-23
Q-05

Awards

Data not available

External Links

Lines of code

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/libraries/Base.sol#L183 https://github.com/code-423n4/2023-12-particle/blob/main/contracts/libraries/Base.sol#L326

Vulnerability details

Impact

slot0 is the most recent data point that can be manipulated trough swap and flash loan. Operations that rely in this data are susceptible to price manipulation attack.

Proof of Concept

It can be observed that slot0 price is used when calculating getRequiredRepay, this will calculate amount need to be repaid when closing or liquidating traders position.

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/libraries/Base.sol#L163-L192

   function getRequiredRepay(
        uint128 liquidity,
        uint256 tokenId
    ) internal view returns (uint256 amount0, uint256 amount1) {
        DataCache.RepayCache memory repayCache;
        (
            ,
            ,
            repayCache.token0,
            repayCache.token1,
            repayCache.fee,
            repayCache.tickLower,
            repayCache.tickUpper,
            ,
            ,
            ,
            ,

        ) = UNI_POSITION_MANAGER.positions(tokenId);
        IUniswapV3Pool pool = IUniswapV3Pool(UNI_FACTORY.getPool(repayCache.token0, repayCache.token1, repayCache.fee));
>>>     (repayCache.sqrtRatioX96, , , , , , ) = pool.slot0();
        repayCache.sqrtRatioAX96 = TickMath.getSqrtRatioAtTick(repayCache.tickLower);
        repayCache.sqrtRatioBX96 = TickMath.getSqrtRatioAtTick(repayCache.tickUpper);
        (amount0, amount1) = LiquidityAmounts.getAmountsForLiquidity(
            repayCache.sqrtRatioX96,
            repayCache.sqrtRatioAX96,
            repayCache.sqrtRatioBX96,
            liquidity
        );
    }

Traders can leverage flash loan to sandwich the operation so it become profitable for them when calculating the amount that need to be provided back to the LPs liquidity position.

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/protocol/ParticlePositionManager.sol#L410-L439

    function _closePosition(
        DataStruct.ClosePositionParams calldata params,
        DataCache.ClosePositionCache memory cache,
        Lien.Info memory lien,
        address borrower
    ) internal {
        // optimistically use the input numbers to swap for repay
        /// @dev amountSwap overspend will be caught by refundWithCheck step in below
        (cache.amountSpent, cache.amountReceived) = Base.swap(
            cache.tokenFrom,
            cache.tokenTo,
            params.amountSwap,
            0, /// @dev we check cache.amountReceived is sufficient to repay LP in below
            DEX_AGGREGATOR,
            params.data
        );

        // based on borrowed liquidity, compute the required return amount
        /// @dev the from-to swapping direction is reverted compared to openPosition
>>>     (cache.amountToAdd, cache.amountFromAdd) = Base.getRequiredRepay(lien.liquidity, lien.tokenId);
        if (!lien.zeroForOne) (cache.amountToAdd, cache.amountFromAdd) = (cache.amountFromAdd, cache.amountToAdd);

        // the liquidity to add must be no less than the available amount
        /// @dev the max available amount contains the tokensOwed, will have another check in below at refundWithCheck
        if (
            cache.amountFromAdd > cache.collateralFrom + cache.tokenFromPremium - cache.amountSpent ||
            cache.amountToAdd > cache.amountReceived + cache.tokenToPremium
        ) {
            revert Errors.InsufficientRepay();
        }

        // add liquidity back to borrower
        if (lien.zeroForOne) {
>>>         (cache.liquidityAdded, cache.amountToAdd, cache.amountFromAdd) = LiquidityPosition.increaseLiquidity(
                cache.tokenTo,
                cache.tokenFrom,
                lien.tokenId,
                cache.amountToAdd,
                cache.amountFromAdd
            );
        } else {
>>>         (cache.liquidityAdded, cache.amountFromAdd, cache.amountToAdd) = LiquidityPosition.increaseLiquidity(
                cache.tokenFrom,
                cache.tokenTo,
                lien.tokenId,
                cache.amountFromAdd,
                cache.amountToAdd
            );
        }
   ...
}

Tools Used

Manual review.

Use TWAP for the price inside the operation.

Assessed type

Uniswap

#0 - c4-judge

2023-12-21T22:22:11Z

0xleastwood marked the issue as duplicate of #23

#1 - romeroadrian

2023-12-22T17:21:01Z

The LP owner is interesting in getting the liquidity back, the underlying amounts of each token are meaningless to this operation. The attacker only looses flash loan and trading fees.

#2 - c4-judge

2023-12-23T19:34:58Z

0xleastwood changed the severity to QA (Quality Assurance)

#3 - c4-judge

2023-12-26T11:31:37Z

0xleastwood marked the issue as grade-a

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter