c4-bot-2 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Oracle.sol#L272-L326

Vulnerability details

Impact

priceX96 and verifyPriceX96 variables are used wrong V3Oracle::_getReferenceTokenPriceX96

The issue occurs when both usesChainlink and usesTWAP are true. In this case, the priceX96 variable is set to the value of chainlinkPriceX96, and verifyPriceX96 is set to the value of twapPriceX96 or vice versa, depending on the oracle mode. However, when the function checks the difference between priceX96 and verifyPriceX96, it does not take into account which variable was set to the chainlink price and which was set to the TWAP price.

This can result in a significant price difference between the two oracle sources, which can cause the function to revert or return incorrect price data.

For example, if the chainlink price is much higher than the TWAP price, and priceX96 is set to the chainlink price and verifyPriceX96 is set to the TWAP price, the function will revert because the difference between the two prices is too high. However, if the opposite is true and priceX96 is set to the TWAP price and verifyPriceX96 is set to the chainlink price, the function will not revert and will return incorrect price data.

Proof of Concept

    function _getReferenceTokenPriceX96(address token, uint256 cachedChainlinkReferencePriceX96)
        internal
        view
        returns (uint256 priceX96, uint256 chainlinkReferencePriceX96)
    {
        if (token == referenceToken) {
            return (Q96, chainlinkReferencePriceX96);
        }

        TokenConfig memory feedConfig = feedConfigs[token];

        if (feedConfig.mode == Mode.NOT_SET) {
            revert NotConfigured();
        }

        uint256 verifyPriceX96;

        bool usesChainlink = (
            feedConfig.mode == Mode.CHAINLINK_TWAP_VERIFY || feedConfig.mode == Mode.TWAP_CHAINLINK_VERIFY
                || feedConfig.mode == Mode.CHAINLINK
        );
        bool usesTWAP = (
            feedConfig.mode == Mode.CHAINLINK_TWAP_VERIFY || feedConfig.mode == Mode.TWAP_CHAINLINK_VERIFY
                || feedConfig.mode == Mode.TWAP
        );

        if (usesChainlink) {
            uint256 chainlinkPriceX96 = _getChainlinkPriceX96(token);
            chainlinkReferencePriceX96 = cachedChainlinkReferencePriceX96 == 0
                ? _getChainlinkPriceX96(referenceToken)
                : cachedChainlinkReferencePriceX96;

            chainlinkPriceX96 = (10 ** referenceTokenDecimals) * chainlinkPriceX96 * Q96 / chainlinkReferencePriceX96
                / (10 ** feedConfig.tokenDecimals);

            if (feedConfig.mode == Mode.TWAP_CHAINLINK_VERIFY) {
                verifyPriceX96 = chainlinkPriceX96;
            } else {
                priceX96 = chainlinkPriceX96;
            }
        }

        if (usesTWAP) {
            uint256 twapPriceX96 = _getTWAPPriceX96(feedConfig);
            if (feedConfig.mode == Mode.CHAINLINK_TWAP_VERIFY) {
                verifyPriceX96 = twapPriceX96;
            } else {
                priceX96 = twapPriceX96;
            }
        }

        if (feedConfig.mode == Mode.CHAINLINK_TWAP_VERIFY || feedConfig.mode == Mode.TWAP_CHAINLINK_VERIFY) {
            _requireMaxDifference(priceX96, verifyPriceX96, feedConfig.maxDifference);
        }
    }

Tools Used

Manual Review

Recommended Mitigation Steps

The function should check which oracle source is being used for priceX96 and verifyPriceX96 and adjust the difference calculation accordingly. For example:

if (usesChainlink && usesTWAP) {
    if (feedConfig.mode == Mode.CHAINLINK_TWAP_VERIFY) {
        priceX96 = chainlinkPriceX96;
        verifyPriceX96 = twapPriceX96;
    } else if (feedConfig.mode == Mode.TWAP_CHAINLINK_VERIFY) {
        priceX96 = twapPriceX96;
        verifyPriceX96 = chainlinkPriceX96;
    }

    uint256 priceDifferenceX96 = priceX96 > verifyPriceX96
        ? priceX96 - verifyPriceX96
        : verifyPriceX96 - priceX96;
    _requireMaxDifference(priceDifferenceX96, feedConfig.maxDifference);
}

This ensures that the difference calculation is always correct, regardless of which oracle source is being used for priceX96 and verifyPriceX96.

Assessed type

Math

c4-pre-sort commented 8 months ago

0xEVom marked the issue as insufficient quality report

c4-pre-sort commented 8 months ago

0xEVom marked the issue as duplicate of #10

0xEVom commented 8 months ago

Fails to articulate the root cause.

c4-judge commented 7 months ago

jhsagd76 marked the issue as partial-50