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

2 stars 1 forks source link

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 infinitely #4

Closed c4-bot-8 closed 10 months ago

c4-bot-8 commented 11 months ago

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

c4-judge commented 10 months ago

0xleastwood marked the issue as primary issue

wukong-particle commented 10 months ago

Good discovery, but we think this by design is covered by the renewalCutOffTime. Please note that when interacting with addPremium there is a check

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

If LP calls reclaimLiquidity https://github.com/code-423n4/2023-12-particle/blob/main/contracts/protocol/ParticlePositionManager.sol#L142, trader won't be able to add premium.

Happy to discuss further along this line. If there's still attack angle, please provide a coded PoC. Thanks!

JeffCX commented 10 months ago

Consider the case when the original borrower that hold NFT does not implement the function reclaimLiquidity.

also,

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 infinitely

before lender calls reclaimLiquidity, every time the borrower add a tiny premium, the position will be extended by 7 days

even when the reclaimLiquidity and the addPremium are called at the same time (borrower frontrun liquidator's reclaimLiquidity to add tiny premium),

the loan. term can still be extended by 7 days

mainly because we are using > instead of >=

        if (lps.getRenewalCutoffTime(lien.tokenId) > lien.startTime) revert Errors.RenewalDisabled();
wukong-particle commented 10 months ago

Yes, the > vs >= issue is similar to https://github.com/code-423n4/2023-12-particle-findings/issues/33, we will fix it!

c4-judge commented 10 months ago

0xleastwood marked the issue as duplicate of #33

0xleastwood commented 10 months ago

It should be noted I think this issue was missing some key points that was outlined in #33. I will give partial credit instead.

c4-judge commented 10 months ago

0xleastwood marked the issue as partial-50

c4-judge commented 10 months ago

0xleastwood changed the severity to 2 (Med Risk)

c4-judge commented 10 months ago

0xleastwood marked the issue as full credit

c4-judge commented 10 months ago

0xleastwood marked the issue as satisfactory