Particle Leverage AMM Protocol Invitational - ladboy233's results

Lines of code

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

Vulnerability details

Impact

position cannot be liquidated if the borrower is blocklisted

Proof of Concept

When the position is closed, the function _closePosition is called

// execute actual position closing
_closePosition(params, cache, lien, msg.sender);

in this case, the msg.sender is the borrower who open the position

the close position logic involves refund logic

  // 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.

while this refund logic works ok when borrower intends to close his position

the same _closePosition runs, the codes tries to close the position for borrower

// if borrower is blocklisted, cannot liquidate
_closePosition(params, closeCache, lien, borrower);

now we have a problem

https://github.com/d-xo/weird-erc20?tab=readme-ov-file#tokens-with-blocklists

Some tokens (e.g. USDC, USDT) have a contract level admin controlled address blocklist. If an address is blocked, then transfers to and from that address are forbidden.

Malicious or compromised token owners can trap funds in a contract by adding the contract address to the blocklist. This could potentially be the result of regulatory action against the contract itself, against a single user of the contract (e.g. a Uniswap LP), or could also be a part of an extortion attempt against users of the blocked contract.

assume the borrower who open the position is not blocklisted, so he can borrow the liquidity out

then he transfer the liquidity (borrowed fund) to another account and make trade

then the original borrower account is blocklisted

the liquiation will revert in refund flow when the code attempts to send the token to the blocklisted borrower

  function refund(address recipient, address token, uint256 amountExpected, uint256 amountActual) internal {
        if (amountExpected > amountActual) {
            TransferHelper.safeTransfer(token, recipient, amountExpected - amountActual);
        }
    }

    /**
     * @notice Helper function to refund a token, with additional check that amountActual must not exceed amountExpected
     * @param recipient the address to receive the refund
     * @param token address of token to potentially refund
     * @param amountExpected amount of token0 expected to spend
     * @param amountActual amount of token0 actually spent
     */
    function refundWithCheck(address recipient, address token, uint256 amountExpected, uint256 amountActual) internal {
        if (amountActual > amountExpected) revert Errors.OverRefund();
        refund(recipient, token, amountExpected, amountActual);
    }

Failed to liquidate the position make original lender lose fund

Tools Used

Manual Review

Recommended Mitigation Steps

let the original borrower claim the fund if their position are liquidated and avoid send token out during the liquidation flow

Assessed type

Token-Transfer

Lines of code

https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/protocol/ParticlePositionManager.sol#L399 https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/libraries/Base.sol#L55

Vulnerability details

Impact

Lack of input validation for ClosePositionParams.amountSwap results in theft of fund

Proof of Concept

ParticlePositionManager.sol hold two part of fund

1. the contract hold premium added by borrower
2. the contract hold protocol fee before protocol withdraw it

but because there is lack of input validation for parameter ClosePositionParams.amountSwap when close the position,

the premium and protocol fee that is stored in the PositionManager can be stolen directly

the struct of ClosePositionParams is below

    struct ClosePositionParams {
        uint96 lienId;
        uint256 amountSwap;
        bytes data;
    }

the parameter amountSwap is user controlled when liquidation or close the position

function closePosition(DataStruct.ClosePositionParams calldata params) external override nonReentrant {

then in the function _closePosition

    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();
        }

the token is swapped to repay LP (lender)

the code only ensure the swapped out amount can repay the LP

but does not the validate parameter amountSwap when swapping

    function swap(
        address tokenFrom,
        address tokenTo,
        uint256 amountFrom,
        uint256 amountToMinimum,
        address dexAggregator,
        bytes calldata data
    ) internal returns (uint256 amountSpent, uint256 amountReceived) {
        uint256 balanceFromBefore = IERC20(tokenFrom).balanceOf(address(this));
        uint256 balanceToBefore = IERC20(tokenTo).balanceOf(address(this));

        if (amountFrom > 0) {
            ///@dev only allow amountFrom of tokenFrom to be spent by the DEX aggregator
            TransferHelper.safeApprove(tokenFrom, dexAggregator, amountFrom);
            // solhint-disable-next-line avoid-low-level-calls
            (bool success, ) = dexAggregator.call(data);
            if (!success) revert Errors.SwapFailed();
            TransferHelper.safeApprove(tokenFrom, dexAggregator, 0);
        }

        amountSpent = balanceFromBefore - IERC20(tokenFrom).balanceOf(address(this));
        amountReceived = IERC20(tokenTo).balanceOf(address(this)) - balanceToBefore;

        if (amountReceived < amountToMinimum) revert Errors.InsufficientSwap();
    }

