c4-submissions commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-11-betafinance/blob/0f1bb077afe8e8e03093c8f26dc0b7a2983c3e47/Omni_Protocol/src/OmniOracle.sol#L46

Vulnerability details

Impact

Band protocol oracle is not work for most of the pair, including ETH

Proof of Concept

This is the current implementation of the band oracle

if (config.provider == Provider.Band) {
    IStdReference.ReferenceData memory data;
    if (_underlying == WETH) {
        data = IStdReference(config.oracleAddress).getReferenceData("ETH", USD);
    } else {
        data = IStdReference(config.oracleAddress).getReferenceData(IERC20Metadata(_underlying).symbol(), USD);
    }
    require(data.lastUpdatedBase >= block.timestamp - config.delay, "OmniOracle::getPrice: Stale price for base.");
    require(data.lastUpdatedQuote >= block.timestamp - config.delayQuote, "OmniOracle::getPrice: Stale price for quote.");
    return data.rate * (PRICE_SCALE / 1e18) / (10 ** IERC20Metadata(_underlying).decimals()); // Price in one base unit with 1e36 precision
}

and the WETH address is hardcoded to WETH address in mainnet

    address public constant WETH = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2; // Need to hardcode WETH address per network deployed for Band

the problem is that the protocol intended to deploy in both BNB chain and ethereum mainnet

assume the protocol is deployed in etheruem mainnet:

if we go to https://data.bandprotocol.com/ and filter by support chain, we can see that in ethereum mainnet, there is no really ETH feed, only three asset price feed is supported

so the code

if (_underlying == WETH) {
        data = IStdReference(config.oracleAddress).getReferenceData("ETH", USD);
    }

does not work in mainnet

assume the protocol is deployed to BNB chain

we cannot change the hardcoded value to WBNB, because the underlying asset is BNB, but we evalute the asset as ETH, USD, the asset valuation is clearly very wrong

but assume the protocol hardcode the WETH address to wrapped ETH in BNB network

we go to https://data.bandprotocol.com/

and filter by BSC, only one asset is supported, and it is not BNB price of ETH price

Tools Used

Manual Review

Recommended Mitigation Steps

band protocol mostly support price feed in band oracle

the price oracle supported in ethereum or bsc is very limited

Assessed type

Oracle

JeffCX commented 10 months ago

Please note that the issue talks more than WBNB address can be used as WETH

the issue is mostly about that the band oracle cannot function properly on mainnet and on BSC network

thereksfour commented 10 months ago

In addition to the duplication with #30, this issue notes that Band Oracle not working on Ethereum and BSC, but deployers can use chainlink instead, will consider QA, leave it to sponsors.

JeffCX commented 10 months ago

but deployers can use chainlink instead

yeap, looks like mostly chainlink will be uesd

c4-judge commented 10 months ago

thereksfour marked the issue as primary issue

allenjlee commented 10 months ago

Agree with QA, also these issues are similar to QA report #23, where I acknowledge we make changes to store symbol for market and also no longer hard code the gas token, e.g. ETH/WETH.

c4-sponsor commented 10 months ago

allenjlee (sponsor) acknowledged

c4-sponsor commented 10 months ago

allenjlee marked the issue as disagree with severity