search

sherlock-audit / 2023-12-ubiquity-judging

2 stars 2 forks source link

Bauchibred - Issue with current external integration in regards to the accuracy of prices gotten #158

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 8 months ago

Bauchibred

medium

Issue with current external integration in regards to the accuracy of prices gotten

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 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 their own specified maximum and minimum bounds

Impact

This objectively breaks all accounting for minting and redeeming, as protocol would assume a wrong price for assets provided and might lead to even breaking the peg of the stable coin when provided assets are overvalued and wrong amount are minted

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

Recommended Mitigation Steps

Check out the fix to the Oracle Returns Incorrect Price During Flash Crashes from this article or TLDR is to check out the min/max price for supported assets and ensure to check against them

Duplicate of #144

sherlock-admin2 commented 7 months ago

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

auditsea commented:

The issue describes about not checking min/max value from Chainlink for data staleness, but this is not required

sherlock-admin2 commented 7 months ago

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

auditsea commented:

The issue describes about not checking min/max value from Chainlink for data staleness, but this is not required