Liquid Staking
Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

DOS Vulnerability in LSTRewardsSplitterController's removeSplitter Function

ro1sharkm

Summary

The removeSplitter function in the LSTRewardsSplitterController fails to account for balance changes after splitting rewards, attempting to withdraw more tokens than available and causing the transaction to remove a splitter to always revert.
https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/lstRewardsSplitter/LSTRewardsSplitterController.sol#L130

Vulnerability Details

In the removeSplitter function:

The function checks the initial balance and compares it to principalDeposits.
If they differ, it calls splitRewards() on the LSTRewardsSplitter contract.
The splitRewards() function distributes rewards and updates principalDeposits to the new balance.
removeSplitter then attempts to withdraw the initial balance, which now exceeds the actual balance after reward distribution.

function removeSplitter(address _account) external onlyOwner {
// ...
uint256 balance = IERC20(lst).balanceOf(address(splitter));
uint256 principalDeposits = splitter.principalDeposits();
if (balance != 0) {
if (balance != principalDeposits) splitter.splitRewards();
splitter.withdraw(balance, _account); // Potential DOS point
}
// ...
}

Impact

Permanent inability to remove splitters that have accumulated rewards.

Tools Used

Manual review

Recommendations

Update the removeSplitter function to re-check the balance after splitting rewards:

function removeSplitter(address _account) external onlyOwner {
// ...
uint256 initialBalance = IERC20(lst).balanceOf(address(splitter));
uint256 principalDeposits = splitter.principalDeposits();
if (initialBalance != 0) {
if (initialBalance != principalDeposits) splitter.splitRewards();
// Re-check the balance after splitting rewards
uint256 finalBalance = IERC20(lst).balanceOf(address(splitter));
splitter.withdraw(finalBalance, _account);
}
// ...
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

In `removeSplitter` the `principalDeposits` should be used as an input for `withdraw` instead of balance after splitting the existing rewards.

Support

FAQs

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