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

`removeSplitter` will revert in most cases

pyro

Summary

removeSplitter will revert and splitters will not be removable.

Vulnerability Details

removeSplitter used to remove splitters (who would have guessed) has a flaw that causes it to revert in most cases. The issue is that it gets balance of the lst inside the splitter and then calls splitRewards, where _splitRewards will lower this balance, by sending the rewards to the appointed receivers.

function _splitRewards(uint256 _rewardsAmount) private {
for (uint256 i = 0; i < fees.length; ++i) {
Fee memory fee = fees[i];
uint256 amount = (_rewardsAmount * fee.basisPoints) / 10000;
​
if (fee.receiver == address(lst)) {
IStakingPool(address(lst)).burn(amount);
} else {
lst.safeTransfer(fee.receiver, amount);
}
}
​
principalDeposits = lst.balanceOf(address(this));
emit RewardsSplit(_rewardsAmount);
}

Later removeSplitter calls withdraw with that same balance, however because we have lowered it using splitRewards the contract will not have enought balance (amount of lst) to actually do the withdraw.

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();
​
// balance is outdated as `splitRewards` updates it
splitter.withdraw(balance, _account);
}

This will cause withdraw to revert, thus reverting the whole call:

function withdraw(uint256 _amount, address _receiver) external onlyController {
principalDeposits -= _amount;
lst.safeTransfer(_receiver, _amount);
emit Withdraw(_amount);
}

Impact

Core function will not work, splitter will not be removed.

Tools Used

Manual withdraw

Recommendations

Get the new balance and use that inside withdraw.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year 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.