Price should be checked after it has been reduced to e6 precision

Summary

After reducing the price to e6 the price value might become zero .

Vulnerability Detail

Following is the code where price is reduced to e6 precision

   // convert chainlink price to 6 decimals
        uint256 price = uint256(answer).mul(UBIQUITY_POOL_PRICE_PRECISION).div(
            10 ** priceFeedDecimals
        );
 poolStorage.collateralPrices[collateralIndex] = price;

So there may be a case when the answer value that is returned by the chainlink feed address is too low as compared to the price feed decimals due to which even if the answer is multiplied with e6 if results zero then the price of collateral can be set to zero

Impact

If the price is set to zero then in the following function where it is used can cause erroneous calculation

 function collateralUsdBalance()
        internal
        view
        returns (uint256 balanceTally)
    {
        UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage();
        uint256 collateralTokensCount = poolStorage.collateralAddresses.length;
        balanceTally = 0;
        for (uint256 i = 0; i < collateralTokensCount; i++) {
            balanceTally += freeCollateralBalance(i)
                .mul(10 ** poolStorage.missingDecimals[i])
    @=>        .mul(poolStorage.collateralPrices[i])
                .div(UBIQUITY_POOL_PRICE_PRECISION);
        }
    }

function getDollarInCollateral(
        uint256 collateralIndex,
        uint256 dollarAmount
    ) internal view returns (uint256) {
        UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage();
        return
            dollarAmount
                .mul(UBIQUITY_POOL_PRICE_PRECISION)
                .div(10 ** poolStorage.missingDecimals[collateralIndex])
                .div(poolStorage.collateralPrices[collateralIndex]);
    }

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L284 https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L247

Tool used

Manual Review

Recommendation

Add the following

function updateChainLinkCollateralPrice(uint256 collateralIndex) internal {
        UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage();

        AggregatorV3Interface priceFeed = AggregatorV3Interface(
            poolStorage.collateralPriceFeedAddresses[collateralIndex]
        );

        // fetch latest price
        (
            ,
            // roundId
            int256 answer, // startedAt
            ,
            uint256 updatedAt,

        ) = // answeredInRound
            priceFeed.latestRoundData();

        // fetch number of decimals in chainlink feed
        uint256 priceFeedDecimals = priceFeed.decimals();

        // validation
        require(answer > 0, "Invalid price");
        require(
            block.timestamp - updatedAt <
                poolStorage.collateralPriceFeedStalenessThresholds[
                    collateralIndex
                ],
            "Stale data"
        );

        // convert chainlink price to 6 decimals
        uint256 price = uint256(answer).mul(UBIQUITY_POOL_PRICE_PRECISION).div(
            10 ** priceFeedDecimals
        );
@=> require(price > 0 ,"Invalid Price");
        poolStorage.collateralPrices[collateralIndex] = price;

        emit CollateralPriceSet(collateralIndex, price);
    }

sherlock-admin2 commented 8 months ago

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

auditsea commented:

Literally, price can not be zero