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

Admin can't remove splitter.

danielarmstrong

Summary

When there is rewards in the splitter, LSTRewardsSplitterController.removeSplitter() function will be reverted by logical error.

Vulnerability Details

https://github.com/Cyfrin/2024-09-stakelink/tree/main/contracts/core/lstRewardsSplitter/LSTRewardsSplitterController.sol#L138

LSTRewardsSplitterController.removeSplitter() function is following.

function removeSplitter(address _account) external onlyOwner {
ILSTRewardsSplitter splitter = splitters[_account];
if (address(splitter) == address(0)) revert SplitterNotFound();
​
134: uint256 balance = IERC20(lst).balanceOf(address(splitter));
uint256 principalDeposits = splitter.principalDeposits();
if (balance != 0) {
137: if (balance != principalDeposits) splitter.splitRewards();
138: splitter.withdraw(balance, _account);
}
​
--- SKIP ---
}

As can be seen in L134, the balance is the initial LST balance of the splitter. If there are rewards to be splitted to receivers, the function calls splitRewards() in L137. As a result, the LST balance of the splitter will be decreased as the amount of splitted fees. Therefore the LST balance of the splitter will be smaller than balance and the withdraw() function call of L138 will be reverted by lacks of LST balance.

PoC:
Initial State: Assume that balance = 1200 and principalDeposits = 1000.

  1. In L137, The total rewards are 1200 - 1000 = 200. Assume that the half of them 200 * 50% = 100 are split to fee receivers. Then LST balance of the splitter will be decreased to 1200 - 100 = 1100.

  2. In L138, since the balance of the splitter 1100 is smaller than balance = 1200, function call withdraw(1200, _account) will be reverted.

Impact

Admin can't remove the splitter when there are rewards to be split to fee receivers.

Tools Used

Manual Review

Recommendations

Modify the LSTRewardsSplitterController.removeSplitter() function as follows.

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);
}
​
--- SKIP ---
}
Updates

Lead Judging Commences

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