code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/protocol/pool/PoolApeStaking.sol#L428 https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/protocol/pool/PoolApeStaking.sol#L439 https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/protocol/libraries/logic/GenericLogic.sol#L74

Vulnerability details

Impact

Pending Apecoin staking reward does not count towards the account health

Proof of Concept

The account health factor is important because it is used to determine how much user can borrow and when the price drops, it is used to determine if the liquidation is allowed.

Let us look into the health factor calculation in GenericLogic


        CalculateUserAccountDataVars memory vars;

        while (vars.i < params.reservesCount) {
            if (!params.userConfig.isUsingAsCollateralOrBorrowing(vars.i)) {
                unchecked {
                    ++vars.i;
                }
                continue;
            }

            vars.currentReserveAddress = reservesList[vars.i];

            if (vars.currentReserveAddress == address(0)) {
                unchecked {
                    ++vars.i;
                }
                continue;
            }

            DataTypes.ReserveData storage currentReserve = reservesData[
                vars.currentReserveAddress
            ];

            vars.reserveConfiguration = currentReserve.configuration;

            (
                vars.ltv,
                vars.liquidationThreshold,
                vars.liquidationBonus,
                vars.decimals,

            ) = currentReserve.configuration.getParams();

            unchecked {
                vars.assetUnit = 10**vars.decimals;
            }

            vars.xTokenAddress = currentReserve.xTokenAddress;

            if (
                vars.reserveConfiguration.getAssetType() ==
                DataTypes.AssetType.ERC20
            ) {
                vars.assetPrice = _getAssetPrice(
                    params.oracle,
                    vars.currentReserveAddress
                );

                if (
                    (vars.liquidationThreshold != 0) &&
                    params.userConfig.isUsingAsCollateral(vars.i)
                ) {
                    vars.userBalanceInBaseCurrency = _getUserBalanceForERC20(
                        params.user,
                        currentReserve,
                        vars.xTokenAddress,
                        vars.assetUnit,
                        vars.assetPrice
                    );

                    vars.payableDebtByERC20Assets += vars
                        .userBalanceInBaseCurrency
                        .percentDiv(vars.liquidationBonus);

                    vars.liquidationThreshold =
                        vars.userBalanceInBaseCurrency *
                        (vars.liquidationThreshold);
                    vars.avgLtv += vars.userBalanceInBaseCurrency * vars.ltv;

                    vars.totalCollateralInBaseCurrency += vars
                        .userBalanceInBaseCurrency;

                    if (vars.ltv == 0) {
                        vars.hasZeroLtvCollateral = true;
                    }

                    vars.avgLiquidationThreshold += vars.liquidationThreshold;
                }

                if (params.userConfig.isBorrowing(vars.i)) {
                    vars.totalDebtInBaseCurrency += _getUserDebtInBaseCurrency(
                        params.user,
                        currentReserve,
                        vars.assetPrice,
                        vars.assetUnit
                    );
                }
            } else {
                if (
                    (vars.liquidationThreshold != 0) &&
                    params.userConfig.isUsingAsCollateral(vars.i)
                ) {
                    vars.xTokenType = INToken(vars.xTokenAddress)
                        .getXTokenType();
                    if (vars.xTokenType == XTokenType.NTokenUniswapV3) {
                        (
                            vars.userBalanceInBaseCurrency,
                            vars.ltv,
                            vars.liquidationThreshold
                        ) = _getUserBalanceForUniswapV3(
                            reservesData,
                            params,
                            vars
                        );
                    } else {
                        vars
                            .userBalanceInBaseCurrency = _getUserBalanceForERC721(
                            params,
                            vars
                        );

                        vars.liquidationThreshold =
                            vars.userBalanceInBaseCurrency *
                            vars.liquidationThreshold;

                        if (vars.ltv == 0) {
                            vars.hasZeroLtvCollateral = true;
                        }

                        vars.ltv = vars.userBalanceInBaseCurrency * vars.ltv;
                    }

                    vars.avgERC721LiquidationThreshold += vars
                        .liquidationThreshold;
                    vars.totalERC721CollateralInBaseCurrency += vars
                        .userBalanceInBaseCurrency;
                    vars.totalCollateralInBaseCurrency += vars
                        .userBalanceInBaseCurrency;
                    vars.avgLtv += vars.ltv;
                    vars.avgLiquidationThreshold += vars.liquidationThreshold;
                }
            }

            unchecked {
                ++vars.i;
            }
        }

        unchecked {
            vars.avgLtv = vars.totalCollateralInBaseCurrency != 0
                ? vars.avgLtv / vars.totalCollateralInBaseCurrency
                : 0;
            vars.avgLiquidationThreshold = vars.totalCollateralInBaseCurrency !=
                0
                ? vars.avgLiquidationThreshold /
                    vars.totalCollateralInBaseCurrency
                : 0;

            vars.avgERC721LiquidationThreshold = vars
                .totalERC721CollateralInBaseCurrency != 0
                ? vars.avgERC721LiquidationThreshold /
                    vars.totalERC721CollateralInBaseCurrency
                : 0;
        }

        vars.healthFactor = (vars.totalDebtInBaseCurrency == 0)
            ? type(uint256).max
            : (
                vars.totalCollateralInBaseCurrency.percentMul(
                    vars.avgLiquidationThreshold
                )
            ).wadDiv(vars.totalDebtInBaseCurrency);

        vars.erc721HealthFactor = (vars.totalDebtInBaseCurrency == 0 ||
            vars.payableDebtByERC20Assets >= vars.totalDebtInBaseCurrency)
            ? type(uint256).max
            : (
                vars.totalERC721CollateralInBaseCurrency.percentMul(
                    vars.avgERC721LiquidationThreshold
                )
            ).wadDiv(
                    vars.totalDebtInBaseCurrency - vars.payableDebtByERC20Assets
                );

        return (
            vars.totalCollateralInBaseCurrency, //a 
            vars.totalERC721CollateralInBaseCurrency, // b
            vars.totalDebtInBaseCurrency, // c
            vars.avgLtv, // d
            vars.avgLiquidationThreshold, // e
            vars.avgERC721LiquidationThreshold, // f
            vars.payableDebtByERC20Assets, // g
            vars.healthFactor, // 
            vars.erc721HealthFactor, // h
            vars.hasZeroLtvCollateral // i
        );
    }

