code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/core/StrategyManagerStorage.sol#L38-L46 https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/core/StrategyManager.sol#L838-L843 https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/core/StrategyManager.sol#L760-L764 https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/pods/DelayedWithdrawalRouter.sol#L18-L24 https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/pods/DelayedWithdrawalRouter.sol#L146-L148

Vulnerability details

Proof of Concept

For withdrawals other than beaconChainETH, the variables withdrawalDelayBlocks and MAX_WITHDRAWAL_DELAY_BLOCKS will be used to enforce a delay for withdrawals in StrategyManager.sol. Currently MAX_WITHDRAWAL_DELAY_BLOCKS is set to 50400.

uint256 public withdrawalDelayBlocks;
// the number of 12-second blocks in one week (60 * 60 * 24 * 7 / 12 = 50,400)
uint256 public constant MAX_WITHDRAWAL_DELAY_BLOCKS = 50400;

https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/core/StrategyManagerStorage.sol#L38-L46

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

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

The same variables will be used in DelayedWithdrawalRouter.sol.

https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/pods/DelayedWithdrawalRouter.sol#L18-L24

https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/pods/DelayedWithdrawalRouter.sol#L146-L148

However, different chains will have different block production times. For example, for BCS it will be 3s and for Optimism it will be 2s.

If we take Optimism as an example, the block production time will be 6 times smaller than the 12 seconds from the ETH mainnet, and, consequently, the MAX_WITHDRAWAL_DELAY_BLOCKS would need to have a value 6 times bigger to have the same behavior.

Optimism example with 2s of block production time
60 * 60 * 24 * 7 / 2 = 302400 (instead of the 50400 currently being used)

The max limit value won't work properly on the current codebase, but most importantly the delay itself will not work as expected.

Impact

Both Lido and Rocketpool allow staking to be done on L2s and this will will become more common every day on Ethereum for various projects in the ecosystem.

If EigenLayer (or a project building on top of EigenLayer) becomes functional on a L2, using irregular values for withdrawalDelayBlocks and MAX_WITHDRAWAL_DELAY_BLOCKS can make withdrawals happen sooner than expected (for chains where block production time is smaller than 12 seconds), or the inverse is also possible where withdrawal delays are set later than expected (for chains where block production time is bigger than 12 seconds, for example Arbitrum uses 15 seconds).

It's also possible for Ethereum itself to change the block product time in the future and also one of the L2 chains to achieve a block production time smaller than 1 second which would escalate the withdrawal delay problem even further.