Liquid Staking

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

DoS Vulnerability Due to Excessive Withdrawal Post-Reward Distribution

Summary

The removeSplitter function in LSTRewardsSplitterController contract causes a denial of service (DoS) as it attempts to withdraw more than the available balance after distributing rewards
https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/lstRewardsSplitter/LSTRewardsSplitterController.sol#L130

Vulnerability Details

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

In the removeSplitter function, the contract follows this sequence:

The contract first checks the current balance of the splitter and compares it to principalDeposits

uint256 balance = IERC20(lst).balanceOf(address(splitter));
uint256 principalDeposits = splitter.principalDeposits();

If the balance differs from principalDeposits, it indicates that rewards have accumulated. The contract then calls splitRewards() to distribute the rewards.

if (balance != principalDeposits) splitter.splitRewards();

In the splitRewards() function of LSTRewardsSplitter, rewards are distributed, and the contract updates principalDeposits to the current balance:

principalDeposits = lst.balanceOf(address(this));

After reward distribution, the removeSplitter function attempts to withdraw the initial balance from the splitter

splitter.withdraw(balance, _account);

However, because the rewards have already been distributed, the balance is reduced, and the contract attempts to withdraw more than what is available. This leads to a failed transaction and a DoS scenario.

Impact

This bug can result in a denial of service when attempting to remove a splitter. A splitter can be malicious/faulty or needs to upgrades and there is a need for removal.

Tools Used

Manual review

Recommendations

Implement a check before the withdraw function to ensure the contract only withdraws the available balance.

Updates

Lead Judging Commences

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