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

kaysoft - Missing check for Whether Chainlink L2 sequencer is ACTIVE in getMarkPrice() function #233

Closed
sherlock-admin opened this issue May 10, 2023 · 0 comments
Labels
Non-Reward This issue will not receive a payout

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented May 10, 2023

kaysoft

high

Missing check for Whether Chainlink L2 sequencer is ACTIVE in getMarkPrice() function

Summary

The getMarkPrice function of the chainlinkAdaptor.sol file does not check for whether the sequence is active as recommeded by chainlink for getting price feeds on L2 networks like Arbitrum.

Chainlink recommends that for L2 like Arbitrum, applications should check if the sequencer is available by using data feeds that tracks the last known of the sequencer at a given point in time in order to prevent mass liquidations by providing a grace period to allow users to react to such an event.
Please read: https://docs.chain.link/data-feeds/l2-sequencer-feeds

Vulnerability Detail

The getMarkPrice function of the chainlinkAdaptor.sol file does not check for whether the sequence is active as recommended by chainlink for getting price feeds on L2 networks like Arbitrum.
Checks for sequencer uptime should be implemented and revert if the sequencer is down.
Take a look at this example code on Chainlink Docs: https://docs.chain.link/data-feeds/l2-sequencer-feeds#example-code

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.7;

import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV2V3Interface.sol";

/**
 * THIS IS AN EXAMPLE CONTRACT THAT USES HARDCODED VALUES FOR CLARITY.
 * THIS IS AN EXAMPLE CONTRACT THAT USES UN-AUDITED CODE.
 * DO NOT USE THIS CODE IN PRODUCTION.
 */

contract PriceConsumerWithSequencerCheck {
    AggregatorV2V3Interface internal priceFeed;
    AggregatorV2V3Interface internal sequencerUptimeFeed;

    uint256 private constant GRACE_PERIOD_TIME = 3600;

    error SequencerDown();
    error GracePeriodNotOver();

    /**
     * Network: Optimism
     * Data Feed: BTC/USD
     * Data Feed Proxy Address: 0xD702DD976Fb76Fffc2D3963D037dfDae5b04E593
     * Sequencer Uptime Proxy Address: 0x371EAD81c9102C9BF4874A9075FFFf170F2Ee389
     * For a list of available sequencer proxy addresses, see:
     * https://docs.chain.link/docs/data-feeds/l2-sequencer-feeds/#available-networks
     */
    constructor() {
        priceFeed = AggregatorV2V3Interface(
            0xD702DD976Fb76Fffc2D3963D037dfDae5b04E593
        );
        sequencerUptimeFeed = AggregatorV2V3Interface(
            0x371EAD81c9102C9BF4874A9075FFFf170F2Ee389
        );
    }

    // Check the sequencer status and return the latest price
    function getLatestPrice() public view returns (int) {
        // prettier-ignore
        (
            /*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();
        }

        // prettier-ignore
        (
            /*uint80 roundID*/,
            int price,
            /*uint startedAt*/,
            /*uint timeStamp*/,
            /*uint80 answeredInRound*/
        ) = priceFeed.latestRoundData();

        return price;
    }
}

Impact

If the sequencer goes down, stale prices may be returned and for example if USDC were to de-peg while the sequencer is offline, stale price is used and can result in massive false liquidation and over-borrowing.

Code Snippet

https://github.com/JOJOexchange/smart-contract-EVM/blob/4a95a8e9a6367ae88dc827e29467229cb5bbad4f/contracts/adaptor/chainlinkAdaptor.sol#L43

Tool used

Manual Review

Recommendation

Implement the chainlink L2 sequencer check since the contract will be deployed to Arbitrum. Take a look at this example here: https://docs.chain.link/data-feeds/l2-sequencer-feeds

Duplicate of #101

@github-actions github-actions bot added Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels May 17, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label May 30, 2023
@sherlock-admin sherlock-admin added Non-Reward This issue will not receive a payout and removed Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Reward A payout will be made for this issue labels Jun 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Non-Reward This issue will not receive a payout
Projects
None yet
Development

No branches or pull requests

1 participant