stake.link

Summary:

The checkUpkeep function iterates over an array of strategy contracts, calling the getDepositChange() function on each. While this function is expected to be a view function, improper implementation or unbounded growth in the number of strategies could lead to unpredictable gas costs and potential DoS attacks.

Finding: The checkUpkeep function in RewardsInitiator.sol performs external calls within a loop, which could lead to increased gas costs and potential denial-of-service vulnerabilities if not properly managed.

Vulnerability Details

The function checkUpkeep contains a loop that makes external calls to getDepositChange() on strategy contracts. If the gas cost of these calls is not predictable or if the number of strategies is unbounded, the loop could consume an excessive amount of gas, potentially causing the function to fail or become prohibitively expensive to call.

Impact

If exploited, this vulnerability could prevent the checkUpkeep function from executing successfully, hindering the contract's ability to update rewards during negative rebases. This could impact the contract's intended functionality and potentially lead to financial loss or degraded performance.

Proof of Concept (PoC):

An attacker deploys several (e.g., hundreds or thousands) of malicious strategy contracts that implement the getDepositChange() function.
The attacker manages to get these strategies included in the stakingPool's strategy array.
When checkUpkeep is called, it iterates over this large array of strategies.
Each call to getDepositChange() on the malicious contracts consumes an excessive amount of gas, either due to complex calculations or deliberate gas wastage.
The cumulative gas cost of the loop exceeds the block gas limit, causing the transaction to fail.
To simulate this, one could create a mock strategy contract with a gas-intensive getDepositChange() function and add multiple instances to the stakingPool. Then, call checkUpkeep and observe the gas usage.

Tools Used

Manual code review

Recommendations

To mitigate the risk, ensure that:

The getDepositChange() function is a view function with a predictable and consistent gas cost.
Implement safeguards to limit the number of strategies to a manageable number.
Consider refactoring the contract to use a pull pattern for updating strategy states, if necessary.
If the above conditions are met and the risk is deemed acceptable, no further action may be required.