Chainlink scale factor calculation may overflow in `OndoPriceOracleV2`

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-ondo/blob/main/contracts/lending/OndoPriceOracleV2.sol#L251-L267

Vulnerability details

The OndoPriceOracleV2 contract allows to setup a Chainlink oracle feed. When adding a new feed, the contract will calculate a scale factor to convert between the underlying token decimals and the oracle feed decimals:

https://github.com/code-423n4/2023-01-ondo/blob/main/contracts/lending/OndoPriceOracleV2.sol#L251-L267

function _setFTokenToChainlinkOracle(
  address fToken,
  address chainlinkOracle
) internal {
  require(
    fTokenToOracleType[fToken] == OracleType.CHAINLINK,
    "OracleType must be Chainlink"
  );
  address underlying = CTokenLike(fToken).underlying();
  fTokenToChainlinkOracle[fToken].scaleFactor = (10 **
    (36 -
      uint256(IERC20Like(underlying).decimals()) -
      uint256(AggregatorV3Interface(chainlinkOracle).decimals())));
  fTokenToChainlinkOracle[fToken].oracle = AggregatorV3Interface(
    chainlinkOracle
  );
}

This function assumes that the sum of both the underlying token and Chainlink feed decimals won't exceed 36. The calculation can be better seen as 36 - (underlying.decimals() + chainlinkFeed.decimals().

Impact

If the previous assumption doesn't hold for a given ERC20 underlying token and Chainlink feed, then the function will revert due to the overflow in the unsigned integer calculation, and will prevent the Chainlink oracle from being set up.

See the following report of a similar issue https://github.com/code-423n4/2022-10-inverse-findings/issues/533

PoC

If the sum of the underlying token decimals and then the Chainlink feed decimals are greater than 36 (for example, 8 for the feed and 30 for the underlying token), then the subtraction on lines 261-263 will overflow due to Solidity's default checked math and cause a revert (36 - 30 - 8 = -2).

https://github.com/code-423n4/2023-01-ondo/blob/main/contracts/lending/OndoPriceOracleV2.sol#L261-L263

(36 -
  uint256(IERC20Like(underlying).decimals()) -
  uint256(AggregatorV3Interface(chainlinkOracle).decimals())));

Recommendation

Expect that the sum of the Chainlink feed decimals and the underlying token decimals may be higher than 36. Note that if this is the case, then the scaling on line 300 should be down (divide instead of multiply).

[c4-judge]

Duplicate of https://github.com/code-423n4/2023-01-ondo-findings/issues/330

[trust1995]

I view this sort of issue as QA as the code in scope uses USDC has collateral and it would be a big stretch to hypothesize a >18 decimals token used.

[c4-judge]

trust1995 marked the issue as grade-b