manifoldfinance / mevETH2

mevETH LST Protocol - Repo has migrated see link
https://github.com/MEV-Protocol/meveth
27 stars 2 forks source link

Audit: MANETH-42 #119

Closed 0xKitsune closed 1 year ago

0xKitsune commented 1 year ago

Status

Reported

Type

Vulnerability

Severity

High

Code Snippet:

function grantValidatorWithdraw() external payable {
    // Check that the sender is the staking module or the MevEthShareVault.
    if (!(msg.sender == address(stakingModule) || msg.sender == mevEthShareVault)) revert MevEthErrors.InvalidSender();

    // Check that the value is not zero
    if (msg.value == 0) {
        revert MevEthErrors.ZeroValue();
    }

    // Process the withdrawal queue, paying out any pending withdrawal tickets before updating the fraction balance.
    processWithdrawalQueue();

    // Emit an event to notify offchain listeners that a validator has withdrawn funds.
    emit ValidatorWithdraw(msg.sender, msg.value);

    // If the msg.value is 32 ether, the elastic should not be updated.
    if (msg.value == 32 ether) {
        return;
    }

    // If the msg.value is less than 32 ether, the elastic should be reduced.
    if (msg.value < 32 ether) {
        /// @dev Elastic will always be at least equal to base. Base will always be at least equal to the MIN_DEPOSIT amount.
        // assume slashed value so reduce elastic balance accordingly
        fraction.elastic -= uint128(32 ether - msg.value);
    } else {
        // If the msg.value is greater than 32 ether, the elastic should be increased.
        // account for any unclaimed rewards
        fraction.elastic += uint128(msg.value - 32 ether);
    }
}

Remediation

Description

MevEth.sol:grantValidatorWithdraw

Whenever a protocol validator gets slashed or penalised for going offline, it would mean that user funds got lost. This loss does not immediately get submitted to the MevEth contract.

Instead, whenever the validator withdraws, the operator calls WagyuStaker.sol:payValidatorWithdraw, which in turns call grantValidatorWithdraw with the ETH amount given by the operator. The function then checks the value against 32 ETH to determine if the validator was slashed or not.

Any slashed amount get subtracted from fraction.elastic, which means that all users are paying evenly (depending on stake) for the loss.

However, because this is the result of a slashed validator, as well as a transaction by the operator, it becomes possible for any staker to avoid paying for these losses and instead unfairly let other users pay a larger amount for the penalty.

The user can:

Watch protocol’s validator on the consensus layer, note when a slashed/penalised validator exits and unstake right before the validator’s exit withdrawal is processed.
Front-run the operator’s transaction to payValidatorWithdraw and unstake.
Afterwards, the user can stake again, gaining more shares due to the share rate going down.
sandybradley commented 1 year ago

Similar to: https://github.com/manifoldfinance/mevETH2/issues/118