code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-eigenlayer/blob/5e4872358cd2bda1936c29f460ece2308af4def6/src/contracts/core/StrategyManager.sol#L536-L579

Vulnerability details

Temporary blocking withdrawals because of slashQueuedWithdrawal function incorrectness

Impact

The incorrectness of the slashQueuedWithdrawalcan block withdraw operations till queuedWithdrawal argument will be changed to exclude strategies with PAUSED_WITHDRAWALS parameter.

Proof of Concept

There is an indicesToSkip optional argument for bypassing strategies, which can't be withdrawn. indicesToSkip contents indices of such strategiess. The loop in the L560 lets you sort strategies, but increment ++i is placed in the wrong line. If the logic branch goes into skipping strategie, the i variable stays the same. So the next iteration will be with the same strategie index.

         for (uint256 i = 0; i < strategiesLength;) {
             // check if the index i matches one of the indices specified in the `indicesToSkip` array
             if (indicesToSkipIndex < indicesToSkip.length && indicesToSkip[indicesToSkipIndex] == i) {
                 unchecked {
                     ++indicesToSkipIndex;
                 }
             } else {
                 if (queuedWithdrawal.strategies[i] == beaconChainETHStrategy){
                      //withdraw the beaconChainETH to the recipient
                     _withdrawBeaconChainETH(queuedWithdrawal.depositor, recipient, queuedWithdrawal.shares[i]);
                 } else {
                     // tell the strategy to send the appropriate amount of funds to the recipient
                     queuedWithdrawal.strategies[i].withdraw(recipient, tokens[i], queuedWithdrawal.shares[i]);
                 }
                 unchecked {
                     ++i;
                 }
             }
         }

https://github.com/code-423n4/2023-04-eigenlayer/blob/5e4872358cd2bda1936c29f460ece2308af4def6/src/contracts/core/StrategyManager.sol#L575

Tools Used

manual review

Recommended Mitigation Steps

Put the increment in the bottom of the loop.

Assessed type

Loop

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #205

c4-judge commented 1 year ago

GalloDaSballo marked the issue as satisfactory