as we can see

suppose the user wants to close the position and needs to repay LP 100 USDC

the contract hold 1 WETH (other's user premium + protocol fee)

the user can use 1inch to swap out WETH to 2200 USDC and then transfer the 100 USDC to the position manager and steal the rest 2100 USDC

for example, if the 1inch v4 is used to pull an arbitrary amount of funds from the caller and execute arbitrary call

https://polygonscan.com/address/0x1111111254fb6c44bAC0beD2854e76F90643097d#codeL2309-2321

even if 1inch v5 is used, attacker can still sweep fund out and only ensure the minimum out of fund is repaid and steal the rest fund after aggreagtor executes multi-hop swap

Tools Used

Manual Review

Recommended Mitigation Steps

make sure validate the ClosePositionParams.amountSwap, and the code should ensure the user transfer the ClosePositionParams.amountSwap fund into the contract before close the position

Assessed type

Invalid Validation

        if (!srcETH) {
            if (permit.length > 0) {
                srcToken.safePermit(permit);
            }
            srcToken.safeTransferFrom(msg.sender, desc.srcReceiver, desc.amount);
        }

        _execute(executor, msg.sender, desc.amount, data);

    function _execute(
        IAggregationExecutor executor,
        address srcTokenOwner,
        uint256 inputAmount,
        bytes calldata data
    ) private {
        bytes4 executeSelector = executor.execute.selector;
        /// @solidity memory-safe-assembly
        assembly {  // solhint-disable-line no-inline-assembly
            let ptr := mload(0x40)

            mstore(ptr, executeSelector)
            mstore(add(ptr, 0x04), srcTokenOwner)
            calldatacopy(add(ptr, 0x24), data.offset, data.length)
            mstore(add(add(ptr, 0x24), data.length), inputAmount)

            if iszero(call(gas(), executor, callvalue(), ptr, add(0x44, data.length), 0, 0)) {
                returndatacopy(ptr, 0, returndatasize())
                revert(ptr, returndatasize())
            }
        }
    }

Lines of code

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

Vulnerability details

Impact

Only ensure the Lp is repaid when close the position invites MEV bot

Proof of Concept

in the function _closePosition

    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();
        }

the token is swapped to repay LP (lender)

the code only ensure the swapped out amount can repay the LP

However, this is not sufficient from protecting MEV frontrunning

suppose the repayment amonut is 100 USDC

the user swap from 1 WETH to USDC

the expect output without frontrunning is

1 WETH get swapped to 2200 USDC

100 UDSC is used to add liquidity to repay the LP

the borrower get 2100 USDC refund (minues the interest)

however, basically it means the minOutput slippage protection is set to 100 USDC,

the MEV bot can still frontrun the transaction to steal fund that belongs to the refund

basically ensure LP is repaid and set LP repayment amount does not protect MEV bot frunning

we can use the example above

1. WETH get swapped to should be swapped to 2200 UDSC, but because of frontrunning, output amount is only 1800 USDC
2. 100 UDSC is used to add liquidity to repay the LP
3. user take the refund 1700 UDSC, MEV bot steal 400 USDC

Tools Used

Manual Review

Recommended Mitigation Steps

revisit slippage control when position is closed, user should able to supply the parameter amountToMinimum so parameter amountToMinimum is not 0

        /// @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
        );

if transaction output amount does not cover the repayment, transaction revert, otherwise, refund excessive amount leftover fund

Assessed type

MEV

Lines of code

https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/protocol/ParticlePositionManager.sol#L581 https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/protocol/ParticlePositionManager.sol#L365

Vulnerability details

Impact

LOAN_TERM adjustment can impact ongoing leverage position and can be backrun to cause immediate liquidation

Proof of Concept

According to the documentation, the LOAN_TERM will be set to 7 days

