Liquid Staking

Summary

The withdrawRewards function in the OperatorVault contract calls _withdrawRewards, which interacts with external contracts and transfers tokens without a reentrancy guard. This could potentially allow reentrancy attacks if the rewardsReceiver is a malicious contract, leading to unauthorized withdrawals or draining of funds.

Vulnerability Details

In the OperatorVault contract:

Issue:

• The function interacts with external contracts (vaultController) and transfers tokens after making external calls.

• State changes (like updating unclaimedRewards) happen after external calls, which is against the Checks-Effects-Interactions pattern.

• If rewardsReceiver is a contract with a fallback function that calls withdrawRewards again, it could re-enter the function before the state is updated, leading to reentrancy.

Impact

• Reentrancy Attack: An attacker could exploit reentrancy to repeatedly withdraw funds, draining the contract.

• Financial Loss: Unauthorized withdrawals could result in significant financial loss to the contract and its stakeholders.

• System Compromise: The integrity of the staking and rewards system could be compromised.

Tools Used

Manual code review.

Recommendations

1. Add the nonReentrant modifier to withdrawRewards function to prevent reentrancy.

2. Update state variables (like unclaimedRewards) before making external calls.

3. Rearrange code so that all state changes occur before any external interaction.

4. Limit the number of external calls within a function.

5. Ensure that any external call is necessary and safe.

6. Ensure that vaultController and rewardsReceiver are trusted contracts.

7. Consider adding checks to prevent untrusted contracts from being set as rewardsReceiver.