boredpukar - getExpectedExchange function doesn't check if L2 sequencer is down in Chainlink feeds.

[github-actions]

boredpukar

medium

getExpectedExchange function doesn't check if L2 sequencer is down in Chainlink feeds.

Summary

When utilizing Chainlink feeds in L2 chains like Arbitrum and Optimism, 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.

Vulnerability Detail

The reward router/collector is only currently intended to be deployed on Optimism, Arbitrum, and Ethereum.

Chainlink recommends that users using price oracles, check whether the L2 based sequencer (for Arbitrum and Optimism) is active.

Quoting from the documentation:

To help your applications identify when the sequencer is unavailable, you can use a data feed that tracks the last known status of the sequencer at a given point in time. This helps you prevent mass liquidations by providing a grace period to allow customers to react to such an event.

Users are still able in principle to avoid liquidations by interacting with the L2 delayed inbox via L1, but this is out of reach for most users. If the sequencer goes down, the oracles may have stale prices, since L2-submitted transactions (i.e. by the aggregating oracles) will not be processed.

Impact

Stale Price. If USDC were to de-peg while the sequencer is offline, stale price is used for the subsequent transactions.

Code Snippet

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

function getExpectedExchange(address yieldToken) external view returns (uint256) {
        uint256 expectedExchange;
        address[] memory token = new address[](1);
        uint256 totalToSwap = TokenUtils.safeBalanceOf(rewardToken, address(this));

        // 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");
        }

        // Find expected amount out before calling harvest
        if (debtToken == alUsdOptimism) {
            expectedExchange = totalToSwap * uint(opToUsd) / 1e8;
        } else if (debtToken == alEthOptimism) {
            expectedExchange = totalToSwap * uint(uint(opToUsd)) / uint(ethToUsd);
        } else {
            revert IllegalState("Invalid debt token");
        }

        return expectedExchange;
    }

Tool used

Manual Review

Recommendation

Use sequencer oracle to determine whether the sequencer is offline or not, and don't allow orders to be executed while the sequencer is offline. The Chainlink documentation contains an example for how to check the sequencer status.

Duplicate of #14