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

[c4-bot-6]

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

[c4-judge]

0xleastwood marked the issue as duplicate of #26

[c4-judge]

0xleastwood changed the severity to 3 (High Risk)

[c4-judge]

0xleastwood marked the issue as satisfactory