Submission Details
Severity:
`LSTRewardsSplitterController.sol::removeSplitter` Reverts When Splitter Has Balance and `balance != principalDeposits`.
## Summary
The `LSTRewardsSplitterController.sol::removeSplitter` function is designed to remove splitters. However, it reverts when the splitter has a balance, and the `balance` is not equal to `principalDeposits`. The issue arises because the function first gets the balance of the splitter and, if the balance is not 0, it checks if `balance != principalDeposits`. If true, it calls `splitter.splitRewards()`, which changes the splitter's balance. Consequently, the next line, `splitter.withdraw(balance, _account);`, fails because the balance has already changed, causing the function to revert due to insufficient balance.
## Vulnerability Details
To remove a splitter in the `LSTRewardsSplitterController` contract, the `removeSplitter` function is used:
```solidity
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();
@> splitter.withdraw(balance, _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);
}
```
In the highlighted lines, if the splitter's `balance != 0` and `balance != principalDeposits`, the function calls `splitter.splitRewards()`, which reduces the splitter's balance. As a result, the next line, `splitter.withdraw(balance, _account);`, attempts to withdraw more than the remaining balance, causing the function to revert due to insufficient funds.
## Impact
This issue causes the `removeSplitter` function to revert, preventing the removal of splitters when their `balance` does not match `principalDeposits`.
## Tools Used
Manual review.
## Recommendations
Update the `withdraw` call to use the current balance after splitting rewards, rather than the original `balance` value.
```diff
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();
- splitter.withdraw(balance, _account);
+ splitter.withdraw(IERC20(lst).balanceOf(address(splitter)), _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);
}
```
This ensures that the `withdraw` call always uses the current balance after rewards have been split, preventing the function from reverting.
Updates
Lead Judging Commences
inallhonesty 10 months ago
Submission Judgement Published Assigned finding tags:
In `removeSplitter` the `principalDeposits` should be used as an input for `withdraw` instead of balance after splitting the existing rewards.
Prize pool breakdown
Total prize
50,000 USDC
nSLOC
2,53020 USDC / LOC
42,000 USDC
4,250 USDC
3,750 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.