yet admin perserve the right to adjust the LOAN_TERM any time can calling the function updateLoanTerm

    function updateLoanTerm(uint256 loanTerm) external override onlyOwner {
        if (loanTerm > _LOAN_TERM_MAX) revert Errors.InvalidValue();
        LOAN_TERM = loanTerm;
        emit UpdateLoanTerm(loanTerm);
    }

this parameter LOAN_TERM is used to validate whethter a leveraged position is subject to liquidation (user's premium is lost to liquidator)

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

for example, if in the beginning, LOAN_TERM is 7 days,

lien.startTime + LOAN_TERM < block.timestamp

return false,

user has open a leverage position and intend to close position in 6 days

5 and half day passes

but admin adjust the LOAN_TERM to 5 days,

user's position is subject to liquidation immediately

in the worst case, a user can monitor the decrease of LOAN_TERM to immediate liquidate user's position and leave user no time to add more premium

Tools Used

Manual Review

Recommended Mitigation Steps

recommend snapshot the loan term and store it in the LIEN struct to avoid LOAN_TERM decrease impact ongoing leverage position

Assessed type

MEV

Lines of code

https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/protocol/ParticlePositionManager.sol#L365 https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/libraries/Lien.sol#L46

Vulnerability details

Impact

Borrower can frontrun to block liquidation by adding a tiny premium to update the loan start time

Proof of Concept

Please note the liquidation considition check

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

if

cutOffTime > startTime AND currentTime > startTime + LOAN_TERM

the loan should be subject to liquidation

however, borrower can keep enough premium to infinitely roll over the LOAN_TERM can adding a tiny amount of premium (even 1 wei of token or 0 amount of premium)

the borrower needs to call addPremium

this would trigger

liens.updatePremium(
	lienKey,
	uint24(((token0Premium + premium0) * Base.BASIS_POINT) / collateral0),
	uint24(((token1Premium + premium1) * Base.BASIS_POINT) / collateral1)
);

which calls this line of code

    function updatePremium(
        mapping(bytes32 => Info) storage self,
        bytes32 lienKey,
        uint24 token0PremiumPortion,
        uint24 token1PremiumPortion
    ) internal {
        self[lienKey].token0PremiumPortion = token0PremiumPortion;
        self[lienKey].token1PremiumPortion = token1PremiumPortion;
        self[lienKey].startTime = uint32(block.timestamp);
    }

the lien start time is reset to block.timestamp

so basically, the third condition

			(lien.startTime < lps.getRenewalCutoffTime(lien.tokenId) &&
				lien.startTime + LOAN_TERM < block.timestamp))

will never to true

then Borrower can frontrun liquidator to block liquidation by adding a tiny premium amount to extend the loan start time and roll over the loan term infnitely

the impact is severe, this borrower can make sure lender never get NFT position orignal liquidity back as long as the premium is sufficient

Tools Used

Manual Review

Recommended Mitigation Steps

one of the option is do not update lien start time when user add new premium

Assessed type

MEV

        if (lps.getRenewalCutoffTime(lien.tokenId) > lien.startTime) revert Errors.RenewalDisabled();

Lines of code

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

Vulnerability details

Impact

Position can be opened even when the position manger does not hold the Uniswap V3 Position NFT

Proof of Concept

As long as user approve the PositionManager for as a operator of their NFT position

even the particle position manager does not hold the Uniswap V3 NFT

anyone can open the position on behalf of the original NFT owner

the original owner may incapable of calling reclaimLiquidity and the borrower just open the position to reduce the original liquidity, as long as he maintain sufficient premium, he will never be liquidated

or even the original own is capable of calling reclaim liquidity, the borrower still capable of leverage trading for LOAN_TERM length while forcefully reduce the liquidity of original nft

this is because when open the position, the code does not check if the particile position manager hold the Uniswap V3 NFT

the external call to decrease liquidity and collect the token is subject to the check

https://etherscan.io/address/0xc36442b4a4522e871399cd717abdd847ab11fe88#code#F1#L184

   modifier isAuthorizedForToken(uint256 tokenId) {
        require(_isApprovedOrOwner(msg.sender, tokenId), 'Not approved');
        _;
    }

while user approve ParticlePositionManager is not a normal flow

