Skip to content
This repository has been archived by the owner on Nov 19, 2023. It is now read-only.

0xChinedu - Chainlink's latestRoundData return stale or incorrect result #54

Closed
sherlock-admin opened this issue May 15, 2023 · 0 comments
Labels
Excluded Excluded by the judge without consulting the protocol or the senior Non-Reward This issue will not receive a payout

Comments

@sherlock-admin
Copy link
Contributor

0xChinedu

medium

Chainlink's latestRoundData return stale or incorrect result

Summary

Chainlink's latestRoundData() is used but there is no check if the return value indicates stale data. This could lead to stale prices according to the Chainlink documentation:

https://docs.chain.link/docs/historical-price-data/#historical-rounds

Vulnerability Detail

In ExchangeRate.sol, you are using latestRoundData, but there is no check if the return value indicates stale data.

    function buildExchangeRate(uint256 currencyId) internal view returns (ETHRate memory) {
        mapping(uint256 => ETHRateStorage) storage store = LibStorage.getExchangeRateStorage();
        ETHRateStorage storage ethStorage = store[currencyId];

        int256 rateDecimals;
        int256 rate;
        if (currencyId == Constants.ETH_CURRENCY_ID) {
            // ETH rates will just be 1e18, but will still have buffers, haircuts,
            // and liquidation discounts
            rateDecimals = Constants.ETH_DECIMALS;
            rate = Constants.ETH_DECIMALS;
        } else {
            // prettier-ignore
            (
                /* roundId */,
                rate,
                /* uint256 startedAt */,
                /* updatedAt */,
                /* answeredInRound */
            ) = ethStorage.rateOracle.latestRoundData();
            require(rate > 0);

            // No overflow, restricted on storage
            rateDecimals = int256(10**ethStorage.rateDecimalPlaces);
            if (ethStorage.mustInvert) {
                rate = rateDecimals.mul(rateDecimals).div(rate);
            }
        }

        int256 buffer = ethStorage.buffer;
        if (buffer > Constants.MIN_BUFFER_SCALE) {
            // Buffers from 100 to 150 are 1-1 (i.e. a buffer of 150 is a 150% increase)
            // in the debt amount. Buffers from 151 to 255 are scaled by a multiple of 10
            // units to allow for much higher buffers at the outer limits. A stored
            // buffer value of 151 = 150 + 10 = 160.
            // The max buffer value of of 255 = 150 + 105 * 10 = 1200.

            // No possibility of overflows due to storage size and constant definition
            buffer = (buffer - Constants.MIN_BUFFER_SCALE) * Constants.BUFFER_SCALE 
                + Constants.MIN_BUFFER_SCALE;
        }

        return
            ETHRate({
                rateDecimals: rateDecimals,
                rate: rate,
                buffer: buffer,
                haircut: ethStorage.haircut,
                liquidationDiscount: ethStorage.liquidationDiscount
            });
    }
}

This could lead to stale prices according to the Chainlink documentation:
https://docs.chain.link/data-feeds/price-feeds/historical-data
Related report:
code-423n4/2021-05-fairside-findings#70

Impact

Function could return stale price data for the underlying asset.

Code Snippet

https://github.com/sherlock-audit/2023-03-notional/blob/main/contracts-v2/contracts/internal/valuation/ExchangeRate.sol#L57

Tool used

Manual Review

Recommendation

Consider Adding Checks For Stale Data

@github-actions github-actions bot added the Excluded Excluded by the judge without consulting the protocol or the senior label May 19, 2023
@sherlock-admin sherlock-admin added the Non-Reward This issue will not receive a payout label May 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Excluded Excluded by the judge without consulting the protocol or the senior Non-Reward This issue will not receive a payout
Projects
None yet
Development

No branches or pull requests

1 participant