Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

staleCheckLatestRoundData() doesn't check Arbitrum l2 chainlink feed is active #1046

Open
codehawks-bot opened this issue Aug 5, 2023 · 0 comments

Comments

@codehawks-bot
Copy link

staleCheckLatestRoundData() doesn't check Arbitrum l2 chainlink feed is active

Severity

Medium Risk

Relevant GitHub Links

/*
* @title OracleLib
* @author Patrick Collins
* @notice This library is used to check the Chainlink Oracle for stale data.
* If a price is stale, the function will revert, and render the DSCEngine unusable - this is by design.
* We want the DSCEngine to freeze if prices become stale.
*
* So if the Chainlink network explodes and you have a lot of money locked in the protocol... too bad.
*/
library OracleLib {
error OracleLib__StalePrice();
uint256 private constant TIMEOUT = 3 hours; // 3 * 60 * 60 = 10800 seconds
function staleCheckLatestRoundData(AggregatorV3Interface priceFeed)
public
view
returns (uint80, int256, uint256, uint256, uint80)
{
(uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) =
priceFeed.latestRoundData();
uint256 secondsSince = block.timestamp - updatedAt;
if (secondsSince > TIMEOUT) revert OracleLib__StalePrice();
return (roundId, answer, startedAt, updatedAt, answeredInRound);
}
function getTimeout(AggregatorV3Interface /* chainlinkFeed */ ) public pure returns (uint256) {
return TIMEOUT;
}
}

Summary

When utilizing Chainlink in L2 chains like Arbitrum, it's important to ensure that the prices provided are not falsely perceived as fresh, even when the sequencer is down. This vulnerability could potentially be exploited by malicious actors to gain an unfair advantage.

https://pasteboard.co/V4gFHOaMuQOz.png

https://discord.com/channels/1127263608246636635/1129041670864916510/1136582556033232917

It was mentioned in the Foundry-defi-stablecoin discord channel that this project will be deployed to any evm-chain. If this project is to be deployed to the Arbitrum network, the staleCheckLatestRoundData() function should be improved and other controls added, for example sequencer is down.

If a sequencer becomes unavailable, it is impossible to access read/write APIs that consumers are using and applications on the L2 network will be down for most users without interacting directly through the L1 optimistic rollup contracts. The L2 has not stopped, but it would be unfair to continue providing service on your applications when only a few users can use them.

Vulnerability Details

there is no check

/*
* @title OracleLib
* @author Patrick Collins
* @notice This library is used to check the Chainlink Oracle for stale data.
* If a price is stale, the function will revert, and render the DSCEngine unusable - this is by design.
* We want the DSCEngine to freeze if prices become stale.
*
* So if the Chainlink network explodes and you have a lot of money locked in the protocol... too bad.
*/
library OracleLib {
error OracleLib__StalePrice();
uint256 private constant TIMEOUT = 3 hours; // 3 * 60 * 60 = 10800 seconds
function staleCheckLatestRoundData(AggregatorV3Interface priceFeed)
public
view
returns (uint80, int256, uint256, uint256, uint80)
{
(uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) =
priceFeed.latestRoundData();
uint256 secondsSince = block.timestamp - updatedAt;
if (secondsSince > TIMEOUT) revert OracleLib__StalePrice();
return (roundId, answer, startedAt, updatedAt, answeredInRound);
}
function getTimeout(AggregatorV3Interface /* chainlinkFeed */ ) public pure returns (uint256) {
return TIMEOUT;
}
}

/*
 * @title OracleLib
 * @author Patrick Collins
 * @notice This library is used to check the Chainlink Oracle for stale data.
 * If a price is stale, the function will revert, and render the DSCEngine unusable - this is by design.
 * We want the DSCEngine to freeze if prices become stale. 
 * 
 * So if the Chainlink network explodes and you have a lot of money locked in the protocol... too bad. 
 */
library OracleLib {
    error OracleLib__StalePrice();


    uint256 private constant TIMEOUT = 3 hours; // 3 * 60 * 60 = 10800 seconds


    function staleCheckLatestRoundData(AggregatorV3Interface priceFeed)
        public
        view
        returns (uint80, int256, uint256, uint256, uint80)
    {
        (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) =
            priceFeed.latestRoundData();


        uint256 secondsSince = block.timestamp - updatedAt;
        if (secondsSince > TIMEOUT) revert OracleLib__StalePrice();


        return (roundId, answer, startedAt, updatedAt, answeredInRound);
    }


    function getTimeout(AggregatorV3Interface /* chainlinkFeed */ ) public pure returns (uint256) {
        return TIMEOUT;
    }
}

Impact

If the Arbitrum sequencer goes down, the protocol will allow users to continue to operate at the previous (stale) rates.

Tools Used

vscode

Recommendations

It is recommended to follow the code example of Chainlink:
https://docs.chain.link/data-feeds/l2-sequencer-feeds#example-code

Same issue in other contests

sherlock-audit/2023-02-bond-judging#1

sherlock-audit/2022-11-sentiment-judging#3

sherlock-audit/2023-01-sentiment-judging#16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants