Stader Labs - rvierdiiev's results

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/ValidatorWithdrawalVault.sol#L29-L83

Vulnerability details

Impact

ValidatorWithdrawalVault.distributeRewards can be called to make operator slashable. Attacker can call distributeRewards right before settleFunds to make operatorShare < penaltyAmount. As result validator will face loses.

Proof of Concept

ValidatorWithdrawalVault.distributeRewards can be called by anyone. It's purpose is to distribute validators rewards among stakers protocol and operator. After the call, balance of ValidatorWithdrawalVault becomes 0.

ValidatorWithdrawalVault.settle is called when validator is withdrawn from beacon chain. In this case balance of contract is used to find operatorShare and in case if it's less than accrued penalty by validator, then operator is slashed.

Because distributeRewards is permissionless, then next situation is possible. 1.Operator decided to withdraw validator. At the moment of that call, balance of ValidatorWithdrawalVault is not 0 and operatorShare is 1 eth. Also validator accrued 4.5 eth of penalty. 2.Malicious user sees when 32 eth of validator's deposit is sent to the ValidatorWithdrawalVault and frontruns it with distributeRewards call. This makes balance to be 32 eth. 3.operatorShare will be 4 eth in this time(permisssionless) and penalty is 4.5, so user is slashed. 4.In case if malicious user didn't call distributeRewards, then slash would not occur.

Also in same way, permissioned operator can call distributeRewards to get his rewards, when he is going to be slashed. As permissioned validators are not forced to have collateral to be slashed. By this move he rescued his eraning as otherwise they would be sent to pool.

Tools Used

VsCode

Recommended Mitigation Steps

Maybe think about restrict access to distributeRewards function.

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/ValidatorWithdrawalVault.sol#L53-L83

Vulnerability details

Impact

ValidatorWithdrawalVault.settleFunds doesn't check amount that user has inside NodeELRewardVault to pay for penalty. That value can increase operator's earned amount which can avoid slashing for him.

Proof of Concept

When validator withdraws from beacon chain then ValidatorWithdrawalVault.settleFunds function is called. This function calculates amount that validator has earned for attestations as validator. So only balance of this contract is considered.

Then function fetches penalty amount. This penalty amount contains of 3 points: _mevTheftPenalty, _missedAttestationPenalty and _missedAttestationPenalty.

In case if penalty amount is bigger than validator's earning on ValidatorWithdrawalVault, then SD collateral is slashed.

Now we need to understand how validator receives funds in this system. All attestation payments comes to ValidatorWithdrawalVault, while mev/block proposal funds are coming to SocializingPool or NodeELRewardVault(depends on user's choice). So actually _missedAttestationPenalty is responding to ValidatorWithdrawalVault earning, while _mevTheftPenalty is responding to NodeELRewardVault earnings.

That means that NodeELRewardVault balance should also be checked in order to find out how mane earning validator has there and they should be also counted when applying penalty.

Simple example. 1.Validator wants to exit. 2.Operator earning is 0.1 eth inside ValidatorWithdrawalVault. 3.Accrued penalty is 0.11, which means that user will be slashed. 4.Operator also has NodeELRewardVault where his operator's reward is 0.05 eth 5.As result user had enough balance to cover penalty, but he still was penalizied.

Tools Used

VsCode

Recommended Mitigation Steps

As you accrued _mevTheftPenalty inside ValidatorWithdrawalVault, then you also should calculate operator's rewards inside NodeELRewardVault.

Assessed type

Error

Lines of code

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

Vulnerability details

Impact

StaderOracle.getPORFeedData doesn't check prices are up to date. As result prices can be invalid, which will affect system.

Proof of Concept

StaderOracle.getPORFeedData fetches prices from chainlink price feeds. https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderOracle.sol#L637-L651

    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, it doesn't do any checks after that. The problem is that oracle price can be stale in case if price feed wasn't updated for some time. As result protocol will use this stale prices.

Tools Used

VsCode

Recommended Mitigation Steps

Ensure that price is not 0 and that it was provided not long time ago.

Assessed type

Oracle