code-423n4 / 2023-12-particle-findings

2 stars 1 forks source link

Owners of LPs can be dosed when removing their position #53

Closed c4-bot-5 closed 11 months ago

c4-bot-5 commented 11 months ago

Lines of code

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

Vulnerability details

Summary

LP owners can reclaim liquidity to stop it from being extended for current liens but this doesn't stop from being used in new positions.

Impact

LP owners can signal their intention to pull liquidity by calling reclaimLiquidity(). This function updates the value of renewalCutoffTime in the liquidity position, which works as a barrier to stop current borrowers from extending the use of liquidity. Affected positions may be liquidated after the elapsed loan term:

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

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

However, this only impacts existing positions. Borrowers may be forced to return liquidity to avoid liquidations, but this doesn't stop it from being reclaimed for a new position before the LP owner can decrease it from the position.

Note that this scenario can happen accidentally or intentionally. An attacker can purposely open positions for a specific tokenId in order to prevent liquidity from being pulled: a simple multicall transaction can close the position only to open it again and renew the loan period, forcing the LP owner to call reclaimLiquidity() again, at which point the attacker can also repeat the process near the end of the loan term. Also, market conditions can lead to repeatedly opening positions due to favorable conditions. An existing borrower may return liquidity, but other players in the market can immediately open new positions from the returned liquidity before the LP owners gets a chance to decrease the liquidity from their position.

Recommendation

Add a new setting to let the owner of the LP configure to stop accepting new positions from their tokenId. This setting can be stored in the LiquidityPosition.Info struct. Prevent new positions from being created if this flag is enabled, so that the owner of LP can eventually collect all the liquidity after existing positions expire.

Assessed type

DoS

c4-judge commented 11 months ago

0xleastwood marked the issue as primary issue

c4-judge commented 11 months ago

0xleastwood marked the issue as duplicate of #35

c4-judge commented 11 months ago

0xleastwood marked the issue as satisfactory