EigenLayer Contest - ToonVH'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

If a Strategy is hacked/selfdestructs such that it reverts when withdraw() is called, then all currently queued withdrawals in StrategyManager containing that strategy are frozen and the funds from non-hacked strategies in that withdrawal are lost. This is because a queuedWithdrawal has to be withdrawn as a whole or not at all.

for (uint256 i = 0; i < strategiesLength;) {
    if (queuedWithdrawal.strategies[i] == beaconChainETHStrategy) {

        // if the strategy is the beaconchaineth strat, then withdraw through the EigenPod flow
        _withdrawBeaconChainETH(queuedWithdrawal.depositor, msg.sender, queuedWithdrawal.shares[i]);
    } else {
        // tell the strategy to send the appropriate amount of funds to the depositor
        queuedWithdrawal.strategies[i].withdraw(
            msg.sender, tokens[i], queuedWithdrawal.shares[i]
        );
    }
    unchecked {
        ++i;
    }
}

Proof of Concept

1. Bob has 100 tokens each in strategies A, B, C and D.
2. Bob queues a withdrawal for all his tokens for these 4 strategies.
3. Strategy B suffers some exploit and now reverts when withdraw() is called.
4. When Bob calls completeQueuedWithdrawal() it reverts since strategyB.withdraw() reverts.

Bob now not only lost his tokens in strategy B (expected), but also those for strategy A, C and D (unexpected).

Tools Used

Manual review

Recommended Mitigation Steps

• Allow skipping strategies when completing a withdrawal. Similar to how indicesToSkip functions in slashQueuedWithdrawal()

Assessed type

Loop