Stader Labs - tallo's results

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/SocializingPool.sol#L112 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/Auction.sol#L48 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/OperatorRewardsCollector.sol#L46 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderOracle.sol#L113

Vulnerability details

The PauseableUpgradeable contract doesn't implement any external/public methods to pause the contract so it is up to the inheriting contract to implement this feature. Several of the contracts do not implement this feature.

Impact

Certain functions won't be pauseable even though they are intended to be

Proof of Concept

For instance, the following function has the whenNotPaused modifier however there is no public/external function inside StaderOracle to actually call the internal '''_pause''' function.

    function submitExchangeRateData(ExchangeRate calldata _exchangeRate)
        external
        override
        trustedNodeOnly
        checkMinTrustedNodes
        checkERInspectionMode
        whenNotPaused
    {//..}

Tools Used

VIM

Recommended Mitigation Steps

Inside SocializingPool.sol, Auction.sol, OperatorRewardsCollector.sol, StaderOracle.sol, insert an external method to pause the contract as done correctly in the other contracts such as ETHx

contract ETHx is Initializable, ERC20Upgradeable, PausableUpgradeable, AccessControlUpgradeable {
    function pause() external {
        UtilLib.onlyManagerRole(msg.sender, staderConfig);
        _pause();
    }
}

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

Users can be served outdated pricing data from functions that call getPORFeedData. This can happen in times of high congestion where chainlink goes down.

Proof of Concept

The chainlink latestRoundData function contains an updatedAt value that represents the timestamp of the last update. This value should always be checked to ensure its not stale.

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

        //@audit here the value should be checked before its returned
        return (uint256(totalETHBalanceInInt), uint256(totalETHXSupplyInInt), block.number);
    }

Tools Used

Vim

Recommended Mitigation Steps

Check for round completion and that the updatedeAt value is not more than desired.

Assessed type

Oracle