search

sherlock-audit / 2023-05-USSD-judging

9 stars 7 forks source link

Bauchibred - StableOracleWBTC use BTC/USD chainlink oracle to price WBTC which is problematic if WBTC depegs #310

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

Bauchibred

medium

StableOracleWBTC use BTC/USD chainlink oracle to price WBTC which is problematic if WBTC depegs

Summary

The StableOracleWBTC contract utilizes a BTC/USD Chainlink oracle to determine the price of WBTC. However, this approach can lead to potential issues if WBTC were to depeg from BTC. In such a scenario, WBTC would no longer maintain an equivalent value to BTC. This can result in significant problems, including borrowing against a devalued asset and the accumulation of bad debt. Given that the protocol continues to value WBTC based on BTC/USD, the issuance of bad loans would persist, exacerbating the overall level of bad debt.

Important to note that this is like a 2 in 1 report as the same idea could work on the StableOracleWBGL contract too.

Vulnerability Detail

The vulnerability lies in the reliance on a single BTC/USD Chainlink oracle to obtain the price of WBTC. If the bridge connecting WBTC to BTC becomes compromised and WBTC depegs, WBTC may depeg from BTC. Consequently, WBTC's value would no longer be equivalent to BTC, potentially rendering it worthless (hopefully this never happens). The use of the BTC/USD oracle to price WBTC poses risks to the protocol and its users.

The following code snippet represents the relevant section of the StableOracleWBTC contract responsible for retrieving the price of WBTC using the BTC/USD Chainlink oracle:

contract StableOracleWBTC is IStableOracle {
    AggregatorV3Interface priceFeed;

    constructor() {
        priceFeed = AggregatorV3Interface(
            0x5f4eC3Df9cbd43714FE2740f5E3616155c5b8419

        );
    }

    function getPriceUSD() external view override returns (uint256) {
        (, int256 price, , , ) = priceFeed.latestRoundData();
        // chainlink price data is 8 decimals for WBTC/USD
        return uint256(price) * 1e10;
    }
}

NB: key to note that the above pricefeed is set to the wrong aggregator, the correct one is this: 0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599

Impact

Should the WBTC bridge become compromised or WBTC depeg from BTC, the protocol would face severe consequences. The protocol would be burdened with a substantial amount of bad debt stemming from outstanding loans secured by WBTC. Additionally, due to the protocol's reliance on the BTC/USD oracle, the issuance of loans against WBTC would persist even if its value has significantly deteriorated. This would lead to an escalation in bad debt, negatively impacting the protocol's financial stability and overall performance.

Code Snippet

StableOracleWBTC.sol#L12-L26

Tool used

Manual review

Recommendation

To mitigate the vulnerability mentioned above, it is strongly recommended to implement a double oracle setup for WBTC pricing. This setup would involve integrating both the BTC/USD Chainlink oracle and an additional on-chain liquidity-based oracle, such as UniV3 TWAP.

The double oracle setup serves two primary purposes. Firstly, it reduces the risk of price manipulation by relying on the Chainlink oracle, which ensures accurate pricing for WBTC. Secondly, incorporating an on-chain liquidity-based oracle acts as a safeguard against WBTC depegging. By monitoring the price derived from the liquidity-based oracle and comparing it to the Chainlink oracle's price, borrowing activities can be halted if the threshold deviation (e.g., 2% lower) is breached.

Adopting a double oracle setup enhances the protocol's stability and minimizes the risks associated with WBTC depegging. It ensures accurate valuation, reduces the accumulation of bad debt, and safeguards the protocol and its users

Bauchibred commented 1 year ago

Escalate for 10 USDC

I believe this issue has been incorrectly duplicated to #817. While I acknowledge the large number of issues submitted during the contest (approximately 1000), it's crucial to clarify that the concern here is not solely related to oracle addresses, despite the inclusion of wrong aggregators and inactive oracle addresses in the report. The main issue at hand is the potential depegging of WBTC, which is a bridged asset.

To address this vulnerability, the recommendation proposes implementing a double oracle setup for WBTC pricing, which serves as a safeguard against WBTC depegging.

To support this escalation, I have provided references to relevant cases:

I believe these references provide additional insights into the importance of considering measures to mitigate risks associated with bridged assets and emphasize why this issue should be treated as a standalone concern.

sherlock-admin commented 1 year ago

Escalate for 10 USDC

