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

kaysoft - Chainlink price feed is not sufficiently validated and can return stale price #241

Closed
sherlock-admin opened this issue Mar 27, 2023 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Mar 27, 2023

kaysoft

medium

Chainlink price feed is not sufficiently validated and can return stale price

Summary

The price feed from chainlink oracle used in getLatestPrice function of ControllerPeggedAssetV2.sol should use the updateAt return value from the latestRound() data to ensure the answer is very recent with respect to time.

Vulnerability Detail

In the current implementation ControllerPeggedAssetV2.sol#getLatestPrice there is no validation to check if the answer is stale based on time as recommended by Chainlink: https://docs.chain.link/data-feeds/#check-the-timestamp-of-the-latest-answer

"Your application should track the latestTimestamp variable or use the updatedAt value from the latestRoundData() function to make sure that the latest answer is recent enough for your application to use it. If your application detects that the reported answer is not updated within the heartbeat or within time limits that you determine are acceptable for your application, pause operation or switch to an alternate operation mode while identifying the cause of the delay."

function getLatestPrice(address _token) public view returns (int256) {
        (
            ,
            /*uint80 roundId*/
            int256 answer,
            uint256 startedAt, /*uint256 updatedAt*/ /*uint80 answeredInRound*/
            ,

        ) = sequencerUptimeFeed.latestRoundData();

        // Answer == 0: Sequencer is up
        // Answer == 1: Sequencer is down
        bool isSequencerUp = answer == 0;
        if (!isSequencerUp) {
            revert SequencerDown();
        }

        // Make sure the grace period has passed after the sequencer is back up.
        uint256 timeSinceUp = block.timestamp - startedAt;
        if (timeSinceUp <= GRACE_PERIOD_TIME) {
            revert GracePeriodNotOver();
        }

        AggregatorV3Interface priceFeed = AggregatorV3Interface(
            vaultFactory.tokenToOracle(_token)
        );
        (uint80 roundID, int256 price, , , uint80 answeredInRound) = priceFeed
            .latestRoundData();
        uint256 decimals = priceFeed.decimals();

        if (decimals < 18) {
            decimals = 10**(18 - (decimals));
            price = price * int256(decimals);
        } else if (decimals == 18) {
            price = price;
        } else {
            decimals = 10**((decimals - 18));
            price = price / int256(decimals);
        }

        if (price <= 0) revert OraclePriceZero();

        if (answeredInRound < roundID) revert RoundIDOutdated();

        return price;
    }

Impact

Price oracle could return stale price which could result to loss of funds by users.

Code Snippet

File: https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Controllers/ControllerPeggedAssetV2.sol#L273

Tool used

Manual Review

Recommendation

Consider validating that the price is not stale based on the updatedAt value as recommended by Chainlink: https://docs.chain.link/data-feeds/#check-the-timestamp-of-the-latest-answer

if (updatedAt < block.timestamp - maxDelayTime) revert PRICE_OUTDATED();

Duplicate of #70

@github-actions github-actions bot closed this as completed Apr 3, 2023
@github-actions github-actions bot added the Excluded Excluded by the judge without consulting the protocol or the senior label Apr 3, 2023
@dmitriia dmitriia added Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label and removed Excluded Excluded by the judge without consulting the protocol or the senior labels Apr 10, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Apr 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

2 participants