Precision loss while downscaling Chainlink price feed

Summary

Chainlink price responses are downscaled to 6 decimals, causing a potential loss of value due to the precision reduction of prices used in intermediary calculations.

Vulnerability Detail

Prices coming from the Chainlink oracle are always downscaled and stored in 6 decimals precision:

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

This allows a normalization of collateral prices in the domain of 6 decimals, but causes a premature loss of precision as prices will be stored with the reduced precision before they are effectively used.

The loss of precision will be in the order of priceFeedDecimals - 6, so when priceFeedDecimals > 6 the effective loss of precision in the price representation will be 10 ** (priceFeedDecimals - 6) - 1. Note that these represent scalar values, prices are expected to be later projected into net amounts in calculations such as collateralUsdBalance() or getDollarInCollateral().

Impact

Premature downscaling of oracle prices will cause a loss of precision of up to 10 ** (priceFeedDecimals - 6) - 1 that will be carried forward to getDollarInCollateral(), a key function that is used to convert between dollars and collateral while minting and redeeming.

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L554-L557

Tool used

Manual Review

Recommendation

Store the prices in full precision, along with the feed decimals. Normalize decimals when these are later used in getDollarInCollateral().

sherlock-admin2 commented 10 months ago

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

auditsea commented:

It's protocol decision, and since it's stablecoin price, 1e-6 precision is enough