Submitted by 0xhals, MaanVader, Cosine, 0xVinylDavyl, SanketKogekar. Selected submission by: Cosine.
Relevant GitHub Links
Summary
That a chainlink oracle works does not mean it will be supported by chainlink in the future and keeps working, and it could also be possible that the address of the price feed changes. Therefore, it does not make sense to prevent price feed addresses from being updated, or removed, but the protocol prevents that.
Vulnerability Details
There is only one function inside ChainlinkARBOracle to update the price feed addresses:
plain textfunction addTokenPriceFeed(address token, address feed) external onlyOwner { if (token == address(0)) revert Errors.ZeroAddressNotAllowed(); if (feed == address(0)) revert Errors.ZeroAddressNotAllowed(); if (feeds[token] != address(0)) revert Errors.TokenPriceFeedAlreadySet(); feeds[token] = feed; }
As we can see it will only allow to set the price feed ones and revert if trying to update, or remove a price feed. Therefore, if chainlink changes something, or the owner accidentally set the wrong address, or the protocol no longer wants to support a price feed, it can not be removed, or updated.
Impact
It is not possible to remove price feeds which are no longer supported by chainlink, or update the addresses of price feeds. This can lead to a complete DoS of the underlying token.
As this feeds mapping is also the only check if it is a valid token when calling the oracle and the feed can not be removed, it will always pass this check even if the protocol no longer wishes to support this token:
plain textfunction consult(address token) public view whenNotPaused returns (int256, uint8) { address _feed = feeds[token]; if (_feed == address(0)) revert Errors.NoTokenPriceFeedAvailable(); ... }
Tools Used
Manual Review
Recommendations
Remove this line:
plain textif (feeds[token] != address(0)) revert Errors.TokenPriceFeedAlreadySet();