ChainlinkProxyPriceProvider Manual Review Findings

CPP-01M: Inexistent Chainlink Data Staleness Sanitization

Description:

The IChainlinkAggregator implementation utilized by the codebase is outdated and as such is not properly sanitized.

Example:

Copy
59/// @notice Gets an asset price by address
60/// @param asset The asset address
61function getAssetPrice(address asset) public view override returns (uint256) {
62    IChainlinkAggregator source = _assetsSources[asset];
63    if (asset == _ethAddress) {
64        return 1 ether;
65    } else {
66        // Require the asset has registered source
67        require(address(source) != address(0), 'SOURCE_IS_MISSING');
68
69        int256 price = IChainlinkAggregator(source).latestAnswer();
70        require(price > 0, 'INVALID_PRICE');
71
72        return uint256(price);
73    }
74}

Recommendation:

While the interface itself needs to be updated in its dedicated finding, the code utilizing it should also be updated to properly sanitize the values returned by the latestRoundData call. Namely, the answeredInRound value should be within a project-specified delta threshold from roundID to ensure data is not stale.

Alleviation:

The codebase has been refactored to utilize the AggregatorV3Interface of Chainlink, however, the data staleness check for the round ID delta has not been imposed. Given that the Platypus team considered this exhibit but opted not to apply a remediation for it, we assume the default data freshness guarantee by Chainlink is sufficient and as such we consider this exhibit dealt with.

CPP-02M: Inexplicable Repeat Invocation Capability

Description:

The _ethAddress is an integral system variable and causes a price of 1 ether to be yielded by the corresponding functions. Misuse can lead to a compromisation of the protocol as a whole.

Example:

Copy
110/// @notice Sets the ethAddress, in aave the ethAddress is a special representation for ETH,
111///         generalized to be configurable per system, can be for example WETH address
112/// @param ethAddress The address of ETH
113function setETHAddress(address ethAddress) external onlyOwner {
114    require(ethAddress != address(0), 'ADDRESS_IS_ZERO');
115    _ethAddress = ethAddress;
116}

Recommendation:

We advise the value to only be settable once as there should not be any scenario under which it is desirable for the value to be re-set.

Alleviation:

The _ethAddress member no longer exists in the contract and as such this exhibit can be considered null.

CPP-03M: Inexistent Validation of Oracle Accuracy

Description:

The internalSetAssetsSources does not properly validate that the feeds inserted have an accuracy of 18, thus guaranteeing that it is an ETH / asset pair.

Example:

Copy
48/// @notice Internal function to set the sources for each asset
49/// @param assets The addresses of the assets
50/// @param sources The address of the source of each asset
51function internalSetAssetsSources(address[] memory assets, address[] memory sources) internal {
52    require(assets.length == sources.length, 'INCONSISTENT_PARAMS_LENGTH');
53    for (uint256 i = 0; i < assets.length; i++) {
54        _assetsSources[assets[i]] = IChainlinkAggregator(sources[i]);
55        emit AssetSourceUpdated(assets[i], sources[i]);
56    }
57}

Recommendation:

We advise such sanitization to be imposed by invoking the decimals member of the Chainlink interface and validating that it is equal to 18.

Alleviation:

Proper validation was introduced in the inclusion of sources by validating that the decimals member is exactly equal to 8, the desired oracle precision by the project.