code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Description

Paraspace implemented their own Oracle wrapper in ParaSpaceOracle.sol. The important function getAssetPrice() is used by many logic functions like health check.

function getAssetPrice(address asset)
    public
    view
    override
    returns (uint256)
{
    if (asset == BASE_CURRENCY) {
        return BASE_CURRENCY_UNIT;
    }
    uint256 price = 0;
    IEACAggregatorProxy source = IEACAggregatorProxy(assetsSources[asset]);
    if (address(source) != address(0)) {
        price = uint256(source.latestAnswer());
    }
    if (price == 0 && address(_fallbackOracle) != address(0)) {
        price = _fallbackOracle.getAssetPrice(asset);
    }
    require(price != 0, Errors.ORACLE_PRICE_NOT_READY);
    return price;
}

The oracle answer is queried from "source" which in the case of ERC721 tokens, is a ERC721OracleWrapper. It goes to its latestAnswer function:

function latestAnswer() external view override returns (int256) {
    return int256(oracleAddress.getPrice(asset));
}

oracleAddress is an NFTFloorOracle: oracleAddress = INFTFloorOracle(_oracleAddress);

function getPrice(address _asset)
    external
    view
    override
    returns (uint256 price)
{
    uint256 updatedAt = assetPriceMap[_asset].updatedAt;
    require(
        (block.number - updatedAt) <= config.expirationPeriod,
        "NFTOracle: asset price expired"
    );
    return assetPriceMap[_asset].twap;
}

In getPrice, if it has been longer than expirationPeriod since last update, the function will revert. Reverts will bubble up all the way to the original getAssetPrice call, which will revert. However, this is not the intended behavior. The aim was to use the fallbackOracle in case of primary oracle not being updated:

IEACAggregatorProxy source = IEACAggregatorProxy(assetsSources[asset]);
if (address(source) != address(0)) {
    price = uint256(source.latestAnswer());
}
if (price == 0 && address(_fallbackOracle) != address(0)) {
    price = _fallbackOracle.getAssetPrice(asset);
}
require(price != 0, Errors.ORACLE_PRICE_NOT_READY);
return price;

This occured because the standard Chainlink oracle latestAnswer does not revert, rather it returns 0:

function latestAnswer()
    external
    view
    returns (int256)
  {
    return currentAnswers[latestCompletedAnswer];
  }

Therefore, when changing to the home-made oracle, the way to wrap latestAnswer must change as well.

Impact

fallbackOracle will not be queried when the NFTFloorOracle does not have an updated answer. Most of the pool functions will not operate as health check is impaired.

Tools Used

Manual audit

Recommended Mitigation Steps

Wrap the latestAnswer query in a try/catch statement and query the fallback oracle in case of failure.

c4-judge commented 1 year ago

dmvt marked the issue as duplicate of #5