hats-finance / VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f

LP token lending protocol
MIT License
2 stars 1 forks source link

The lack of flexibility of `getOracleAssetPrice()` would prevent the listing of some widely known assets #60

Open hats-bug-reporter[bot] opened 1 year ago

hats-bug-reporter[bot] commented 1 year ago

Github username: @abhishekvispute Submission hash (on-chain): 0xf7b114c1b3f218a7c9032ddefce0231b918afcab67bf1064112a54c230ac75d4 Severity: low severity

Description: Description\ VMEX uses the Chainlink price feed to obtain the latest price data and get a base price for all assets. However, there are multiple instances where price derivation needs to occur explicitly. For instance, there is no Chainlink price feed for WBTC/USD on Mainnet, so you need to derive it using WBTC/BTC and BTC/USD. Similarly, there is no dedicated price feed for Coinbase Wrapped Staked ETH (CBETH) on Optimism, so you need to derive it using CBETH/ETH and ETH/USD.

function getOracleAssetPrice(address asset) internal returns (uint256){
        IChainlinkPriceFeed source = _assetsSources[asset].feed;
        if (address(source) == address(0)) {
            return _fallbackOracle.getAssetPrice(asset);
        } else {
            (
                /* uint80 roundID */,
                int256 price,
                /*uint startedAt*/,
                uint256 updatedAt,
                /*uint80 answeredInRound*/
            ) = IChainlinkPriceFeed(source).latestRoundData(); // @audit wont work for tokens like WBTC
            IChainlinkAggregator aggregator = IChainlinkAggregator(IChainlinkPriceFeed(source).aggregator());
            if (price > int256(aggregator.minAnswer()) && 
                price < int256(aggregator.maxAnswer()) && 
                block.timestamp - updatedAt < _assetsSources[asset].heartbeat
            ) {
                return uint256(price);
            } else {
                return _fallbackOracle.getAssetPrice(asset);
            }
        }
}

The current code of getOracleAssetPrice is not flexible enough to allow for these derivations, forcing VMEX to either drop these widely used assets or deploy their own price feed contract that mimics Chainlink data.

Attack Scenario\ NA

Recommendation\ One solution is to define a separate contract for each asset and create a price feed per asset. This can be fetched inside the VMEX oracle. However, this approach could be stretch to implement for all assets.

Another option is to have a switch between a simple chainlink fetch and a derived one.

Alternatively, you can leave it as it is and, for certain assets, have a separate contract with the same function signatures as the chainlink aggregator and can then derive the prices from real chainlink price feeds there.

ksyao2002 commented 1 year ago

Thanks for the submission. For the cases where we want to add a new oracle that our current framework can't cover, our protocol was to upgrade the VMEXOracle contract and include a new DataTypes.ReserveAssetType to handle. For now, we won't fix this since it depends on what assets we choose to include.