ParaSpace contest - Franfran's results

Lines of code

https://github.com/code-423n4/2022-11-paraspace/blob/main/paraspace-core/contracts/misc/UniswapV3OracleWrapper.sol#L241-L277

Vulnerability details

Impact

When the squared root of the Uniswap V3 position is calculated from the _getOracleData() function, the price may return a very high number (in the case that the token1 decimals are strictly superior to the token0 decimals). See: https://github.com/code-423n4/2022-11-paraspace/blob/main/paraspace-core/contracts/misc/UniswapV3OracleWrapper.sol#L249-L260

The reason is that at the denominator, the 1E9 (10**9) value is hard-coded, but should take into account the delta between both decimals. As a result, in the case of token1Decimal > token0Decimal, the getAmountsForLiquidity() is going to return a huge value for the amount of token0 and token1 as the user position liquidity.

The getTokenPrice(), using this amount of liquidity to calculate the token price is as its turn going to return a huge value.

Proof of Concept

This POC demonstrates in which case the returned squared root price of the position is over inflated

// SPDX-License-Identifier: UNLISENCED
pragma solidity 0.8.10;

import {SqrtLib} from "../contracts/dependencies/math/SqrtLib.sol";
import "forge-std/Test.sol";

contract Audit is Test {
    function testSqrtPriceX96() public {
        // ok
        uint160 price1 = getSqrtPriceX96(1e18, 5 * 1e18, 18, 18);

        // ok
        uint160 price2 = getSqrtPriceX96(1e18, 5 * 1e18, 18, 9);

        // Has an over-inflated squared root price by 9 magnitudes as token0Decimal < token1Decimal
        uint160 price3 = getSqrtPriceX96(1e18, 5 * 1e18, 9, 18);
    }

    function getSqrtPriceX96(
        uint256 token0Price,
        uint256 token1Price,
        uint256 token0Decimal,
        uint256 token1Decimal
    ) private view returns (uint160 sqrtPriceX96) {
        if (oracleData.token1Decimal == oracleData.token0Decimal) {
            // multiply by 10^18 then divide by 10^9 to preserve price in wei
            sqrtPriceX96 = uint160(
                (SqrtLib.sqrt(((token0Price * (10**18)) / (token1Price))) *
                    2**96) / 1E9
            );
        } else if (token1Decimal > token0Decimal) {
            // multiple by 10^(decimalB - decimalA) to preserve price in wei
            sqrtPriceX96 = uint160(
                (SqrtLib.sqrt(
                    (token0Price * (10**(18 + token1Decimal - token0Decimal))) /
                        (token1Price)
                ) * 2**96) / 1E9
            );
        } else {
            // multiple by 10^(decimalA - decimalB) to preserve price in wei then divide by the same number
            sqrtPriceX96 = uint160(
                (SqrtLib.sqrt(
                    (token0Price * (10**(18 + token0Decimal - token1Decimal))) /
                        (token1Price)
                ) * 2**96) / 10**(9 + token0Decimal - token1Decimal)
            );
        }
    }
}

Tools Used

Recommended Mitigation Steps

        if (oracleData.token1Decimal == oracleData.token0Decimal) {
            // multiply by 10^18 then divide by 10^9 to preserve price in wei
            oracleData.sqrtPriceX96 = uint160(
                (SqrtLib.sqrt(
                    ((oracleData.token0Price * (10**18)) /
                        (oracleData.token1Price))
                ) * 2**96) / 1E9
            );
        } else if (oracleData.token1Decimal > oracleData.token0Decimal) {
            // multiple by 10^(decimalB - decimalA) to preserve price in wei
            oracleData.sqrtPriceX96 = uint160(
                (SqrtLib.sqrt(
                    (oracleData.token0Price *
                        (10 **
                            (18 +
                                oracleData.token1Decimal -
                                oracleData.token0Decimal))) /
                        (oracleData.token1Price)
                ) * 2**96) /
                    10 **
                        (9 +
                            oracleData.token1Decimal -
                            oracleData.token0Decimal)
            );
        } else {
            // multiple by 10^(decimalA - decimalB) to preserve price in wei then divide by the same number
            oracleData.sqrtPriceX96 = uint160(
                (SqrtLib.sqrt(
                    (oracleData.token0Price *
                        (10 **
                            (18 +
                                oracleData.token0Decimal -
                                oracleData.token1Decimal))) /
                        (oracleData.token1Price)
                ) * 2**96) /
                    10 **
                        (9 +
                            oracleData.token0Decimal -
                            oracleData.token1Decimal)
            );
        }

Lines of code

https://github.com/code-423n4/2022-11-paraspace/blob/main/paraspace-core/contracts/misc/ParaSpaceOracle.sol#L128

Vulnerability details

Impact