user approve particlePositionManager does not signal user wants to borrow the fund

only when user transfer the NFT into the ParticlePositionManager, borrower can borrow with lender's implicit permision

Tools Used

Manunal Review

Recommended Mitigation Steps

recommendation validting the particle position mange hold the position nft when position is opened

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/libraries/Base.sol#L132 https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/protocol/ParticlePositionManager.sol#L247 https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/protocol/ParticlePositionManager.sol#L118

Vulnerability details

Impact

Malicious lender can manipulate the fee to force borrower pay high premium

Proof of Concept

When user create a position, the fee is snapshot is queryed from uniswap v3 position manager when preparing leverage

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

and stored in the lien struct

// prepare data for swap
(
	cache.tokenFrom,
	cache.tokenTo,
	cache.feeGrowthInside0LastX128,
	cache.feeGrowthInside1LastX128,
	cache.collateralFrom,
	collateralTo
) = Base.prepareLeverage(params.tokenId, params.liquidity, params.zeroForOne);

then stored in the liens struct

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

then when the position is closed, the premium interested paid depends on the spot value of the fee

// obtain the position's latest FeeGrowthInside after increaseLiquidity
(, , , , , , , , cache.feeGrowthInside0LastX128, cache.feeGrowthInside1LastX128, , ) = Base
	.UNI_POSITION_MANAGER
	.positions(lien.tokenId);

// caculate the amounts owed since last fee collection during the borrowing period
(cache.token0Owed, cache.token1Owed) = Base.getOwedFee(
	cache.feeGrowthInside0LastX128,
	cache.feeGrowthInside1LastX128,
	lien.feeGrowthInside0LastX128,
	lien.feeGrowthInside1LastX128,
	lien.liquidity
);

if the fee increased during the position opening time, the premium is used to cover the fee to make sure there are incentive for lenders to deposit V3 NFT as lender

as the comments points out

// 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

However, because the fee amount is queried from position manager in spot value,

malicious lender increase the liquidity to that ticker range and then can swap back and forth between ticker range to inflate the fee amount to force borrower pay high fee / high premium

while the lender has to pay the gas cost + uniswap trading fee

but if the lender add more liquidity to nft's ticker ragne, he can collect the majority of the liqudity fee + collect high premium from borrower (or forcefully liquidated user to take premium)

Tools Used

Manual Review

Recommended Mitigation Steps

it is recommend to cap the premium interest payment instead of query spot fee amount to avoid swap fee manipulation

Assessed type

Token-Transfer

Lines of code

https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/libraries/LiquidityPosition.sol#L195 https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/libraries/LiquidityPosition.sol#L258

Vulnerability details

Impact

AddLiquidity and decreaseLiquidity missing slippage protection

Proof of Concept

When user mint NFT add liquidity

user can specify two parameter, params.amount0Min and params.amount1Min

        // mint the position
        (tokenId, liquidity, amount0Minted, amount1Minted) = Base.UNI_POSITION_MANAGER.mint(
            INonfungiblePositionManager.MintParams({
                token0: params.token0,
                token1: params.token1,
                fee: params.fee,
                tickLower: params.tickLower,
                tickUpper: params.tickUpper,
                amount0Desired: params.amount0ToMint,
                amount1Desired: params.amount1ToMint,
                amount0Min: params.amount0Min,
                amount1Min: params.amount1Min,
                recipient: address(this),
                deadline: block.timestamp
            })
        );

if the minted amount is too small, transaction revert in this check in Uniswap position manager when addling liquidity

(amount0, amount1) = pool.mint(
	params.recipient,
	params.tickLower,
	params.tickUpper,
	liquidity,
	abi.encode(MintCallbackData({poolKey: poolKey, payer: msg.sender}))
);

require(amount0 >= params.amount0Min && amount1 >= params.amount1Min, 'Price slippage check');

However, when addling liquidity, the parameter amount0Min and amount1Min is set to 0

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

as Uniswap V3 docs highlight

https://docs.uniswap.org/contracts/v3/guides/providing-liquidity/mint-a-position#calling-mint

We set amount0Min and amount1Min to zero for the example - but this would be a vulnerability in production. A function calling mint with no slippage protection would be vulnerable to a frontrunning attack designed to execute the mint call at an inaccurate price.

