Liquid Staking

Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Submission Details
Severity: medium
Valid

Missing check in removeSplitter() will lead to reverts in LSTRewardsSplitterController

Summary

Method removeSplitter() is used to remove the underlying splitter from the splitters[]. It withdraw() the amount before removing it. But the balance sent to withdraw() is calculated wrong. This will lead it to revert every single time. There is also a possibility of DoS as if the malicious user sends some gwei of Lst to splitter contract every time removeSplitter() is called. Which is lead it always revert.

https://github.com/Cyfrin/2024-09-stakelink/blob/main/contracts/core/lstRewardsSplitter/LSTRewardsSplitterController.sol#L136C1-L139C10

Vulnerability Details

In the removeSplitter() firstly balance is compared with principalDeposits and then splitter.splitRewards() is called. From the rewards the fees will be distributed to the recipients and final amount will be always less than balance. So the next call splitter.withdraw(balance, _account); will revert because of insufficient balance.

uint256 balance = IERC20(lst).balanceOf(address(splitter));
uint256 principalDeposits = splitter.principalDeposits();
if (balance != 0) {
if (balance != principalDeposits) splitter.splitRewards();
splitter.withdraw(balance, _account);
}

Impact

The owner will have to wait for rewards to reach rewardThreshold and then immediately execute both performUpkeep() and removeSplitter() to be able to execute it fully. As the balance == principalDeposits for the removeSplitter() to be successfull. If the malicious user sandwich these calls and send some wei to underlying splitter contract then the removeSplitter()will still revert.

Tools Used

Vs Code

Recommendations

function removeSplitter(address _account) external onlyOwner {
ILSTRewardsSplitter splitter = splitters[_account];
if (address(splitter) == address(0)) revert SplitterNotFound();
uint256 balance = IERC20(lst).balanceOf(address(splitter));
uint256 principalDeposits = splitter.principalDeposits();
if (balance != 0) {
if (balance != principalDeposits) splitter.splitRewards();
+++ principalDeposits = splitter.principalDeposits();
+++ splitter.withdraw( principalDeposits, _account);
}
delete splitters[_account];
uint256 numAccounts = accounts.length;
for (uint256 i = 0; i < numAccounts; ++i) {
if (accounts[i] == _account) {
accounts[i] = accounts[numAccounts - 1];
accounts.pop();
break;
}
}
IERC677(lst).safeApprove(address(splitter), 0);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 8 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.