EigenLayer Contest - rvierdiiev's results

Lines of code

https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/core/StrategyManager.sol#L780-L794

Vulnerability details

Impact

User will not be able to claim his withdrawal and will lose funds.

Proof of Concept

When user wants to withdraw from strategies, then he can call queueWithdrawal function. User needs to provide array of strategies from where he wants to withdraw and also array of share amounts that he wants to withdraw.

For each strategy shares will be removed for user. https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/core/StrategyManager.sol#L358-L382

        for (uint256 i = 0; i < strategies.length;) {
            if (strategies[i] == beaconChainETHStrategy) {
                require(withdrawer == msg.sender,
                    "StrategyManager.queueWithdrawal: cannot queue a withdrawal of Beacon Chain ETH to a different address");
                require(strategies.length == 1,
                    "StrategyManager.queueWithdrawal: cannot queue a withdrawal including Beacon Chain ETH and other tokens");
                require(shares[i] % GWEI_TO_WEI == 0,
                    "StrategyManager.queueWithdrawal: cannot queue a withdrawal of Beacon Chain ETH for an non-whole amount of gwei");
            }   


            // the internal function will return 'true' in the event the strategy was
            // removed from the depositor's array of strategies -- i.e. stakerStrategyList[depositor]
            if (_removeShares(msg.sender, strategyIndexes[strategyIndexIndex], strategies[i], shares[i])) {
                unchecked {
                    ++strategyIndexIndex;
                }
            }


            emit ShareWithdrawalQueued(msg.sender, nonce, strategies[i], shares[i]);


            //increment the loop
            unchecked {
                ++i;
            }
        }

After that queuedWithdrawal is created for user that contains information about strategies and shares amount. After some amount of blocks user can call completeQueuedWithdrawal in order to claim his withdrawal and receieve funds.

completeQueuedWithdrawal will call _completeQueuedWithdrawal function. This function will go through all strategies of queued withdrawal and will withdraw needed amount of shares.

The problem here is next. Each strategy is a separate contract that can have integration with different defi protocols in order to earn rewards. So it's possible that any of those protocols or strategy will be paused/hacked which will make queuedWithdrawal.strategies[i].withdraw to revert and because of that, it will be not possible to withdraw any funds from not affected strategies that are in the queuedWithdrawal.strategies array. Such queuedWithdrawal then will be stuck until affected strategy will recover(if this will happen).

And pls, note that it will be not possible to mitigate this using slashQueuedWithdrawal function as it can be called only for withdrawals with frozen delegator.

Tools Used

VsCode

Recommended Mitigation Steps

Maybe it will be better to allow only withdrawals with 1 strategy per withdrawal.