plainshift-2 - PriceOracle#getPriceFromChainlink doesn't do a staleness check

[sherlock-admin]

plainshift-2

medium

PriceOracle#getPriceFromChainlink doesn't do a staleness check

Summary

PriceOracle retrieves prices from Chainlink. There are no staleness checks on the latest round data. This can be problematic since price oracles are used in critical calculations such as determining the liquidation seizure.

Vulnerability Detail

If Chainlink ever returns a stale price, IronBank runs a risk of utilizing incorrect pricing. This can result in IronBank making incorrect decisions, such as allowing or disallowing liquidations.

IronBank currently doesn't have a staleness check when calling the Chainlink price. This can be seen below where the only value IronBank retrieves is the price:

(, int256 price,,,) = registry.latestRoundData(base, quote);

By not checking the updatedAt or answeredInRound values, IronBank runs the risk of receiving a stale price. More info on Chainlink's latestRoundData can be found here.

Impact

Consider the following example where a liquidation seizure is occurring. Within the _getLiquidationSeizeAmount function, IronBank retrieves both the borrow market price and the collateral market price. If either of these prices are stale, then the seize amount will be incorrect leading to user's potentially manipulating the IronBank market or the liquidator receiving an incorrect amount of tokens.

Below is the _getLiquidationSeizeAmount function:

function _getLiquidationSeizeAmount(
    address marketBorrow,
    address marketCollateral,
    DataTypes.Market storage mCollateral,
    uint256 repayAmount
) internal view returns (uint256) {
    uint256 borrowMarketPrice = PriceOracleInterface(priceOracle).getPrice(marketBorrow);
    uint256 collateralMarketPrice = PriceOracleInterface(priceOracle).getPrice(marketCollateral);
    uint256 borrowMarketPrice = PriceOracleInterface(priceOracle).getPrice(marketBorrow);
    uint256 collateralMarketPrice = PriceOracleInterface(priceOracle).getPrice(marketCollateral);
    require(borrowMarketPrice > 0 && collateralMarketPrice > 0, "invalid price");

    // collateral amount = repayAmount * liquidationBonus * borrowMarketPrice / collateralMarketPrice
    // IBToken amount = collateral amount / exchangeRate
    //   = repayAmount * (liquidationBonus * borrowMarketPrice) / (collateralMarketPrice * exchangeRate)
    uint256 numerator = (mCollateral.config.liquidationBonus * borrowMarketPrice) / FACTOR_SCALE;
    uint256 denominator = (_getExchangeRate(mCollateral) * collateralMarketPrice) / 1e18;

    return (repayAmount * numerator) / denominator;
}

Code Snippet

/**
 * @notice Get price from Chainlink.
 * @param base The base asset
 * @param quote The quote asset
 * @return The price
 */
function getPriceFromChainlink(address base, address quote) internal view returns (uint256) {
    // AUDIT: No check on whether the retrieved price is stale
    (, int256 price,,,) = registry.latestRoundData(base, quote);
    require(price > 0, "invalid price");

    // Extend the decimals to 1e18.
    // DONE - Audit: decimals is assumed to be less than 18, which is not always the case.
    return uint256(price) * 10 ** (18 - uint256(registry.decimals(base, quote)));
}

Relevant code can be found here.

Tool used

Manual Review

Recommendation

Staleness checks should be implemented on the Chainlink price. Confirming that the updatedAt or answeredInRound values are relatively recent ensures that IronBank is receiving correct pricing. If the staleness check fails IronBank should consider temporarily disabling the market till the price is up-to-date.

In addition, IronBank should check that the date values are not zero and the round data is complete since in some cases a round can time out if it does not reach consensus.

Finally, if prices are stale, IronBank should consider allowing borrowers who may have potentially liquidated loans a grace period to repay their loans lest liquidation bots will immediately liquidate their loan as the market is re-enabled.

Duplicate of #9