Stader Labs - whimints's results

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/7566b5a35f32ebd55d3578b8bd05c038feb7d9cc/contracts/StaderOracle.sol#L645-L651

Vulnerability details

Impact

The StaderOracle contract uses the Chainlink AggregatorV3 interface to fetch the total ETH balance as well as the total ETHx balance through the latestRoundData function. However, this function may return stale data if Chainlink node consensus is blocked or any other system failure (or manipulation) occurs.

To make these cases detectable, latestRoundData returns multiple values that need to be validated:

• uint80 roundId
• int256 answer
• uint256 startedAt
• uint256 updatedAt
• uint80 answeredInRound

None of this data is validated and only the data is returned, posing a high risk for stale or invalid oracle data entering the system.

Proof of Concept

The lack of validation is present here:

https://github.com/code-423n4/2023-06-stader/blob/7566b5a35f32ebd55d3578b8bd05c038feb7d9cc/contracts/StaderOracle.sol#L645-L651

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

Tools Used

None.

Recommended Mitigation Steps

If the underlying Chainlink aggregator data is stale, the round ID will stay the same. Furthermore, the startedAt timestamp should remain the same as in the previously fetched round. answeredInRound is the round ID where the answer was computed, so it will stay the same as the last round's information as well.

Please refer to the Chainlink documentation regarding the specific properties and how to best leverage them to avoid stale data.

Assessed type

Oracle

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/7566b5a35f32ebd55d3578b8bd05c038feb7d9cc/contracts/StaderOracle.sol#L650

Vulnerability details

Impact

The POR feed is used to update the exchange rate data through the StaderOracle contract. For the system to calculate the exchange rate correctly, it is paramount that the oracle-provided data is recent. However, the getPORFeedData function always returns block.number as its reportingBlockNumber.

This is insecure as the Chainlink aggregator may return stale data, which would be presented by this function as recent and valid.

Proof of Concept

The line always returns block.number:

https://github.com/code-423n4/2023-06-stader/blob/7566b5a35f32ebd55d3578b8bd05c038feb7d9cc/contracts/StaderOracle.sol#L650

The function consuming the block number and updating the price data based on it:

https://github.com/code-423n4/2023-06-stader/blob/7566b5a35f32ebd55d3578b8bd05c038feb7d9cc/contracts/StaderOracle.sol#L160-L166

Tools Used

None.

Recommended Mitigation Steps

I recommend either refactoring the price data system to work with time stamps and propagating the Chainlink Aggregator-provided time stamps, or adding general validation mechanisms for the Aggregator-integration that prevents stale data from bubbling up into the system, e.g. by reverting if a stale or invalid update would be executed.

Assessed type

Oracle