Zero amount token transfers may cause a denial of service during liquidations

[c4-bot-4]

Lines of code

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

Vulnerability details

Summary

Some ERC20 implementations revert on zero value transfers. Since liquidation rewards are based on a fraction of the available position's premiums, this may cause an accidental denial of service that prevents the successful execution of liquidations.

Impact

Liquidations in the LAMM protocol are incentivized by a reward that is calculated as a fraction of the premiums available in the position.

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

348:         // calculate liquidation reward
349:         liquidateCache.liquidationRewardFrom =
350:             ((closeCache.tokenFromPremium) * LIQUIDATION_REWARD_FACTOR) /
351:             uint128(Base.BASIS_POINT);
352:         liquidateCache.liquidationRewardTo =
353:             ((closeCache.tokenToPremium) * LIQUIDATION_REWARD_FACTOR) /
354:             uint128(Base.BASIS_POINT);

These amounts are later transferred to the caller, the liquidator, at the end of the liquidatePosition() function.

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

376:         // reward liquidator
377:         TransferHelper.safeTransfer(closeCache.tokenFrom, msg.sender, liquidateCache.liquidationRewardFrom);
378:         TransferHelper.safeTransfer(closeCache.tokenTo, msg.sender, liquidateCache.liquidationRewardTo);

Reward amounts, liquidationRewardFrom and liquidationRewardTo, can be calculated as zero if tokenFromPremium or tokenToPremium are zero, if the liquidation ratio gets rounded down to zero, or if LIQUIDATION_REWARD_FACTOR is zero.

Coupled with that fact that some ERC20 implementations revert on zero value transfers, this can cause an accidental denial of service in the implementation of liquidatePosition(), blocking certain positions from being liquidated.

Recommendation

Check that the amounts are greater than zero before executing the transfer.

        // reward liquidator
+       if (liquidateCache.liquidationRewardFrom > 0) {
          TransferHelper.safeTransfer(closeCache.tokenFrom, msg.sender, liquidateCache.liquidationRewardFrom);
+       }
+       if (liquidateCache.liquidationRewardTo > 0) {
          TransferHelper.safeTransfer(closeCache.tokenTo, msg.sender, liquidateCache.liquidationRewardTo);
+       }

Assessed type

ERC20

[0xleastwood]

Unlikely token type to even support in the first place. Probably more of a QA issue.

[c4-judge]

0xleastwood marked the issue as primary issue

[wukong-particle]

Agree with the judge. Though we can add a zero check to all transfers to potentially save gas.

[c4-judge]

0xleastwood changed the severity to QA (Quality Assurance)

[romeroadrian]

I believe this is similar to the issue that mentions tokens with blocklists (#31, judged as high) as both of these are non standard (in the strict sense of the standard), though it is of course fair to say that blocklists are more frequent (eg usdc, usdt).

Note that the protocol doesn't have any sort of allow list to control which ERC20 tokens are supported inside the protocol, and anyone can open a position using any Uniswap pool, which also means any token. The main problem here is that liquidations can be blocked after a position is open, that's why I consider the med severity justified.

[0xleastwood]

The difference being that there are little to no tokens supported across all lending platforms which revert on zero token transfer where there are almost always tokens supported with blocklists.

[0xleastwood]

I guess I can bump this up to medium because anyone can LP into a position and protocol liveness should be highlighted here.

[c4-judge]

This previously downgraded issue has been upgraded by 0xleastwood

[c4-judge]

0xleastwood marked the issue as selected for report

[c4-sponsor]

wukong-particle (sponsor) confirmed