if the user transaction suffer from frontrunning, a much less amount of token can be minted

same issue happens when user decrease liquidity

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

the amonut0 and amount1Min are set to 0

when MEV bot frontruns the decrease liquidity, much less amount0 and amount1 are released

Tools Used

Manual Review

Recommended Mitigation Steps

recommend do not hardcode slippage protection parameter amount0Min and amount1Min to 0 when increase liquidity or decrease liquidity

Assessed type

Token-Transfer

consider batch withdraw protocol fee

the current withdraw fee function is too gas inefficient

it is likely a wide range of Uniswap V3 NFT will be deposit into the Position manager and the Position Manager will accumulate a lots of fee but settled in different type of ERC20 token

if the protocol has to call the function

function withdrawTreasury(address token, address recipient) external override onlyOwner nonReentrant {
	uint256 withdrawAmount = _treasury[token];
	if (withdrawAmount > 0) {
		if (recipient == address(0)) {
			revert Errors.InvalidRecipient();
		}
		_treasury[token] = 0;
		TransferHelper.safeTransfer(token, recipient, withdrawAmount);
		emit WithdrawTreasury(token, recipient, withdrawAmount);
	}
}

every time to withdraw the token

it is not gas efficient

it is recommend to take the token address array as parameter so the protocol can withdraw multiple token fee in one single transaction

uniswap v3 can add more tiers

the code assumes that there will only be three fee tiers

however, the uniswap protocol governance is capable of adding more fee tiers,

https://docs.uniswap.org/concepts/protocol/fees#pool-fees-tiers

Uniswap v3 introduces multiple pools for each token pair, each with a different swapping fee. Liquidity providers may initially create pools at three fee levels: 0.05%, 0.30%, and 1%. More fee levels may be added by UNI governance, e.g. the 0.01% fee level added by this governance proposal in November 2021, as executed here.

then after the new fee tier is added, the function getDeepPool may not return the pool with deepest liquidity

    function getDeepPool(address token0, address token1) external view returns (address deepPool) {
        uint24[3] memory feeTiers = [uint24(500), uint24(3000), uint24(10000)];
        uint128 maxLiquidity = 0;

        for (uint256 i = 0; i < feeTiers.length; i++) {
            address poolAddress = Base.UNI_FACTORY.getPool(token0, token1, feeTiers[i]);
            if (poolAddress != address(0)) {
                IUniswapV3Pool pool = IUniswapV3Pool(poolAddress);
                uint128 liquidity = pool.liquidity();
                if (liquidity > maxLiquidity) {
                    maxLiquidity = liquidity;
                    deepPool = poolAddress;
                }
            }
        }
    }

it is recommend to make the feeTiers configurable

Should expose whether position the is liquidable as a view function in ParticleIinfoReader.sol

In ParticleInfoReader.sol

there is no view function to validate and expose if a single position is liquidable

it is recommendated to wrap the liquidation check to a view function and expose whether the position is liquidable for potential liquidator

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

Avoid hardcode address

THe Uniswap V3 Position manager address is hardcoded ,

however, if the protocol wants to deploy in multiple EVM network

the Uniswap V3 Position Manager address maybe different

https://docs.uniswap.org/contracts/v3/reference/deployments

for example, the Uniswap V3 Position Manager address is different from ethereum mainnet blockchain and base network blockchain

it is recommend to pass in the position manager as a parameter and pass the address into the constructor

Lack of method to transfer NFT out in ParticlePositionManager.sol

In ParticlePositionManager.sol

User can mint and create Uniswap V3 Position NFT directly in Position Manager,

user can increase liquidity, decrease liquidity or harvest the claim the reawrd

however once the Uniswap V3 NFT is transferred to the contract,

there is lack of function to transfer the NFT out

the bigger problem is as long as user's nft is in the particle position contract, anyone can borrow the fund out even while original owner does not want the fund to be borrowed

so if the original nft does not want others to borrow fund, he cannot do that so he will choose to only remove liquidity but not add liquidity once he does not want to borrow any more

and there is lack of function to burn the NFT as well

it is recommended to implement the function to transfer V3 NFT out in PositionManager.sol