code-423n4 / 2023-04-eigenlayer-findings

1 stars 1 forks source link

Staker can bypass the debt accrued via `beaconChainETHSharesToDecrementOnWithdrawal` by transferring shares to another address #437

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Description

When a staker is verified to have over-committed and the over-committed amount is greater than their outstanding shares, they accrue a debt that is captured by beaconChainETHSharesToDecrementOnWithdrawal.

This debt eventually gets settled when the staker withdraws ETH in StrategyManager::_withdrawBeaconChainEth. A staker who has accrued debt this way can bypass it by queuing a withdrawal and instead of transfering tokens/ETH, they choose to transfer shares. In StrategyManager::completeQueuedWithdrawal, the staker can pass receiveAsToken as false and effectively transfer their shares to another address (withdrawer). Note that when receiveTokens is false, a new withdrawer gets shares added to their account. However, existing debt of the staker is not transferred to the new recipient.

Now, when the recipient queues a new withdrawal and chooses to withdraw ETH (instead of transferring shares), there is no debt a recipient has to pay to complete the withdrawal. Stakers can avoid paying debts accrued due to over-commitment by transferring their shares to another address via withdrawals.

Impact

Stakers can avoid paying debts accrued due to over-commitment, when they should in fact be paid, so we evaluate the severity to HIGH.

Proof of Concept

Consider following scenario with a chain of events in this order:

  1. Alice stakes 32 ETH and receives shares in BeaconEthStrategy.
  2. Alice's beacon chain balance gets slashed and watcher verifies an over-commitment.
  3. Over-commitment exceeds Alice shares and debt is accrued to Alice (beaconChainETHSharesToDecrementOnWithdrawal > 0).
  4. Alice queues a withdrawal and then completes a queued withdrawal with receiveTokens as false and withdrawer as Bob.
  5. Bob now places a withdrawal request & chooses to receive ETH.
  6. At the time of final settlement, Bob does not have any debt accrued. As a result, Bob receives the full amount.

Tools Used

Manual review

Recommended Mitigation Steps

Consider shifting the debt accrued from the staker to the new recipient when the staker chooses to transfer sharesm, i.e. beaconChainETHSharesToDecrementOnWithdrawal for transfer originator should become 0, and beaconChainETHSharesToDecrementOnWithdrawal for recipient should now reflect the debt accrued by previous staker.

Assessed type

Other

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as primary issue

c4-sponsor commented 1 year ago

Sidu28 marked the issue as sponsor disputed

Sidu28 commented 1 year ago

transferring virtual 'beacon chain ETH shares' is disallowed -- see https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/core/StrategyManager.sol#L359-L361

GalloDaSballo commented 1 year ago

Looks off because you also have: https://github.com/code-423n4/2023-04-eigenlayer/blob/398cc428541b91948f717482ec973583c9e76232/src/contracts/core/StrategyManager.sol#L798-L799

                _addShares(msg.sender, queuedWithdrawal.strategies[i], queuedWithdrawal.shares[i]);

Which means you cannot transfer to Bob

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Insufficient proof