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

removeSplitter failure because of incorrect withdraw amount

aksoy

Summary

The removeSplitter function is supposed to remove an splitter contract and withdraw its funds. However, if the splitter balance doesn't match its principalDeposits, the function calls splitRewards(), which reduces the balance. After this, the function tries to withdraw the old balance (before the split), causing the transaction to fail.

Vulnerability Details

The removeSplitter function splits rewards when the balance doesn't match the principalDeposits, which reduces the splitter's balance by transferring rewards to fee receivers. After splitting, it tries to withdraw the original balance, but the balance is now lower. This causes the withdrawal to fail.

uint256 balance = IERC20(lst).balanceOf(address(splitter));
uint256 principalDeposits = splitter.principalDeposits();
if (balance != 0) {
if (balance != principalDeposits) splitter.splitRewards();
//@audit try to send old balance
splitter.withdraw(balance, _account);
}

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/lstRewardsSplitter/LSTRewardsSplitterController.sol#L137

​
it('it shouldnt be able to removeSplitter', async () => {
const { accounts, controller, token, splitter0 } = await loadFixture(deployFixture)
​
await token.transferAndCall(controller.target, toEther(100), '0x')
​
assert.equal(fromEther(await splitter0.principalDeposits()), 100)
await token.transfer(splitter0.target, toEther(22))
assert.equal(fromEther(await token.balanceOf(splitter0.target)), 122)
assert.equal(fromEther(await splitter0.principalDeposits()), 100)
await controller.removeSplitter(accounts[0])
})

Impact

The withdrawal will fail, preventing the removal of the splitter

Tools Used

Manual

Recommendations

Change the function to withdraw the new balance after splitting rewards, not the old balance:

if (balance != 0) {
if (balance != principalDeposits) splitter.splitRewards();
- splitter.withdraw(balance, _account);
+ uint256 newBalance = IERC20(lst).balanceOf(address(splitter));
+ splitter.withdraw(newBalance, _account);
}
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.