sherlock-audit / 2023-04-blueberry-judging

8 stars 5 forks source link

Brenzee - No checks if Arbitrum sequencer is down in Chainlink feeds #75

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Brenzee

medium

No checks if Arbitrum sequencer is down in Chainlink feeds

Summary

In Q&A section Blueberry mentioned that contracts will be deployed on Mainnet and Arbitrum chains.

When using Chainlink on second-layer chains such as Arbitrum, it is required to ensure that the sequencer is up to avoid prices from looking like they are fresh although they are not.

Vulnerability Detail

There is no check made to sequencer

function getPrice(address token_) external view override returns (uint256) {
        // remap token if possible
        address token = remappedTokens[token_];
        if (token == address(0)) token = token_;

        uint256 maxDelayTime = timeGaps[token];
        if (maxDelayTime == 0) revert Errors.NO_MAX_DELAY(token_);

        // Get token-USD price
        uint256 decimals = registry.decimals(token, USD);
        (, int256 answer, , uint256 updatedAt, ) = registry.latestRoundData(
            token,
            USD
        );
        if (updatedAt < block.timestamp - maxDelayTime)
            revert Errors.PRICE_OUTDATED(token_);
        if (answer <= 0) revert Errors.PRICE_NEGATIVE(token_);

        return
            (answer.toUint256() * Constants.PRICE_PRECISION) / 10 ** decimals;
    }

Impact

Can be exploited by malicious users to get an advantage.

Code Snippet

https://github.com/sherlock-audit/2023-04-blueberry/blob/main/blueberry-core/contracts/oracle/ChainlinkAdapterOracle.sol#L77-L97

Tool used

Manual Review

Recommendation

I would recommend to create a 2 different contracts - one for Mainnet and one for Arbitrum, where the sequencer is checked. Example: https://docs.chain.link/data-feeds/l2-sequencer-feeds#example-code

Duplicate of #142