Liquid Staking

Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Submission Details
Severity: low
Invalid

Missing access control in LSTRewardsSplitter::performUpkeep leads to race condition in rewards splitting between fee receivers

Summary

The LSTRewardsSplitterController::performUpkeep contains the logic to call performUpkeep on any LSTRewardsSplitter that was flagged as requiring upkeep in LSTRewardsSplitterController::checkUpkeep splitting new rewards between fee receivers.
The LSTRewardsSplitter::performUpkeep is missing an access control so can be called anytime by anyone leading to a race condition when called concurrently with the LSTRewardsSplitterController::performUpkeep (on the same LSTRewardsSplitter flagged as requiring upkeep in the list). The race condition occurs because both functions independently calculate and split rewards based on the balance of the LSTRewardsSplitter contract.

Link: https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/lstRewardsSplitter/LSTRewardsSplitter.sol#L101

Vulnerability Details

@> function performUpkeep(bytes calldata) external {
int256 newRewards = int256(lst.balanceOf(address(this))) - int256(principalDeposits);
if (newRewards < 0) {
principalDeposits -= uint256(-1 * newRewards);
} else if (uint256(newRewards) < controller.rewardThreshold()) {
revert InsufficientRewards();
} else {
_splitRewards(uint256(newRewards));
}
}

Impact

When performUpkeep is called in both contracts on the same LSTRewardsSplitter at the same time, each function calculates rewards independently and either function proceeds to split rewards before the other completes. This results in duplicate reward splitting and inconsistent state updates.

Likelihood: very low (execution on the same splitter must be at the same time)
Impact: medium/high (rewards on fee receiver are duplicated)

Tools Used

Manual review

Recommendations

Add onlyController modifier in the LSTRewardsSplitter::performUpkeep.

- function performUpkeep(bytes calldata) external {
+ function performUpkeep(bytes calldata) external onlyController{
int256 newRewards = int256(lst.balanceOf(address(this))) - int256(principalDeposits);
if (newRewards < 0) {
principalDeposits -= uint256(-1 * newRewards);
} else if (uint256(newRewards) < controller.rewardThreshold()) {
revert InsufficientRewards();
} else {
_splitRewards(uint256(newRewards));
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.