Issue with current external integration in regards to the availability of querying prices

Proof of Concept

From this section of the ReadMe:

### Q: In case of external protocol integrations, are the risks of external contracts pausing or executing an emergency withdrawal acceptable? If not, Watsons will submit issues related to these situations that can harm your protocol's functionality.
No, the risks of external contracts pausing or executing an emergency withdrawal are not acceptable.

Protocol has clearly stated that they do not accept any external risks and would like to protect against, but code implementation suggests otherwise cause the current integration of Chainlink does not actually consider that somethings could go wrong, for example take a look at LibUbiquityPool.sol#L523-L56 and how the price is being updated from Chainlink, the only check there is that the price is not "stale" but protocol does not take into account that actually attempting to query prices from CHainlink might just revert for whatever reasons maybe due to the feed being deprecated or what have you, i.e checking this we can see that there are multiple feeds that are soon to be deprecated.

Another issue here is the fact that there are no min/max circuit breaker checks, this is cause Chainlink is known to return their specified border prices anytime the price goes out of bounds

Impact

For the first case, as explained if the query to chainlink reverts then all the pricing logic attempts are effectively DOS'd now, i.e mintDollar() and redeemDollar() would also be inacessible since they directly call this function, now a user that's trying to redeem in order to sell of his assets would not be allowed to do so and could lose US$ value of his assets as prices plummet

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L523-L562

Tool used

Manual Review
https://medium.com/cyfrin/chainlink-oracle-defi-attacks-93b6cb6541bf

Recommended Mitigation Steps

Check out the fix to the unhandled chainlink revert case from this blog: https://medium.com/cyfrin/chainlink-oracle-defi-attacks-93b6cb6541bf or TLDR is to implement a falback mechainsm to querying prices and query from chainlink via try/catch.

Duplicate of #110

sherlock-admin2 commented 7 months ago

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

It will be temporary DOS at most, depend on Chainlink