code-423n4 / 2022-11-paraspace-findings

7 stars 4 forks source link

Bad debt will likely incur when multiple NFTs are liquidated. #479

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/protocol/libraries/logic/GenericLogic.sol#L394

Vulnerability details

Description

_getUserBalanceForERC721() in GenericLogic gets the value of a user's specific ERC721 xToken. It is later used for determining the account's health factor. In case isAtomicPrice is false such as in ape NTokens, price is calculated using:

    uint256 assetPrice = _getAssetPrice(
        params.oracle,
        vars.currentReserveAddress
    );
    totalValue =
        ICollateralizableERC721(vars.xTokenAddress)
            .collateralizedBalanceOf(params.user) *
        assetPrice;

It is the number of apes multiplied by the floor price returns from _getAssetPrice. The issue is that this calculation does not account for slippage, and is unrealistic. If user's account is liquidated, it is very unlikely that releasing several multiples of precious NFTs will not bring the price down in some significant way.

By performing simple multiplication of NFT count and NFT price, protocol is introducing major bad debt risks and is not as conservative as it aims to be. Collateral value must take slippage risks into account.

Impact

Bad debt will likely incur when multiple NFTs are liquidated.

Tools Used

Manual audit

Recommended Mitigation Steps

Change the calculation to account for slippage when NFT balance is above some threshold.

c4-judge commented 1 year ago

dmvt marked the issue as primary issue

c4-judge commented 1 year ago

dmvt marked the issue as selected for report

WalidOfNow commented 1 year ago

there's no real issue here. its pretty much saying that the design of the protocol is not good for certain market behaviours. this is more of a suggestion than an issue. on top of that, we actually account for this slippage by choosing low LTV and LT

trust1995 commented 1 year ago

Regardless of the way we look at it, I've established assets are at risk under stated conditions which are not correctly taken into account in the protocol. That seems to meet the bar set for M level submissions.