sherlock-audit / 2023-07-blueberry-judging

2 stars 1 forks source link

nisedo - Hardcoded Decimal Precision in `ChainlinkAdapterOracleL2.getPrice()` #133

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

nisedo

medium

Hardcoded Decimal Precision in ChainlinkAdapterOracleL2.getPrice()

Summary

The BlueBerryBank contract utilizes a hardcoded constant for deriving the price from the Chainlink price feed, which assumes a representation with 8 decimals. This becomes problematic when integrating tokens like AMPL, which Chainlink price feed returns with 18 decimals. Such an approach restricts the system's adaptability to different decimal precisions across various Chainlink price feeds and could cause the protocol to enter a state where user funds can be lost.

Vulnerability Detail

The contract ChainlinkAdapterOracleL2.sol employs a hardcoded constant Constants.CHAINLINK_PRICE_FEED_PRECISION to adjust the price returned from the Chainlink price feed, assuming it represents the value with 8 decimals. This approach presupposes that all Chainlink price feeds return values with 8 decimals. However, not all Chainlink feeds conform to this assumption. For instance, the AMPL/USD Chainlink feed actually returns values with 18 decimals.

Comparatively, in ChainlinkAdapterOracle.sol, the system uses a dynamic approach by dividing with 10 ** decimals, which automatically adapts to the precision of the returned price feed. The hardcoded approach in ChainlinkAdapterOracleL2.sol makes it inflexible and incompatible with any Chainlink feed that doesn't strictly return 8 decimals.

Impact

Code Snippet

ChainlinkAdapterOracleL2.sol#L144-L146

return (price.toUint256() * Constants.PRICE_PRECISION) / Constants.CHAINLINK_PRICE_FEED_PRECISION; //@audit-issue AMPL/USD = 18 decimals
/// @dev Precision factor for Chainlink price feed values.
uint256 constant CHAINLINK_PRICE_FEED_PRECISION = 1e8; //@audit-issue not all XXX/USD Chainlink price feeds return 8 decimals

Tool used

Manual Review

Recommendation

Replace the hardcoded constant with a dynamic approach that fetches the decimal precision directly from the Chainlink feed for the respective token as it is done in ChainlinkAdapterOracle.sol:

        /// Get token-USD price
        uint256 decimals = registry.decimals(token, USD);
        (uint80 roundID, int256 answer,, uint256 updatedAt, uint80 answeredInRound) =
            registry.latestRoundData(token, USD);
        if (updatedAt < block.timestamp - maxDelayTime) {
            revert Errors.PRICE_OUTDATED(token_);
        }
        if (answer <= 0) revert Errors.PRICE_NEGATIVE(token_);
        if (answeredInRound < roundID) revert Errors.PRICE_OUTDATED(token_);

        return (answer.toUint256() * Constants.PRICE_PRECISION) / 10 ** decimals;
sherlock-admin2 commented 1 year ago

2 comment(s) were left on this issue during the judging contest.

0xyPhilic commented:

invalid because tokens used are whitelisted so this issue can be considered informational

darkart commented:

Invalid

Gornutz commented 1 year ago

All tokens are whitelisted. Additionally, at the moment all price feeds from chainlink on non-mainnet environments are 8 decimal places when quoted against USD.

Shogoki commented 1 year ago

All tokens are whitelisted. Additionally, at the moment all price feeds from chainlink on non-mainnet environments are 8 decimal places when quoted against USD.

Because of the whitelisting of tokens and the fact that most USD oracles use 8 decimals this issue is judged as low/info.