Besides the use of the deprecated latestAnswer() function from Chainlink oracles, there is not a sufficient amount of checks to make sure that returned price can be used across the protocol because it can be considered as safe. For example, there is no guarantee that the price is not stale. Additionally, it should make sure that this price is not inferior to 0. According to Chainlink docs, it is encouraged to use latestRoundData() because this function returns everything that is needed in order to check after that the data is correct and can be used more properly. In short, we must make sure that:

• The price is not less than 0
• The round is finished
• The price is not stale

Proof of Concept

Oracle is a central point of the protocol, and can badly influence the users assets price and multiple mechanics of the protocol.

Liquidations

For ERC20: https://github.com/code-423n4/2022-11-paraspace/blob/main/paraspace-core/contracts/protocol/libraries/logic/LiquidationLogic.sol#L678-L681

function _calculateERC20LiquidationParameters(
        DataTypes.ReserveData storage collateralReserve,
        DataTypes.ExecuteLiquidateParams memory params,
        ExecuteLiquidateLocalVars memory superVars
    )
        internal
        view
        returns (
            uint256,
            uint256,
            uint256,
            uint256
        )
    {
        // [...]
        vars.collateralPrice = IPriceOracleGetter(params.priceOracle)
            .getAssetPrice(params.collateralAsset);
        vars.liquidationAssetPrice = IPriceOracleGetter(params.priceOracle)
            .getAssetPrice(params.liquidationAsset);
        // [...]
        if (maxCollateralToLiquidate > vars.userCollateral) {
            vars.actualCollateralToLiquidate = vars.userCollateral;
            vars.actualLiquidationAmount = (
                ((vars.collateralPrice *
                    vars.actualCollateralToLiquidate *
                    vars.liquidationAssetUnit) /
                    (vars.liquidationAssetPrice * vars.collateralAssetUnit))
            ).percentDiv(superVars.liquidationBonus);
        }
        // [...]
        return (
            vars.userCollateral,
            vars.actualCollateralToLiquidate - vars.liquidationProtocolFee,
            vars.actualLiquidationAmount,
            vars.liquidationProtocolFee
        );

For ERC721: https://github.com/code-423n4/2022-11-paraspace/blob/main/paraspace-core/contracts/protocol/libraries/logic/LiquidationLogic.sol#L757

    function _calculateERC721LiquidationParameters(
        DataTypes.ReserveData storage collateralReserve,
        DataTypes.ExecuteLiquidateParams memory params,
        ExecuteLiquidateLocalVars memory superVars
    )
        internal
        view
        returns (
            uint256,
            uint256,
            uint256,
            uint256
        )
    {
        // price of the asset that is used as collateral
        if (INToken(superVars.collateralXToken).getAtomicPricingConfig()) {
            vars.collateralPrice = IPriceOracleGetter(params.priceOracle)
                .getTokenPrice(
                    params.collateralAsset,
                    params.collateralTokenId
                );
        } else {
            vars.collateralPrice = IPriceOracleGetter(params.priceOracle)
                .getAssetPrice(params.collateralAsset);
        }
        // [...]

        // price of the asset the liquidator is liquidating with
        vars.liquidationAssetPrice = IPriceOracleGetter(params.priceOracle)
            .getAssetPrice(params.liquidationAsset);
        // [...]

        uint256 collateralToLiquidate = (vars.collateralPrice *
            vars.liquidationAssetUnit) / vars.liquidationAssetPrice;

        // base currency to convert to liquidation asset unit.
        uint256 globalDebtAmount = (superVars.userGlobalDebt *
            vars.liquidationAssetUnit) / vars.liquidationAssetPrice;

        vars.actualLiquidationAmount = collateralToLiquidate.percentDiv(
            superVars.liquidationBonus
        );

        if (vars.liquidationProtocolFeePercentage != 0) {
            uint256 bonusCollateral = collateralToLiquidate -
                vars.actualLiquidationAmount;

            vars.liquidationProtocolFee = bonusCollateral.percentMul(
                vars.liquidationProtocolFeePercentage
            );

            return (
                vars.userCollateral,
                vars.actualLiquidationAmount + vars.liquidationProtocolFee,
                vars.liquidationProtocolFee,
                globalDebtAmount
            );
        } else {
            return (
                vars.userCollateral,
                vars.actualLiquidationAmount,
                0,
                globalDebtAmount
            );
        }
    }

Which has effect on the executeLiquidateERC20 logic and on the executeLiquidateERC721 similarly.

Tools Used

Manual inspection

Recommended Mitigation Steps

As advised by Chainlink, the code can be updated as such:

(uint80 roundId, int256 answer, /* uint256 startedAt */, uint256 updatedAt, uint80 answeredInRound) = source.latestRoundData();
require(answer > 0, "wrong answer");
require(answeredInRound >= roundId, "stale answer");
require(updatedAt > 0, "incomplete round");
uint256 price = uint256(answer);