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

dimah7 - Deployments to L2 networks should check for active sequencer for Chainlink feeds #118

Closed
github-actions bot opened this issue Apr 22, 2024 · 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

@github-actions
Copy link

github-actions bot commented Apr 22, 2024

dimah7

medium

Deployments to L2 networks should check for active sequencer for Chainlink feeds

Summary

As mentioned in the protocol's Q&A for the contest: "Optimism and Arbitrum are currently live. Future plans include Layer 2 chains such as Metis, Linea, and Base". As of right now protocol's current usage of Chainlink price feeds doesn't consider whether the sequencer is down for deployments on L2 blockchains. It's important to ensure that the prices provided are not falsely perceived as fresh, even when the sequencer is down. It should be always checked before consuming any data from Chainlink.

Vulnerability Detail

There is no check if the sequencer is down:

function getExpectedExchange(address yieldToken) external view returns (uint256) {
        .
        .
        .
        // Ensure that round is complete, otherwise price is stale.
        (
            uint80 roundID,
            int256 opToUsd,
            ,
            uint256 updateTime,
            uint80 answeredInRound
        ) = IChainlinkOracle(opToUsdOracle).latestRoundData();
        
        require(
            opToUsd > 0, 
            "Chainlink Malfunction"
        );

        if( updateTime < block.timestamp - 1200 seconds ) {
            revert("Chainlink Malfunction");
        }

        // Ensure that round is complete, otherwise price is stale.
        (
            uint80 roundIDEth,
            int256 ethToUsd,
            ,
            uint256 updateTimeEth,
            uint80 answeredInRoundEth
        ) = IChainlinkOracle(ethToUsdOracle).latestRoundData();
        
        require(
            ethToUsd > 0, 
            "Chainlink Malfunction"
        );

        if( updateTimeEth < block.timestamp - 1200 seconds ) {
            revert("Chainlink Malfunction");
        }
        .
        .
        .

Examples from similar issues/reports on previous Sherlock contests/audits can be seen here:

https://solodit.xyz/issues/m-5-getpricefromchainlink-doesnt-check-if-arbitrum-sequencer-is-down-in-chainlink-feeds-sherlock-none-iron-bank-git

https://solodit.xyz/issues/m-4-no-check-if-arbitrum-l2-sequencer-is-down-in-chainlink-feeds-sherlock-sentiment-sentiment-update-3-git

https://solodit.xyz/issues/trst-m-3-no-check-for-active-arbitrum-sequencer-in-chainlink-oracle-trust-security-none-stella-markdown_

Impact

The reward collector is attempting to claim, but the sequencer is down, and this results in stale/invalid values given to be claimed.

Code Snippet

https://github.com/sherlock-audit/2024-04-alchemix/blob/32e4902f77b05ea856cf52617c55c3450507281c/v2-foundry/src/utils/collectors/OptimismRewardCollector.sol#L91-L141

Tool used

Manual Review

Recommendation

Implement functionality to check the sequencer uptime with Chainlink oracles for deploying to L2s.

Reference here: https://docs.chain.link/data-feeds/l2-sequencer-feeds#example-code

Duplicate of #14

@github-actions github-actions bot added Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue labels Apr 22, 2024
@sherlock-admin4 sherlock-admin4 changed the title Glamorous Tan Flamingo - Deployments to L2 networks should check for active sequencer for Chainlink feeds dimah7 - Deployments to L2 networks should check for active sequencer for Chainlink feeds Apr 30, 2024
@sherlock-admin4 sherlock-admin4 added the Reward A payout will be made for this issue label Apr 30, 2024
@WangSecurity WangSecurity added Excluded Excluded by the judge without consulting the protocol or the senior and removed High A valid High severity issue labels May 14, 2024
@sherlock-admin2 sherlock-admin2 added Non-Reward This issue will not receive a payout and removed Reward A payout will be made for this issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels May 14, 2024
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

3 participants