ChainlinkOracle Manual Review Findings

COE-01M: Inexistent Volatility Protection Mechanisms

Type	Severity	Location
External Call Validation		ChainlinkOracle.sol:L54

Description:

The ChainlinkOracle::_getQuote function will not enforce any volatility checks and will simply yield a price that protocols can utilize.

This is insecure from a DeFi perspective as prices generated from volatile assets are not expected to be consumed, and this is listed as a security consideration by the Chainlink documentation.

Impact:

The severity of this exhibit is considered unknown as it is not clear whether the oracles of the Euler Finance team should guard against volatility.

Example:

Copy
46/// @notice Get the quote from the Chainlink feed.
47/// @param inAmount The amount of `base` to convert.
48/// @param _base The token that is being priced.
49/// @param _quote The token that is the unit of account.
50/// @return The converted amount using the Chainlink feed.
51function _getQuote(uint256 inAmount, address _base, address _quote) internal view override returns (uint256) {
52    bool inverse = ScaleUtils.getDirectionOrRevert(_base, base, _quote, quote);
53
54    (, int256 answer,, uint256 updatedAt,) = AggregatorV3Interface(feed).latestRoundData();
55    if (answer <= 0) revert Errors.PriceOracle_InvalidAnswer();
56    uint256 staleness = block.timestamp - updatedAt;
57    if (staleness > maxStaleness) revert Errors.PriceOracle_TooStale(staleness, maxStaleness);
58
59    uint256 price = uint256(answer);
60    return ScaleUtils.calcOutAmount(inAmount, price, scale, inverse);
61}

Recommendation:

We advise the Euler Finance team to evaluate this trait, and potentially incorporate volatility protections into the ChainlinkOracle itself.

Alleviation:

The Euler Finance team evaluated this exhibit, and clarified that they do not consider volatility protections to be a feature exposed by the Euler Finance oracle system.

As such, we consider this exhibit as inapplicable given that lack of validation is a desirable business trait of the system.

COE-02M: Inexistent Validation of Acceptable Answer Range

Type	Severity	Location
External Call Validation		ChainlinkOracle.sol:L54

Description:

The AggregatorV3Interface::aggregator of a particular proxy oracle in Chainlink has an acceptable set of values that it can report that are exposed by its IOffchainAggregator::minAnswer and IOffchainAggregator::maxAnswer variables. A price reported close to these values indicates a sharp market event and as such should in most circumstances not be consumed.

Additionally, the aggregators may continue to report the minimum / maximum price they are able to thereby bypassing the staleness check whilst reporting a price that may be incorrect.

Example:

Copy
46/// @notice Get the quote from the Chainlink feed.
47/// @param inAmount The amount of `base` to convert.
48/// @param _base The token that is being priced.
49/// @param _quote The token that is the unit of account.
50/// @return The converted amount using the Chainlink feed.
51function _getQuote(uint256 inAmount, address _base, address _quote) internal view override returns (uint256) {
52    bool inverse = ScaleUtils.getDirectionOrRevert(_base, base, _quote, quote);
53
54    (, int256 answer,, uint256 updatedAt,) = AggregatorV3Interface(feed).latestRoundData();
55    if (answer <= 0) revert Errors.PriceOracle_InvalidAnswer();
56    uint256 staleness = block.timestamp - updatedAt;
57    if (staleness > maxStaleness) revert Errors.PriceOracle_TooStale(staleness, maxStaleness);
58
59    uint256 price = uint256(answer);
60    return ScaleUtils.calcOutAmount(inAmount, price, scale, inverse);
61}

Recommendation:

As this particular warning is listed in the security best practices of Chainlink, we advise it to be potentially incorporated into the operation of the ChainlinkOracle. As a potential solution, the minimum and maximum acceptable answers by the aggregator of a data feed proxy can be extracted and the answer detected could be evaluated as being within the range of the aggregator and specifically outside of a narrow margin close to the limits.

Alleviation:

