The function payRewards in WagyuStaker.sol is used to send rewards
back to MevEth. It uses the storage variable balance to calculate the
rewards from the balance on line 91.
However, this calculation is incorrect, because it seems to assume that
address(this).balance would keep the total validator’s balance plus the
rewards, such that by subtracting the balance (total deposited validator’s
balance) would yield the rewards.
But by depositing ETH into the DepositContract and to create validators in
deposit(), address(this).balance gets decreased. As such, the calculation
on line 91 will revert almost always (unless rewards become greater than
the total validator’s balance, which will take years).
In the other case, where payRewards is a method to skim rewards from an
exited validator, then this function would only work if all validators exit at
once and all balance would be withdrawn to WagyuStaker. If less than all
validators exit and this function is called, then address(this).balance will
always be smaller than balance because balance is never decreased.
Apart from blocking the payRewards() function, validator and balance
variables do not seem to serve any purpose, are not decreased upon a
validator exit and the function oracleUpdate (that sets both these
variables) has access control for MevEth:
if (msg.sender != MEV_ETH) {
revert UnAuthorizedCaller();
}
However, the function is not called anywhere in MevEth.sol.
If the protocol has no clear use for them, they could be removed, however it
seems they haven't been fully implemented.
REMEDIATION
correctly decrease validator and balance upon a
validator exit and to remove oracleUpdate or use it in MevEth
WAGYUSTAKER REWARDS WILL BECOME BLOCKED
SEVERITY: High
PATH: WagyuStaker.sol:payRewards, oracleUpdate (L89-103,L65-72)
https://github.com/manifoldfinance/mevETH2/blob/63edde66d91c263b919fe9c21e128a382219880e/src/WagyuStaker.sol#L77-L103
DESCRIPTION
The function payRewards in WagyuStaker.sol is used to send rewards back to MevEth. It uses the storage variable balance to calculate the rewards from the balance on line 91. However, this calculation is incorrect, because it seems to assume that address(this).balance would keep the total validator’s balance plus the rewards, such that by subtracting the balance (total deposited validator’s balance) would yield the rewards. But by depositing ETH into the DepositContract and to create validators in deposit(), address(this).balance gets decreased. As such, the calculation on line 91 will revert almost always (unless rewards become greater than the total validator’s balance, which will take years).
In the other case, where payRewards is a method to skim rewards from an exited validator, then this function would only work if all validators exit at once and all balance would be withdrawn to WagyuStaker. If less than all validators exit and this function is called, then address(this).balance will always be smaller than balance because balance is never decreased. Apart from blocking the payRewards() function, validator and balance variables do not seem to serve any purpose, are not decreased upon a validator exit and the function oracleUpdate (that sets both these variables) has access control for MevEth:
However, the function is not called anywhere in MevEth.sol. If the protocol has no clear use for them, they could be removed, however it seems they haven't been fully implemented.
REMEDIATION
correctly decrease validator and balance upon a validator exit and to remove oracleUpdate or use it in MevEth