this function is very long, but on the high level, what is does is that:

loop over all ERC20 collateral and calculate the borrow amount, the asset worth. loop over ERC721 token including Uniswap V3 NFT and other NFT supported.

the list of NFT supported according to the documentation is:

https://docs.para.space/para-space/introduction-to-paraspace/supplying-nfts/nft-utility-and-delegated-rights#what-other-benefits-does-paraspace-offer-to-nft-holders

Bored Apt, MoonBird, CryptoPunk, Mutant Ape, Doodles, Otherdeed, Clone x and meetbits.

The parapspace also support ape coin staking optimization.

but here is the issue: the code above that calculate the account health factor does not count the pending apecoin staking reward.

When apecoin launch the staking, this happens:

https://twitter.com/peckshieldalert/status/1599961388298166272

basically user stake their ape with ape coin for reward and they create a sell order to sell the NFT in NFT marketplace.

Another user happily step in, he first borrow flashloan, buy the NFT, claim the ape coin, then sold apecoin + the NFT for profits and repay the flashloan. It is user that create the sell order suffers: he loses the apecoin and the staking reward.

Now consider this case in the current paraspace account health factor implementation:

A User have a bored APE NFT, worth 80 ETH
He stakes the ape coin for his NFT, the ape coin staked worth 20 ETH.
The User use the bored APE NFT as collateral in the paraspace system and borrow 30 ETH.
The bored APE NFT price drops to 10 ETH, the system thinks the loan backed by the NFT is bad debt and the NFT is liquidated.
However, when the APE NFT worth NFT, his staked apecoin + the staking reward can worth 50 ETH, without counting the pending staking reward of the apecoin into the account health factor, the user's NFT is liquidated and the user lose his NFT + the apecoin.