Stader Labs - Breeje's results

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/HEAD/contracts/StaderOracle.sol#L637-L651

Vulnerability details

Impact

The retrieved price from the oracle can be stale value or outdated and used anyways as a valid data. The usage of such data can impact on how the further logics of that price are implemented.

Proof of Concept

File: StaderOracle.sol

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

Link to code

Neither round completeness or the quoted timestamp are checked to ensure that the reported price is not stale.

function latestRoundData() external view
    returns (
        uint80 roundId,
        int256 answer, 
        uint256 startedAt,
        uint256 updatedAt,
        uint80 answeredInRound
    )

That's the reason Chainlink recommends using their data feeds along with some controls to prevent mismatches with the retrieved data.

Chainlink oracles updates the value when one of the 2 Trigger happens:

1. If Deviation Threshold exceeds. Deviation Threshold is the specific amount of deviation in prices which happens after which a new aggregation round will start and the price will be updated.
2. If Heartbeat Threshold exceeds. Heartbeat Threshold is a specific amount of time from the last update after which a new aggregation round will start and the price will be updated.

Link: https://docs.chain.link/architecture-overview/architecture-decentralized-model/

But in getPORFeedData, Heartbeat Threshold is not implemented to check whether the last observed price was stale or not.

Chainlink in there docs have clearly written: "Your application should track the latestTimestamp variable or use the updatedAt value from the latestRoundData() function to make sure that the latest answer is recent enough for your application to use it. If your application detects that the reported answer is not updated within the heartbeat or within time limits that you determine are acceptable for your application, pause operation or switch to an alternate operation mode while identifying the cause of the delay."

So by adding the require check as recommended, it can stop the protocol from using the stale price which is even recommended by chainlink.

Link: https://docs.chain.link/data-feeds/#check-the-timestamp-of-the-latest-answer

Tools Used

VS Code

Recommended Mitigation Steps

As Chainlink recommends:

Your application should track the latestTimestamp variable or use the updatedAt value from the latestRoundData() function to make sure that the latest answer is recent enough for your application to use it. If your application detects that the reported answer is not updated within the heartbeat or within time limits that you determine are acceptable for your application, pause operation or switch to an alternate operation mode while identifying the cause of the delay.

Recommended Mitigation:

1. Add the Following 2 more require Statements in the getPORFeedData() method to mitigate the issue after setting the HEARTBEAT_PERIOD as set by chainlink.

File: PriceOracle.sol


    function getPORFeedData()
          internal
          view
          returns (
              uint256,
              uint256,
              uint256
          )
      {
-       (, int256 totalETHBalanceInInt, , , ) = AggregatorV3Interface(staderConfig.getETHBalancePORFeedProxy())
+       (uint80 roundID, int256 totalETHBalanceInInt, , uint256 updatedAt, uint80 answeredInRound) = AggregatorV3Interface(staderConfig.getETHBalancePORFeedProxy())
              .latestRoundData();
+       require(totalETHBalanceInInt > 0, "invalid price");
+       require(block.timestamp - updatedAt < HEARTBEAT_PERIOD, "Chainlink: Stale Price");
+       require(answeredInRound >= roundID, "Chainlink: Stale Price");
-         (, int256 totalETHXSupplyInInt, , , ) = AggregatorV3Interface(staderConfig.getETHXSupplyPORFeedProxy())
+       (uint80 roundID, int256 totalETHXSupplyInInt, , uint256 updatedAt, uint80 answeredInRound) = AggregatorV3Interface(staderConfig.getETHXSupplyPORFeedProxy())
              .latestRoundData();
+       require(totalETHXSupplyInInt > 0, "invalid price");
+       require(block.timestamp - updatedAt < HEARTBEAT_PERIOD, "Chainlink: Stale Price");
+       require(answeredInRound >= roundID, "Chainlink: Stale Price");
        return (uint256(totalETHBalanceInInt), uint256(totalETHXSupplyInInt), block.number);
      }
     

Assessed type

Oracle