Slashing can be frontrunned

[code423n4]

Lines of code

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

Vulnerability details

Proof of Concept

When attempting to withdraw funds, the user calls queueWithdrawal first. queueWithdrawal() checks that the caller is not frozen, then marks the withdrawal as pending.

    function queueWithdrawal(
        uint256[] calldata strategyIndexes,
        IStrategy[] calldata strategies,
        uint256[] calldata shares,
        address withdrawer,
        bool undelegateIfPossible
    )
        external
        onlyWhenNotPaused(PAUSED_WITHDRAWALS)
        onlyNotFrozen(msg.sender)
.
.
.
        // mark withdrawal as pending
        withdrawalRootPending[withdrawalRoot] = true;

Next, the user calls completeQueuedWithdrawal(), which checks whether the caller is frozen again, and makes sure that withdrawal is pending.

    function _completeQueuedWithdrawal(QueuedWithdrawal calldata queuedWithdrawal, IERC20[] calldata tokens, uint256 middlewareTimesIndex, bool receiveAsTokens) onlyNotFrozen(queuedWithdrawal.delegatedAddress) internal {
        // find the withdrawalRoot
        bytes32 withdrawalRoot = calculateWithdrawalRoot(queuedWithdrawal);

        // verify that the queued withdrawal is pending
        require(
            withdrawalRootPending[withdrawalRoot],
            "StrategyManager.completeQueuedWithdrawal: withdrawal is not pending"
        );

Then, it checks that either queuedWithdrawal.withdrawalStartBlock + withdrawalDelayBlocks <= block.number or queuedWithdrawal.strategies[0] == beaconChainETHStrategy before initiating any withdraws.

        // enforce minimum delay lag (not applied to withdrawals of 'beaconChainETH', since the EigenPods enforce their own delay)
        require(queuedWithdrawal.withdrawalStartBlock + withdrawalDelayBlocks <= block.number 
                || queuedWithdrawal.strategies[0] == beaconChainETHStrategy,
            "StrategyManager.completeQueuedWithdrawal: withdrawalDelayBlocks period has not yet passed"
        );
.
.
                    _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]
                    );

If the user knows that he is being slashed, he can frontrun the slashing by calling queuedWithdrawal and completeQueuedWithdrawal in the same function, and if the withdrawal strategy is beaconChain or there is no withdrawalDelayBlocks (owner can set withdrawalDelayBlocks to zero potentially), then the user can avoid any sort of slashing attempts.

    function setWithdrawalDelayBlocks(uint256 _withdrawalDelayBlocks) external onlyOwner {
        _setWithdrawalDelayBlocks(_withdrawalDelayBlocks);
    }

    function _setWithdrawalDelayBlocks(uint256 _withdrawalDelayBlocks) internal {
        require(_withdrawalDelayBlocks <= MAX_WITHDRAWAL_DELAY_BLOCKS, "StrategyManager.setWithdrawalDelay: _withdrawalDelayBlocks too high");
        emit WithdrawalDelayBlocksSet(withdrawalDelayBlocks, _withdrawalDelayBlocks);
        withdrawalDelayBlocks = _withdrawalDelayBlocks;
    }

Documentation also states that minimum delay can be zero, which allows for the frontrunning of slashing attempts.

Minimum delay enforced by this contract for completing queued withdrawals. Measured in blocks, and adjustable by this contract's owner,
up to a maximum of `MAX_WITHDRAWAL_DELAY_BLOCKS`. Minimum value is 0 (i.e. no delay enforced).

Impact

User can potentially escape slashing attempts.

Recommended Mitigation Steps

Recommend having a set minimum delay between queueing withdrawals and completing queue withdrawals.

Assessed type

Invalid Validation

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #294

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid