code-423n4 / 2023-07-moonwell-findings

1 stars 0 forks source link

ChainlinkOracle assumes that the assets of all USD denominated pair has 18 decimal places #304

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/fced18035107a345c31c9a9497d0da09105df4df/src/core/Oracles/ChainlinkOracle.sol#L97-L114

Vulnerability details

Proof of Concept

The getChainlinkPrice() function in the ChainlinkOracle contract assumes that all USD-denominated feeds store answers at 8 decimals and assumes that all assets has 18 decimal places.

->      // Chainlink USD-denominated feeds store answers at 8 decimals
        uint256 decimalDelta = uint256(18).sub(feed.decimals());
        // Ensure that we don't multiply the result by 0
        if (decimalDelta > 0) {
            return uint256(answer).mul(10 ** decimalDelta);
        } else {
            return uint256(answer);
        }

However, there are some USD-denominated feeds that do not return 8 decimals and some assets that do not have 18 decimal places. For example, the USDC/USD feed returns 8 decimal places but USDC has 6 decimal places. The USDC token will return 18 decimal places instead of 6 decimal places

USDC/USD feed: https://etherscan.io/address/0x8fffffd4afb6115b954bd326cbe7b4ba576818f6#readContract

Impact

The returned price can be inflated by 12 decimal places.

Tools Used

Manual Review

Recommended Mitigation Steps

Do not assume that all assets in the Chainlink USD-denominated feeds return 18 decimals. Check each asset's decimal places before swapping to 18 decimal places.

Assessed type

Decimal

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as primary issue

ElliotFriedman commented 1 year ago

comment differs from implementation, see code

c4-sponsor commented 1 year ago

ElliotFriedman marked the issue as sponsor disputed

alcueca commented 1 year ago

Downgraded to QA, please fix the comment.

c4-judge commented 1 year ago

alcueca changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

alcueca marked the issue as grade-a