Omniscia Tangible Audit

RealtyOracleV2 Manual Review Findings

RealtyOracleV2 Manual Review Findings

ROV-01M: Inexistent Normalization of Price

Description:

The RealtyOracleV2::convertNativePriceToUSD function does not appear to normalize the input currency's decimals, potentially leading to discrepant USD-based price measurements.

Impact:

The severity of this exhibit depends on what actions the Tangible team takes to remediate it.

Example:

contracts/priceOracles/RealtyOracleV2.sol
70function convertNativePriceToUSD(
71 uint256 nativePrice,
72 uint16 currencyISONum
73) internal view returns (uint256) {
74 // take it differently from currency feed
75 AggregatorV3Interface priceFeedNativeToUSD = currencyFeed.currencyPriceFeedsISONum(
76 currencyISONum
77 );
78 (, int256 price, , , ) = priceFeedNativeToUSD.latestRoundData();
79 if (price < 0) {
80 price = 0;
81 }
82 //add conversion premium
83 uint256 nativeToUSD = uint256(price) +
84 currencyFeed.conversionPremiumsISONum(currencyISONum);
85 return (nativePrice * nativeToUSD) / 10 ** uint256(priceFeedNativeToUSD.decimals());
86}

Recommendation:

We advise the code to normalize the nativePrice decimals as certain currencies (i.e. yen) do not contain any decimals in contrast to others (i.e. euro).

Alleviation (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

The Tangible team has specified that in the context of Chainlink oracles, all currencies yield a precision of 8 decimals inclusive of yen for example.

Based on this fact, we consider this exhibit to be invalid as the nativePrice can be specified with a degree of 8 decimals at all times.

Description:

The referenced invocation of latestRoundData is insecure as it does not properly sanitize the result of the oracle call.

Impact:

Currently, a misbehaving Chainlink oracle will not be detected by the Tangible protocol causing it to consume incorrect or outdated / stale price points for the assets it is querying.

Example:

contracts/priceOracles/RealtyOracleV2.sol
78(, int256 price, , , ) = priceFeedNativeToUSD.latestRoundData();

Recommendation:

We advise the code to be updated, enforcing proper sanitization measure(s) to the external Chainlink oracle call.

The data point of interest the latestRoundData function yields is the updatedAt timestamp. The desire is to enforce a particular "heartbeat" of data validity that ensures the updatedAt value satisfies the time threshold imposed by the Tangible protocol. We should note that Chainlink imposes different heartbeats for different asset types and as such the time limit that should be imposed needs to be sensible based on the Tangible protocol's needs and the idle-time threshold Chainlink has set for each particular data feed.

As an alternative, we advise an administrative manual "pause" mechanism to be introduced, preventing price measurements from the RealtyOracleV2::convertNativePriceToUSD method to be utilized. This will permit the Tangible team to be able to quickly react in case of abnormal market events such as that of the LUNA price crash.

Alleviation (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

A time-based check was introduced ensuring that the price has been reported at least within the past day, preventing outdated / stale Chainlink data from being consumed.

As such, we consider this restriction sufficient in the context of the Tangible team's use case.