code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderOracle.sol#L646 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderOracle.sol#L648

Vulnerability details

Impact

Chainlink's latestRoundData is used here to retrieve price feed data, however there is insufficient protection against price staleness.

Return arguments other than int256 answer are necessary to determine the validity of the returned price, as it is possible for an outdated price to be received. See here for reasons why a price feed might stop updating.

The return value updatedAt contains the timestamp at which the received price was last updated, and can be used to ensure that the price is not outdated. See more information about latestRoundID in the Chainlink docs. Inaccurate price data can lead to functions not working as expected and/or lost funds.

Proof of Concept

    function getPORFeedData()
        internal
        view
        returns (
            uint256,
            uint256,
            uint256
        )
    {
        (, int256 totalETHBalanceInInt, , , ) = AggregatorV3Interface(staderConfig.getETHBalancePORFeedProxy())
            .latestRoundData();
        (, int256 totalETHXSupplyInInt, , , ) = AggregatorV3Interface(staderConfig.getETHXSupplyPORFeedProxy())
            .latestRoundData();
        return (uint256(totalETHBalanceInInt), uint256(totalETHXSupplyInInt), block.number);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Add a check for the updatedAt returned value from latestRoundData.

    function getPORFeedData()
        internal
        view
        returns (
            uint256,
            uint256,
            uint256
        )
    {
-       (, int256 totalETHBalanceInInt, , , ) = AggregatorV3Interface(staderConfig.getETHBalancePORFeedProxy())
+       (, int256 totalETHBalanceInInt, , uint256 balanceUpdatedAt, ) = AggregatorV3Interface(staderConfig.getETHBalancePORFeedProxy())
            .latestRoundData();
+       require(block.timestamp - balanceUpdatedAt <= MAX_DELAY, "stale price");

-       (, int256 totalETHXSupplyInInt, , , ) = AggregatorV3Interface(staderConfig.getETHXSupplyPORFeedProxy())
+       (, int256 totalETHXSupplyInInt, , uint256 supplyUpdatedAt, ) = AggregatorV3Interface(staderConfig.getETHXSupplyPORFeedProxy())
            .latestRoundData();
+       require(block.timestamp - supplyUpdatedAt <= MAX_DELAY, "stale price");

        return (uint256(totalETHBalanceInInt), uint256(totalETHXSupplyInInt), block.number);
    }

Assessed type

Oracle

c4-judge commented 1 year ago

Picodes marked the issue as primary issue

c4-sponsor commented 1 year ago

manoj9april marked the issue as sponsor acknowledged

manoj9april commented 1 year ago

Solution with chainlink is not finalized.