code-423n4 / 2024-04-renzo-validation

2 stars 2 forks source link

Stakers can avoid slashing and penalties while making next claimers take the loss #504

Closed c4-bot-10 closed 6 months ago

c4-bot-10 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L274-L358 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L338-L345

Vulnerability details

Impact

Depositors in the Renzo system can avoid penalties and slashing and make the next withdrawers take the loss by requesting a withdrawal and then updating the effective balance of the validator.

Proof of Concept

Validators can lose part of their deposit via penalties or slashing events.

It is important to note that Renzo as a staker itself is not incentivized to call EigenPod::verifyBalanceUpdates, since there is a chance the podOwnerShares that are included in the calculations of calculateTVL, to be decreased, which will decrease the price of ezETH.

But now users, without any risk for themselves, can make the next withdrawers take the loss of slashing and penalties by first initiating withdrawal from the WithdrawQueue::withdraw and then calling EigenPod::verifyBalanceUpdates on behalf of the penalized OperatorDelegators or simply frontrun another user’s transactions which will apply the slashing penalty.

The issue is that nowhere when a withdrawal request is initiated there is a check or update on whether one or more of the validators have their podOwnerShares decreased which will be the case when the Delegator validator (EigenPod) has been slashed.

Here is the function that calculates the TVL in the Renzo protocol:

RestakeManager.sol#L274-L358

function calculateTVLs() public view returns (uint256[][] memory, uint256[] memory, uint256) {
    uint256[][] memory operatorDelegatorTokenTVLs = new uint256[][](operatorDelegators.length);
    uint256[] memory operatorDelegatorTVLs = new uint256[](operatorDelegators.length);
    uint256 totalTVL = 0;

    // Iterate through the ODs
    uint256 odLength = operatorDelegators.length;

    for (uint256 i = 0; i < odLength; ) {
...MORE CODE
        // Track the TVL for this OD
        uint256 operatorTVL = 0;

        // Get the value of native ETH staked for the OD
        uint256 operatorEthBalance = operatorDelegators[i].getStakedETHBalance();//@audit calculates effective balance of validator

        // Save it to the array for the OD
        operatorValues[operatorValues.length - 1] = operatorEthBalance;

        // Add it to the total TVL for this OD
        operatorTVL += operatorEthBalance;

        // Add it to the total TVL for the protocol
        totalTVL += operatorTVL;

        // Save the TVL for this OD
        operatorDelegatorTVLs[i] = operatorTVL;
...MORE CODE
}

And the function that is responsible for getting the manipulatable values is OperatorDelegator::getStakedETHBalance:

OperatorDelegator.sol#L338-L345

function getStakedETHBalance() external view returns (uint256) {
    // accounts for current podOwner shares + stakedButNotVerified ETH + queued withdraw shares
    int256 podOwnerShares = eigenPodManager.podOwnerShares(address(this));
    return
        podOwnerShares < 0
            ? queuedShares[IS_NATIVE] + stakedButNotVerifiedEth - uint256(-podOwnerShares)
            : queuedShares[IS_NATIVE] + stakedButNotVerifiedEth + uint256(podOwnerShares);
}

As well as the function for calculating the exchange ratio of ezETH:

RenzoOracle.sol#L152-L164

function calculateRedeemAmount(
    uint256 _ezETHBeingBurned, //@audit ezETH amt to withdraw
    uint256 _existingEzETHSupply, //@audit ezETH.totalSupply
    uint256 _currentValueInProtocol //@audit totalTVL (decreased when validator is slashed)
) external pure returns (uint256) {
    // This is just returning the percentage of TVL that matches the percentage of ezETH being burned
    uint256 redeemAmount = (_currentValueInProtocol * _ezETHBeingBurned) / _existingEzETHSupply;

    // Sanity check
    if (redeemAmount == 0) revert InvalidTokenAmount();

    return redeemAmount;
}

As we can see when the _currentValueInProtocol (totalTVL) decreases, the redeemable shares decrease as well.

Tools Used

Manual Review

Recommended Mitigation Steps

Add a check whether one of the validators is slashed and apply the price reduction to all the users equally do not rely on random call to verifyBalanceUpdates.

Assessed type

Context

DadeKuma commented 6 months ago

@howlbot accept