The price will be wrong when the token's USD price feed's decimals != 8

zlsecure3 commented 1 year ago

subject

description

PriceOracle assumes and inexplicitly requires the token's USD feed's decimals to be 8. However, there are certain token's USD feed has a different decimals.

In the current implementation, it assumes basePriceDecimals == quotePriceDecimals

    function getIndexPrice(
        address priceOracle,
        uint256 baseAssetId,
        uint256 quotePrice
    ) internal view returns (uint256) {
        uint256 basePrice = IPriceOracle(priceOracle).getPriceFeed(baseAssetId);
        return (basePrice * 1e8) / quotePrice;
    }

However, there are tokens with USD price feed's decimals != 8 (E.g: AMPL / USD feed decimals = 18)

When the token's USD feed's decimals != 8, getIndexPrice() will return an incorrect price.

recommendation

Consider adding a check for feed.decimals() to make sure feed's decimals = 8:

        if (block.timestamp - vars.hfOracleUpdatedAt > 10) {
            if (feed.defaultOracle != address(0)) {
                (
                    ,
                    vars.defaultOraclePrice,
                    ,
                    vars.defaultOracleUpdatedAt,

                ) = IAggregatorV3Interface(feed.defaultOracle)
                    .latestRoundData();
+              require(IAggregatorV3Interface(feed.defaultOracle).decimals() == 8, "...");

            }
        }

locations

code/contracts/libraries/utils/PriceUtil.sol#L36-L44
core/price-oracle/PriceOracle.sol#L140-L166

severity

Medium

damage

exploitability

grading (edit)

submission_id:1772418690

review_type:GRADING

result: TBD-yes,no

rating: TBD-123

comment: TBD-Rejected,Accepted by Secure3.

severity: TBD-Critical,Medium,Low,Informational

category:

description:

zlsecure3 commented 1 year ago

client feedback (manual copy)

submission_id:1772418690

review_type:CLIENT_FEEDBACK

result: TBD-yes,no

severity: TBD-Critical,Medium,Low,Informational

comment:

zlsecure3 commented 1 year ago

client feedback decision(edit)

submission_id:1772418690

review_type:CLIENT_FEEDBACK_DECISION

result: TBD-yes,no,yes-honored,no-honored

severity: TBD-Critical,Medium,Low,Informational

comment:

zlsecure3 / review_Aark