Decimals not scaled correctly in `getOraclePrice()` cause huge price discrepancies

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibOracle.sol#L19-L67

Vulnerability details

Summary

Where the Oracle called is not the baseOracle, the values returned are not scaled correctly; leading to either a very large overpricing or very large underpricing

Vulnerability Details

In the getOraclePrice() try block, baseOracle returned price basePrice is scaled up from 8 to 18 decimals, however the same is not done for other oracles. Given that other oracles (e.g. JPY, XAU) will also return 8 decimal USD price; this means that their returned value will be significantly overpriced.

    try baseOracle.latestRoundData() returns (uint80 baseRoundID, int256 basePrice, uint256,uint256 baseTimeStamp, uint80) {
        if (oracle == baseOracle) {
            // SOME CODE
        } else {

            // SOME CODE
>>>         uint256 priceInEth = uint256(price).div(uint256(basePrice));
            oracleCircuitBreaker(roundID, baseRoundID, price, basePrice, timeStamp, baseTimeStamp);
>>>         return priceInEth;
        }
    }

In the catch block, twapInv is an 18 decimal value returned from twapCircuitBreaker() which is then multiplied by uint256(price * C.BASE_ORACLE_DECIMALS). This equates to (8 dec * 10 dec) * 18 dec, which will be a 36 decimal value and a massive underpricing of priceinEth.

    } catch {
        if (oracle == baseOracle) {
            return twapCircuitBreaker();
        } else {

>>>         uint256 twapInv = twapCircuitBreaker();
>>>         uint256 priceInEth = uint256(price * C.BASE_ORACLE_DECIMALS).mul(twapInv);
            return priceInEth;
        }
    }

Impact

The vastly incorrect pricing of "other" stable assets such as JPY and XAU will lead to easy exploitation; costing users to lose a lot of money and rendering their markets unusable.

Tools Used

Manual Review Foundry Testing

Recommendations

Scale up in the try block as:

    try baseOracle.latestRoundData() returns (uint80 baseRoundID, int256 basePrice, uint256,uint256 baseTimeStamp, uint80) {
        if (oracle == baseOracle) {
            // SOME CODE
        } else {

            // SOME CODE
           uint256 priceInEth = uint256(price).div(uint256(basePrice));
+          priceInEth = priceInEth.mul(10**C.BASE_ORACLE_DECIMALS);
            oracleCircuitBreaker(roundID, baseRoundID, price, basePrice, timeStamp, baseTimeStamp);
            return priceInEth;
        }
    }

Scale down in the catch block. First add a new variable ETH_DECIMALS = 10 ** 18 to Constants.sol and use that to scale down in the catch block as:

uint256 twapInv = twapCircuitBreaker();
uint256 priceInEth = uint256(price * C.BASE_ORACLE_DECIMALS).mul(twapInv);

// Scale down from 1e36 to 1e18 by dividing by 1e18
priceInEth = priceInEth / C.ETH_DECIMALS;

Assessed type

Oracle

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[raymondfam]

uint256(price).div(uint256(basePrice)) is 1e18 scaled unless price entails decimals different than 8.

[c4-pre-sort]

raymondfam marked the issue as primary issue

[c4-judge]

hansfriese marked the issue as unsatisfactory: Insufficient proof