I believe this issue has been incorrectly duplicated to #817. While I acknowledge the large number of issues submitted during the contest (approximately 1000), it's crucial to clarify that the concern here is not solely related to oracle addresses, despite the inclusion of wrong aggregators and inactive oracle addresses in the report. The main issue at hand is the potential depegging of WBTC, which is a bridged asset.

To address this vulnerability, the recommendation proposes implementing a double oracle setup for WBTC pricing, which serves as a safeguard against WBTC depegging.

To support this escalation, I have provided references to relevant cases:

  • 836 is a sponsor-validated issue that emphasizes the importance of not relying solely on 100% of an asset's oracle price. While it may not directly relate to this specific issue, it underscores the need to consider implementing a "deviation threshold" when determining asset prices, particularly in the context of bridged assets.

  • 862, which is a valid duplicate, explores the potential impact of depegging on the protocol within a different context for this contest.

  • Additionally, references 1 and 2 are previous validated findings from other contests that further emphasize the standalone nature and significance of this issue.

I believe these references provide additional insights into the importance of considering measures to mitigate risks associated with bridged assets and emphasize why this issue should be treated as a standalone concern.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

twicek commented 1 year ago

Escalate for 10 USDC

Comments from Bauchibred regarding the fact that the present issue #310 is not a duplicate of #817 are correct. However, the main argument of this report is that:

WBTC may depeg from BTC

The only fact that WBTC/BTC would depeg is questionable. Since it is issued by a centralized entity (BitGo) it should be treated as trusted, because the only way for it to depeg would be via an error of this centralized entity or if the DAO voted a malicious proposal. See for details: https://www.gemini.com/cryptopedia/wbtc-what-is-wrapped-bitcoin#section-how-w-btc-works

Additionally, one of the justifications used to support the escalation regarding the deviation threshold mentioned in #836 . There is no guarantee that the TWAP will not stay within the deviation threshold even in the (very) unlikely event that WBTC/BTC depegs if the depegging happens slowly.

Regarding #862, this is not a duplicate to this report because DAI principal depeg risk comes from depegging of the underlying collateral reserves, which as we have seen is not possible for WBTC since the reserves are held by a centralized party. Also #862 specifically related to an overflow problem during a potential DAI depeg which is totally different than this report.

These are all the reason why I think this report is invalid.

sherlock-admin commented 1 year ago

Escalate for 10 USDC

Comments from Bauchibred regarding the fact that the present issue #310 is not a duplicate of #817 are correct. However, the main argument of this report is that:

WBTC may depeg from BTC

The only fact that WBTC/BTC would depeg is questionable. Since it is issued by a centralized entity (BitGo) it should be treated as trusted, because the only way for it to depeg would be via an error of this centralized entity or if the DAO voted a malicious proposal. See for details: https://www.gemini.com/cryptopedia/wbtc-what-is-wrapped-bitcoin#section-how-w-btc-works

Additionally, one of the justifications used to support the escalation regarding the deviation threshold mentioned in #836 . There is no guarantee that the TWAP will not stay within the deviation threshold even in the (very) unlikely event that WBTC/BTC depegs if the depegging happens slowly.

Regarding #862, this is not a duplicate to this report because DAI principal depeg risk comes from depegging of the underlying collateral reserves, which as we have seen is not possible for WBTC since the reserves are held by a centralized party. Also #862 specifically related to an overflow problem during a potential DAI depeg which is totally different than this report.

These are all the reason why I think this report is invalid.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

hrishibhat commented 1 year ago

@Bauchibred any further comments on the validity of the issue due?

Bauchibred commented 1 year ago

@hrishibhat I still think issue should be valid and stand on its own, the reply to my escalation was that the idea of “WBTC depegging is not valid”, but I argue that that's wrong, Wrapped BTCs have depegged multiple times in the past, one of the popular instance being after the whole FTX saga, though fair enough that was soBTC, now worth around 7% of what it's supposed to be pegged to.

Note that Chainlink even provides a separate price feed to query the WBTC/BTC price,seen here.

So I still believe that the price of WBTC/USD for more accuracy should be calculated based on WBTC/BTC and BTC/USD price feeds instead of directly using the BTC/USD feed.

Additionally this article from the bitcoin manual, could provide more insight on how and why wrapped bitcoins could depeg.

hrishibhat commented 1 year ago

Result: Medium Has duplicates This is not a duplicate of #817 Considering this issue and other duplicates of this issue as a valid medium given the edge case possibility of WBTC de-pegging.

sherlock-admin commented 1 year ago

Escalations have been resolved successfully!

Escalation status: