code-423n4 / 2022-05-bunker-findings

1 stars 0 forks source link

ChainLink's `.decimals()` function never checked #96

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/PriceOracleImplementation.sol#L22-L32

Vulnerability details

Impact

The response from the ChainlinkFeed always assumes 8 decimals: but it’s never actually checked if the response has 8 decimals using .decimals() function.

Proof of Concept

    function getUnderlyingPrice(CToken cToken) external view returns (uint) {
        if (address(cToken) == cEtherAddress) {
            // ether always worth 1
            return 1e18;
        }

        // For now, we only have USDC and ETH.
        int256 usdcPrice = ChainlinkFeed(0x986b5E1e1755e3C2440e960477f25201B0a8bbD4).latestAnswer();
        if (usdcPrice <= 0) {
            return 0;
        }

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/PriceOracleImplementation.sol#L22-L32

ChainLink Data Feeds API Reference

Tools Used

Manual Review

Recommended Mitigation Steps

Consider checking if the response has 8 decimals using .decimals() function.

bunkerfinance-dev commented 2 years ago

We think we can assume that the decimals will stay constant here.

gzeoneth commented 2 years ago

Since PriceOracleImplementation is hardcoding the USDC/ETH here, this seems to be fine and save gas.