search

sherlock-audit / 2023-12-flatmoney-judging

9 stars 7 forks source link

AuditorPraise - wrong assumption in OracleModule._getOnchainPrice() causes overvalueing of rETH/ETH price #279

Closed sherlock-admin closed 5 months ago

sherlock-admin commented 5 months ago

AuditorPraise

high

wrong assumption in OracleModule._getOnchainPrice() causes overvalueing of rETH/ETH price

Summary

price returned from call to oracle.latestRoundData() is multiplied by (10 ** 10) because it is assumed to be in 1e8.

Vulnerability Detail

function _getOnchainPrice() internal view returns (uint256 price, uint256 timestamp) {
        IChainlinkAggregatorV3 oracle = onchainOracle.oracleContract;
        if (address(oracle) == address(0)) revert FlatcoinErrors.ZeroAddress("oracle");

        (, int256 _price, , uint256 updatedAt, ) = oracle.latestRoundData();
        timestamp = updatedAt;
        // check Chainlink oracle price updated within `maxAge` time.
        if (block.timestamp > timestamp + onchainOracle.maxAge)
            revert FlatcoinErrors.PriceStale(FlatcoinErrors.PriceSource.OnChain);

        if (_price > 0) {
            price = uint256(_price) * (10 ** 10); // convert Chainlink oracle decimals 8 -> 18 //@audit-issue
        } else {
            // Issue with onchain oracle indicates a serious problem
            revert FlatcoinErrors.PriceInvalid(FlatcoinErrors.PriceSource.OnChain);
        }
    }

The assumption is made that rETH/ETH price feed is returned in 8 decimals and is therefore multiplied by 1e10 to scale it up to 18 decimals .

/ETH price feeds are returned in 18 decimals so this is wrong. /USD ones are returned in 8 decimals. rETH/ETH is an ETH price feed so its in 18 decimals.

Impact

This means the price returned from OracleModule._getOnchainPrice() is overscaled making it have 28 decimals instead of 18 decimals.

this causes an overvaluing of the rETH/ETH

Code Snippet

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/OracleModule.sol#L141C5-L157C6

Tool used

Manual Review

Recommendation

don't multiply the price by (10 ** 10) here since its already in 18 decimals

if (_price > 0) {
            price = uint256(_price) * (10 ** 10); // convert Chainlink oracle decimals 8 -> 18
        }
sherlock-admin commented 5 months ago

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

takarez commented:

invalid: this is getting the USD value i believe

nevillehuang commented 4 months ago

Invalid, protocol is assumed to utilize rETH/USD chainlink price feed, and all non ETH chainlink pairs has 8 decimals and so this is presumed to be scaled correctly. This misses the issue on root cause of no rETH/USD in base chain.

Yes, the rETH/USD oracle is implemented separately using rETH/ETH and ETH/USD oracles in combination.