code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

The function getPORFeedData() inside the StaderOracle does make a call to the Chainlink latestRoundData() function but doesn't contain checks on the returned price (if its equal to zero or not), round completeness and update timestamp, which could lead to returning a stale or wrong price and thus the protocol functions that rely on accurate price feed might not work as expected and sometimes can lead to a loss of funds.

Proof of Concept

The issue occurs in the getPORFeedData() function below :

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

As you can see the function getPORFeedData() calls the function latestRoundData() to get the prices of the (ETH, ETHX) tokens using chainlink price feeds, but the function does not implement the required checks for verifying that the returned prices from the latestRoundData() call are not equal to zero and there is no check on the round timestamp and completeness to avoid outdated results, this can lead to stale prices according to Chainlink's documentation.

As the protocol uses the oracle prices to update the exchange rate with the updateERFromPORFeed(), it must receive accurate price feed and in the case when an oracle return a stale/wrong price this will have a negative impact as the new exchange rate will be wrong and can potentialy lead to a to a loss of funds.

Tools Used

Manual review

Recommended Mitigation Steps

Add the required checks whenever the latestRoundData() function is called in the aforementioned instances, the function getPORFeedData() should be updated as follows :

function getPORFeedData() internal view returns (uint256, uint256, uint256) {
    (uint80 roundID, int256 totalETHBalanceInInt, uint256 timestamp, uint256 updatedAt, ) = AggregatorV3Interface(staderConfig.getETHBalancePORFeedProxy())
        .latestRoundData();
    // @audit Add the required checks
    require(totalETHBalanceInInt > 0,"Chainlink answer reporting 0");
    require(updatedAt >= roundID, "Stale price");
    require(timestamp != 0,"Round not complete");
    require(
        block.timestamp - updatedAt <= 3600,
        "ORACLE_HEARTBEAT_FAILED"
    );

    (roundID, int256 totalETHXSupplyInInt, timestamp, updatedAt, ) = AggregatorV3Interface(staderConfig.getETHXSupplyPORFeedProxy())
        .latestRoundData();
    // @audit Add the required checks
    require(totalETHXSupplyInInt > 0,"Chainlink answer reporting 0");
    require(updatedAt >= roundID, "Stale price");
    require(timestamp != 0,"Round not complete");
    require(
        block.timestamp - updatedAt <= 3600,
        "ORACLE_HEARTBEAT_FAILED"
    );

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

Assessed type

Oracle

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #15