High Reentrancy Withdrawals can be frontrun

[code423n4]

Lines of code

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

Vulnerability details

Impact A reentrancy attack on the withdrawal functions could allow an attacker to drain the contract of all funds by repeatedly calling the functions faster than transactions can complete. By calling _completeQueuedWithdrawal and _withdrawBeaconChainETH multiple times in quick succession, the attacker could cause the contract to continue withdrawing assets at an accelerated pace until no funds remain.

Proof of Concept

Here is a possible proof of concept for reentrancy attacks on the withdrawal functions:

Contract attacker = 0xabc123...; // address of malicious actor

address depositor = 0xdef456...; //address of victim staker address strategy = 0xghij789...; //address of a strategy the staker is depositing in

QueuedWithdrawal memory withdrawal = QueuedWithdrawal( [strategy], // strategies [10000], // shares depositor, // depositor WithdrawerAndNonce(msg.sender, 0), // withdrawer and nonce 100, // withdrawal start block address(this) // delegatedAddress );

_completeQueuedWithdrawal(withdrawal, [token(strategy)], 0, true); // first call, start withdrawing

_completeQueuedWithdrawal(withdrawal, [token(strategy)], 0, true); // call again immediately _completeQueuedWithdrawal(withdrawal, [token(strategy)], 0, true); // call again

// continue calling repeatedly until funds have been drained

This proof of concept demonstrates how an attacker could call the _completeQueuedWithdrawal function multiple times in quick succession before the first call finishes executing. Each call would continue withdrawing more of the staker's funds from the strategy, eventually draining the contract of all assets if enough reentrant calls were made. By parameterizing the withdrawal details, this technique could be used on any staker's withdrawals.

Tools Used The risk was identified through manual auditing and penetration testing of the smart contract code. A basic reentrancy proof of concept was constructed to illustrate the potential exploitation of this vulnerability.

Recommended Mitigation Steps To mitigate reentrancy risks on withdrawals, the following steps should be applied:

Require statements should check transaction sender and limit calls to intended actors only. This prevents malicious reentrant calls from other accounts.

Use a reentrancy guard pattern with a "locked" boolean. Set to true on first call and require false on subsequent calls. This limits calls to a single transaction.

Add limits to the maximum number of withdrawals permitted in a single transaction. This caps the damage that could be done even if reentrancy occurs.

Consider an alternative withdrawal mechanism that requires approvals rather than just require checks. Approvals provide stronger guarantees that reentrant calls cannot occur.

Ensure any UI/UX clearly displays pending withdrawals and limits new requests accordingly. This makes reentrancy attacks by users less likely.

Conduct periodic audits of the withdrawal logic to identify any new reentrancy opportunities before deployment to mainnet. Careful auditing is required to minimize risks.

Consider using a "refund / queue cancel" function to revert withdrawals if an issue is detected. This contains damage from successful reentrancy attacks.

Assessed type

Reentrancy

[0xSorryNotSorry]

Spam.

[c4-pre-sort]

0xSorryNotSorry marked the issue as low quality report

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid