search

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

2 stars 2 forks source link

Ineffective withdrawal bonding period protection #915

Closed c4-bot-3 closed 5 months ago

c4-bot-3 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Withdraw/WithdrawQueue.sol#L279-L313 https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Withdraw/WithdrawQueue.sol#L206-L263

Vulnerability details

Summary

the protocol implemented a withdrawal bonding period to protect against exploits by giving them time to pause and protect against frontrunning scenarios where users might exploit forthcoming decreases in the ezETH exchange rate, such as those caused by validator slashing. but the current implementation doesn't protect against neither of them.

Vulnerability Detail

In the current setup, the WithdrawQueue:withdraw function is used to initiate a withdrawal request, withdrawing ETH or another LST for ezETH. calculating the redeemable amount based on the current exchange rates between ezETH/ETH and ETH/outputToken:

uint256 amountToRedeem = renzoOracle.calculateRedeemAmount(
            _amount,
            ezETH.totalSupply(),
            totalTVL
        );
=> amountToRedeem = renzoOracle.lookupTokenAmountFromValue(
                IERC20(_assetOut),
                amountToRedeem
            );

This amount is locked in at the time of the request and does not adapt to subsequent changes in token value, in the bonding period

withdrawRequests[msg.sender].push(
        WithdrawRequest(
            _assetOut,
            withdrawRequestNonce,
=>            amountToRedeem,
            _amount,
            block.timestamp
        )
    );
ezETH.burn(address(this), _withdrawRequest.ezETHLocked);

This setup allows a user to preemptively lock in the exchange rate before events like validator slashing, which leads to a decrease in the ezETH value, forcing other restakers to absorb more significant losses. Moreover, once a withdrawal request is made, the protocol team has no mechanism to intervene or reverse the transaction, even in the event of an attack. There is no function to remove funds from the withdrawal queue.

Additionally, the existing system setup dedicates funds for claims as soon as a withdrawal request is made:

function getAvailableToWithdraw(address _asset) public view returns (uint256) {
        if (_asset != IS_NATIVE) {
            return IERC20(_asset).balanceOf(address(this)) - claimReserve[_asset];
        } else {
            return address(this).balance - claimReserve[_asset];
        }
    }

Pausing the ezETH token does not address these issues because the pause function allows for minting and burning, but not for transfers:

function _beforeTokenTransfer(
        address from,
        address to,
        uint256 amount
    ) internal virtual override {
        super._beforeTokenTransfer(from, to, amount);

        // If not paused return success
        if (!paused) {
            return;
        }

        // If paused, only minting and burning should be allowed
=>        if (from != address(0) && to != address(0)) {
            revert ContractPaused();
        }
    }

and in the claim function we only burn ezETH:

ezETH.burn(address(this), _withdrawRequest.ezETHLocked);

As a result, even in the face of protocol exploits, attackers can still claim funds once the bonding period is over, undermining the protocol's security measures.

Proof Of Concept

a coded POC is hard since the protcol has no test suite , so an attack path should suffice:

Impact

The withdrawal bonding mechanism is ineffective. Malicious restakers can exploit this system flaw, causing other restakers to incur greater losses, especially in events like validator slashing or any other incidents that result in a decrease in the ezETH exchange rate. and it doesn't enable the protocol team to respond to attacks.

Tool used

Manual Review

Recommendation

Instead of fixing the exchange rate at the time of the withdrawal request, it should be determined at the moment of the actual fund claim:

  1. Modify the withdrawal request to only record the amount of ezETH to burn and the output token type:

    withdrawRequests[msg.sender].push(
        WithdrawRequest(
            _assetOut,
            withdrawRequestNonce,
            _amount,
            block.timestamp
        )
    );

  2. Recalculate the redeemable amount based on the current exchange rates at the time of claiming:

    uint256 amountToRedeem = renzoOracle.calculateRedeemAmount(
_amount,
ezETH.totalSupply(),
totalTVL
);

    // update amount in claim asset, if claim asset is not ETH
if (_assetOut != IS_NATIVE) {
// Get ERC20 asset equivalent amount
amountToRedeem = renzoOracle.lookupTokenAmountFromValue(
    IERC20(_assetOut),
    amountToRedeem
);
}

However, this would introduce a risk of slippage; users may receive a different amount than expected due to fluctuations in exchange rates between the request and claim times.

and the protocol should add mechanism to pause the withdrawal queue if there intention is to stop exploiters from withdrawing funds.

Assessed type

MEV

DadeKuma commented 5 months ago

@howlbot accept