Liquid Staking

Summary

The updateStrategyRewards function in StakingPool has an access control vulnerability that allows any strategy to call it and update rewards, when the intention was to restrict this capability to only the rebaseController. Which is due to an incorrect logical condition that checks if the caller is either the rebaseController or an existing strategy, instead of strictly requiring the caller to be the rebaseController.

Vulnerability Details

To reproduce:

1. Add a strategy to the StakingPool

2. Call updateStrategyRewards from that strategy, passing in valid parameters

3. The function will execute successfully even though the caller is not the rebaseController

The updateStrategyRewards function in the StakingPool contract has an incorrect access control check that allows any existing strategy to call the function and update rewards, bypassing the intended restriction that only the rebaseController should have this privilege.

The issue is with the condition msg.sender != rebaseController && !_strategyExists(msg.sender). This will allow the function to execute if either the caller is the rebaseController OR if the caller is an existing strategy. The intention was to only allow the rebaseController, but the && operator introduces a logical flaw that also grants access to strategies.

Burden of Proof

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L413-L417

The vulnerability is in the if condition. The condition msg.sender != rebaseController && !_strategyExists(msg.sender) is incorrect and allows any existing strategy to call updateStrategyRewards, in addition to the intended rebaseController.

Is also referenced here: https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L619-L626

The _strategyExists function is used in the vulnerable condition. It checks if the given address is an existing strategy in the strategies array.

More Details

The intention is to only allow the rebaseController to call this function and update rewards. However, the current if condition is checking:

if (msg.sender != rebaseController && !_strategyExists(msg.sender))

This means the function will revert (deny access) if the caller is not the rebaseController AND the caller is not an existing strategy.

The bug is introduced by the && (AND) operator. This condition will pass if either:

1. The caller is the rebaseController (regardless of whether they are a strategy)

2. The caller is an existing strategy (regardless of whether they are the rebaseController)

So any caller that is an existing strategy will be allowed to update rewards, bypassing the intended access control.

This vulnerability is located in the StakingPool.sol contract, specifically in the updateStrategyRewards function. It connects to the logic for updating and distributing rewards earned by strategies.

Impact

Strategies can arbitrarily manipulate rewards in the StakingPool. A malicious strategy could exploit this to inflate its own rewards or siphon rewards intended for other users. This breaks the expected behavior and trust model where only the designated rebaseController should have control over reward distribution.

Tools Used

Vs

Recommendations

updateStrategyRewards should be updated to strictly require the caller to be the rebaseController