Skip to content

If OWR is used with rebase tokens and there's a negative rebase, principal can be lost #79

@zobront

Description

@zobront

The OptimisticWithdrawalRecipient is deployed with a specific token immutably set on the clone. It is presumed that that token will usually be ETH, but it can also be an ERC20 to account for future integrations with tokenized versions of ETH.

In the event that one of these integrations used a rebasing version of ETH (like stETH), the architecture would need to be set up as follows:

OptimisticWithdrawalRecipient => rewards to something like LidoSplit.sol => Split Wallet

In this case, the OWR would need to be able to handle rebasing tokens.

In the event that rebasing tokens are used, there is the risk that slashing or inactivity leads to a period with a negative rebase. In this case, the following chain of events could happen:

  • distribute(PULL) is called, setting fundsPendingWithdrawal == balance
  • rebasing causes the balance to decrease slightly
  • distribute(PULL) is called again, so when fundsToBeDistributed = balance - fundsPendingWithdrawal is calculated in an unchecked block, it ends up being near type(uint256).max
  • since this is more than 16 ether, the first amountOfPrincipalStake - _claimedPrincipalFunds will be allocated to the principal recipient, and the rest to the reward recipient
  • we check that endingDistributedFunds <= type(uint128).max, but unfortunately this check misses the issue, because only fundsToBeDistributed underflows, not endingDistributedFunds
  • _claimedPrincipalFunds is set to amountOfPrincipalStake, so all future claims will go to the reward recipient
  • the pullBalances for both recipients will be set higher than the balance of the contract, and so will be unusable

In this situation, the only way for the principal to get their funds back would be for the full amountOfPrincipalStake to hit the contract at once, and for them to call withdraw() before anyone called distribute(PUSH). If anyone was to be able to call distribute(PUSH) before them, all principal would be sent to the reward recipient instead.

Recommendation

Similar to #74, I would recommend removing the ability for the OptimisticWithdrawalRecipient to accept non-ETH tokens.

Otherwise, I would recommend two changes for redundant safety:

  1. Do not allow the OWR to be used with rebasing tokens.

  2. Move the _fundsToBeDistributed = _endingDistributedFunds - _startingDistributedFunds; out of the unchecked block. The case where _endingDistributedFunds underflows is already handled by a later check, so this one change should be sufficient to prevent any risk of this issue.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions