code-423n4 / 2024-04-revert-mitigation-findings

1 stars 1 forks source link

M-07 MitigationConfirmed #3

Open c4-bot-2 opened 4 months ago

c4-bot-2 commented 4 months ago

Lines of code

Vulnerability details

C4 issue

M-07: Large decimal of referenceToken causes overflow at oracle price calculation

Comments

The original implementation to scale chainlink price with reference token is as follows:

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

With chainlinkPriceX96 type is uint256, suppose referenceTokenDecimals = 18 then this will overflow if chainlinkPriceX96 >= 2**256/(Q96* 10**18) => chainlinkPrice >= 2**256(Q96 * Q96 * 10**18) => chainlinkPrice > 18.4. This is totally possible if the input token is ETH and ETH/USDC feed is used (ETH price is much larger than 18.4 USDC)

Mitigation

PR #21

In the mitigated code, the calculation is updated to:

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;
            }

In the previous example, this updated code can only overflow if chainlinkPrice > 2**256/(Q96*Q96*10**10) (because referenceTokenDecimals = 18 and feedConfig.tokenDecimals = 8) or chainlinkPrice > 1844674407 (1.8 billions USD), which is far above reasonable values.

This implementation is also recommended by chainlink doc

The mitigation resolved the original issue.

Conclusion

LGTM

c4-judge commented 4 months ago

jhsagd76 marked the issue as satisfactory

c4-judge commented 4 months ago

jhsagd76 marked the issue as confirmed for report