StrategyManager.slashQueuedWithdrawal might not work as expected

[code423n4]

Lines of code

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

Vulnerability details

Impact

Function StrategyManager.slashQueuedWithdrawal's param indicesToSkip is an array, according to the source code, the function can work only if the data stored in indicesToSkip is ascending order.
For example, if there are 15 strategies in queuedWithdrawal.strategies, and the StrategyManager needs to slash strategie 1, 3, 5, to make the slash success, he has to fill indicesToSkip with the indices in ascending order. as [1, 3, 5], otherwise he will fails

Proof of Concept

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

function slashQueuedWithdrawal(address recipient, QueuedWithdrawal calldata queuedWithdrawal, IERC20[] calldata tokens, uint256[] calldata indicesToSkip)
    external
    onlyOwner
    onlyFrozen(queuedWithdrawal.delegatedAddress)
    nonReentrant
{
....
    // keeps track of the index in the `indicesToSkip` array
    uint256 indicesToSkipIndex = 0;

    uint256 strategiesLength = queuedWithdrawal.strategies.length;
    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;
            }
        }
    }
}

Tools Used

VS

Recommended Mitigation Steps

Assessed type

Error

[c4-pre-sort]

0xSorryNotSorry marked the issue as primary issue

[c4-sponsor]

Sidu28 marked the issue as sponsor disputed

[Sidu28]

This is how the system is intended to function

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)

[GalloDaSballo]

Dowgraded to QA - R for documentation

[c4-judge]

GalloDaSballo marked the issue as grade-b