UniV3 oracle price is incorrect if `token0.decimals() > token1.decimals()`

[hats-bug-reporter[bot]]

Github username: @bahurum Submission hash (on-chain): 0xabe8d559e0ca80944ca2508da2b8de07c40377ef426f15ad2f9541311e683354 Severity: medium

Description: Description\ The function _getV3Price() of CvgOracle will return an incorrect result depending on the order of the tokens if they have different decimals. The result is correct only if the token0 is the one with the smaller amount of decimals, and incorrect otherwise.

The issue comes from the scaling factor computed in _getDecimalsDelta(), which is independent of the order of the tokens, while it should be dependent.

Attack Scenario\ The issue is confirmed by the PoC attached where, as an example, the price of UniV3 ETH/USDC and ETH/USDT is computed. The 2 pools have ETH as token1 and token0 respectively:

Logs:
  ETH/USDC price: 1632116256333849412388
  ETH/USDT price: 0

The value for ETH/USDT price is incorrect, and this will make the oracle calls fail during the comparison to the ETH Chainlink price.

Recommendation Change the _getDecimalsDelta() so that it returns the correct amount if token0Decimals > token1Decimals.

    function _getDecimalsDelta(address uniswapPool) internal view returns (uint256 decimalsDelta) {
        uint256 token0Decimals = IERC20Metadata(IUniswapV3Pool(uniswapPool).token0()).decimals();
        uint256 token1Decimals = IERC20Metadata(IUniswapV3Pool(uniswapPool).token1()).decimals();
        decimalsDelta = token0Decimals <= token1Decimals
            ? 18 - (token1Decimals - token0Decimals)
-           ? 18 - (token0Decimals - token1Decimals)
+           : 18 + (token0Decimals - token1Decimals);
    }

Attachments

1. Proof of Concept (PoC) File

Files:

• UniV3incorrectPrice.t.sol (https://hats-backend-prod.herokuapp.com/v1/files/QmY6TJDTkhaozEUY3fqefSU92VWz5zzsvV7tgS7D1VaTNF)

[walk-on-me]

Hello, Thanks a lot for your attention.

You are right, we miscalculated the delta decimals between both tokens ... Thanks a lot for this finding.

To add a bit more context :

• This contract will be easily interchangeable
• We would test the value of the token before creating an entry on this token
• We have the deltaCvgAggregator

So it'd be very unlikely that an exploiter succeed in stealing some funds with this exploit.

To conclude, we agree with you that this finding is a MEDIUM