code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

In StrategyManager a user can queue up withdrawal and wait for a minimum delay lag in blocktime in other to complete the withdrawal. However QueuedWithdrawal struct which is initialized in queuedWithdrawal function is stored in memory which is not persistent after the transaction, and it is not stored anywhere within the contract.

QueuedWithdrawal memory queuedWithdrawal;//@audit cached in memory
...
queuedWithdrawal = QueuedWithdrawal({
                strategies: strategies,
                shares: shares,
                depositor: msg.sender,
                withdrawerAndNonce: withdrawerAndNonce,
                withdrawalStartBlock: uint32(block.number),
                delegatedAddress: delegatedAddress
            });

...
emit WithdrawalQueued(msg.sender, nonce, withdrawer, delegatedAddress, withdrawalRoot);//@audit withdrawalRoot is emitted instead

Meanwhile this struct is used to recompute the withdrawalRoot hash in slashQueueWithdrawal function, which means that a slasher cannot retrieve the QueueWithdrawal struct provided by the user. In a scenerio where a malicious user who have queued up for a withdraw is to be slashed the queueWithdrawal struct cannot be retrieved which means that a queued withdrawal cannot be slashed.

 //@audit-issue QueuedWithdrawal is the second parameter
 function slashQueuedWithdrawal(address recipient, QueuedWithdrawal calldata queuedWithdrawal, IERC20[] calldata tokens, uint256[] calldata indicesToSkip)
        external
        onlyOwner
        onlyFrozen(queuedWithdrawal.delegatedAddress)
        nonReentrant
    {
        require(tokens.length == queuedWithdrawal.strategies.length, "StrategyManager.slashQueuedWithdrawal: input length mismatch");

        // find the withdrawalRoot
        //@audit use to recompute the withdrawalRoot hash
        bytes32 withdrawalRoot = calculateWithdrawalRoot(queuedWithdrawal);

Proof of Concept

1) Alice stakes on Eigenlayer. 2) Alice behaved maliciously and immediately queueWithdrawal. 3) The owner/slasher attempts to slash alice queued withdrawal but cannot find the QueuedWithdrawal struct to process the slashing. 4) After the delay period alice completes her withdrawal if the entire system is not paused.