The Euler Finance team evaluated this exhibit, and clarified that they do not wish to incorporate the Chainlink price range limitations within the oracle.

As such, we consider this exhibit as safely acknowledged.

COE-03M: Misleading Specification of Usability

Type	Severity	Location
Standard Conformity		ChainlinkOracle.sol:L32

Description:

The ChainlinkOracle::constructor documentation specifies that the base and quote tokens are not required to correspond to the feed assets, however, this is incorrect unless under a very specific set of circumstances.

The example specified is a condition under which security would be compromised as the USD is not always at parity with the USDC stablecoin.

Example:

Copy
26/// @notice Deploy a ChainlinkOracle.
27/// @param _base The address of the base asset corresponding to the feed.
28/// @param _quote The address of the quote asset corresponding to the feed.
29/// @param _feed The address of the Chainlink price feed.
30/// @param _maxStaleness The maximum allowed age of the price.
31/// @dev Base and quote are not required to correspond to the feed assets.
32/// For example, the ETH/USD feed can be used to price WETH/USDC.
33constructor(address _base, address _quote, address _feed, uint256 _maxStaleness) {
34    base = _base;
35    quote = _quote;
36    feed = _feed;
37    maxStaleness = _maxStaleness;
38
39    // The scale factor is used to correctly convert decimals.
40    uint8 baseDecimals = IERC20(base).decimals();
41    uint8 quoteDecimals = IERC20(quote).decimals();
42    uint8 feedDecimals = AggregatorV3Interface(feed).decimals();
43    scale = ScaleUtils.calcScale(baseDecimals, quoteDecimals, feedDecimals);
44}

Recommendation:

We strongly advise the comment line to be removed, and the overall recommendation to be avoided.

While developers are free to disassociate the base, quote, and feed implementations it is ill-advised to do so in most circumstances as even "identical" assets naturally deviate from each other in price.

Alleviation:

The potentially misinterpreted statement has been removed from the ChainlinkOracle::constructor documentation as advised.

COE-04M: Potentially Unsupported Function Signature

Type	Severity	Location
Standard Conformity		ChainlinkOracle.sol:L40, L41

Description:

The code of the ChainlinkOracle::constructor will invoke the IERC20::decimals function as exposed by the forge-std library, however, the IERC20::decimals function is not actually part of the EIP-20 specification.

Impact:

Most EIP-20 assets do implement the IERC20::decimals function signature, however, it is not mandated by the standard and as such a small subset of EIP-20 tokens is incompatible with the ChainlinkOracle presently.

Example:

Copy
26/// @notice Deploy a ChainlinkOracle.
27/// @param _base The address of the base asset corresponding to the feed.
28/// @param _quote The address of the quote asset corresponding to the feed.
29/// @param _feed The address of the Chainlink price feed.
30/// @param _maxStaleness The maximum allowed age of the price.
31/// @dev Base and quote are not required to correspond to the feed assets.
32/// For example, the ETH/USD feed can be used to price WETH/USDC.
33constructor(address _base, address _quote, address _feed, uint256 _maxStaleness) {
34    base = _base;
35    quote = _quote;
36    feed = _feed;
37    maxStaleness = _maxStaleness;
38
39    // The scale factor is used to correctly convert decimals.
40    uint8 baseDecimals = IERC20(base).decimals();
41    uint8 quoteDecimals = IERC20(quote).decimals();
42    uint8 feedDecimals = AggregatorV3Interface(feed).decimals();
43    scale = ScaleUtils.calcScale(baseDecimals, quoteDecimals, feedDecimals);
44}

Recommendation:

In case all EIP-20 assets are expected to be supported, we advise decimals to either be opportunistically queried or for decimals to be supplied as input arguments thus permitting any token to have a ChainlinkOracle deployed.

Alleviation:

A common BaseAdapter::_getDecimals implementation has been introduced in the BaseAdapter upstream contract that will attempt to fetch the IERC20::decimals of an asset and default to 32 if they cannot be fetched.

As such, we consider this exhibit fully alleviated.