code-423n4 / 2024-03-revert-lend-findings

7 stars 7 forks source link

Large decimal of referenceToken causes overflow at oracle price calculation #409

Open c4-bot-3 opened 6 months ago

c4-bot-3 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Oracle.sol#L304

Vulnerability details

Impact

The price calculation at the V3Oracle.sol will revert upon reaching certain level when referenceToken is used with high decimal value (e. g. 18). The revert (specifically happening when calling getValue()) would make the Chainlink price feed useless. Yet the TWAP price source would still be available. The protocol team would have to disable Chainlink and rely exclusively on the TWAP source reducing security of the pricing. The issue could manifest itself after certain amount of time once the project is already live and only when price returned by the feed reaches certain point.

Proof of Concept

The following code line has an issue:

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

When referenceTokenDecimals is 18 chainlinkPriceX96 is higher than some threshold between 18 and 19 (in Q96 notation), it will cause arithmetic overflow.

Tools Used

Manual review.

Recommended Mitigation Steps

Instead of calculating the price this way:

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

It could be done the following way as per Chainlink's recommendation:

if (referenceTokenDecimals > feedConfig.tokenDecimals)
            chainlinkPriceX96 = (10 ** referenceTokenDecimals - feedConfig.tokenDecimals) * chainlinkPriceX96 * Q96 
            / chainlinkReferencePriceX96;
        else if (referenceTokenDecimals < feedConfig.tokenDecimals)
            chainlinkPriceX96 = chainlinkPriceX96 * Q96 / chainlinkReferencePriceX96 
            / (10 ** feedConfig.tokenDecimals - referenceTokenDecimals);
        else 
            chainlinkPriceX96 = chainlinkPriceX96 * Q96 / chainlinkReferencePriceX96;

https://docs.chain.link/data-feeds/using-data-feeds#getting-a-different-price-denomination

Assessed type

Decimal

c4-pre-sort commented 6 months ago

0xEVom marked the issue as primary issue

c4-pre-sort commented 6 months ago

0xEVom marked the issue as sufficient quality report

c4-sponsor commented 5 months ago

kalinbas (sponsor) confirmed

c4-judge commented 5 months ago

jhsagd76 marked the issue as satisfactory

c4-judge commented 5 months ago

jhsagd76 marked the issue as selected for report

kalinbas commented 5 months ago

https://github.com/revert-finance/lend/pull/21