code-423n4 / 2024-05-predy-findings

10 stars 9 forks source link

No check if Arbitrum L2 sequencer is down in Chainlink feeds #212

Closed howlbot-integration[bot] closed 4 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-05-predy/blob/main/src/PriceFeed.sol#L46

Vulnerability details

Impact

Using Chainlink in L2 chains such as Arbitrum requires to check if the sequencer is down to avoid prices from looking like they are fresh although they are not.

The bug could be leveraged by malicious actors to take advantage of the sequencer downtime.

Proof of Concept

according to protocol docs:

Chains the protocol will be deployed on Arbitrum,Base,Optimism

we can see the procotol will be deployed on L2 chains.

according to chainlink docs:

L2 sequencer feeds track the last known status of the sequencer on an L2 network at a given point in time. This helps you prevent mass liquidations by providing a grace period to allow customers to react to these events.

PriceFeed.sol

    function getSqrtPrice() external view returns (uint256 sqrtPrice) {
>       (, int256 quoteAnswer,,,) = AggregatorV3Interface(_quotePriceFeed).latestRoundData();

        IPyth.Price memory basePrice = IPyth(_pyth).getPriceNoOlderThan(_priceId, VALID_TIME_PERIOD);

        require(basePrice.expo == -8, "INVALID_EXP");

        require(quoteAnswer > 0 && basePrice.price > 0);

        uint256 price = uint256(int256(basePrice.price)) * Constants.Q96 / uint256(quoteAnswer);
        price = price * Constants.Q96 / _decimalsDiff;

        sqrtPrice = FixedPointMathLib.sqrt(price);
    }

Tools Used

Foundry

Recommended Mitigation Steps

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

Assessed type

Invalid Validation

c4-judge commented 4 months ago

alex-ppg changed the severity to QA (Quality Assurance)

c4-judge commented 4 months ago

alex-ppg marked the issue as grade-c

c4-judge commented 4 months ago

This previously downgraded issue has been upgraded by alex-ppg

c4-judge commented 4 months ago

alex-ppg changed the severity to QA (Quality Assurance)