Liquid Staking

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

Withdrawal Failure in `LSTRewardsSplitterController:removeSplitter` Due to Underflow from Improper Balance Calculation After Reward Distribution

Summary

The removeSplitter function in the LSTRewardsSplitterController fails to account for rewards already distributed when calculating the balance to withdraw. This miscalculation results in a potential underflow error during the withdrawal process, leading to an unexpected contract revert. Specifically, the issue arises when the entire balance of the splitter is passed to the withdraw function without considering the portion of the balance already distributed as rewards through the splitRewards function.

Vulnerability Details

In the removeSplitter function, the function incorrectly calculates the balance passed to the withdraw function. The balance includes both the splitter's principal deposits and the rewards distributed to various accounts. After rewards are split, the actual balance of the splitter decreases, but the withdraw function still attempts to withdraw the full, original balance, which causes an underflow in the principalDeposits.

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 removeSplitter function, we track the value of LST tokens (the balance of a splitter) in the balance variable and the principalDeposits of a specific splitter. If the values of balance and principalDeposits are not equal—possibly due to a direct transfer of LST tokens to the splitter without using the deposit function—we split the rewards. In the splitRewards function, some LST tokens are transferred as fee amounts to the fee.receiver, and the principalDeposits state variable is updated to reflect the actual balance of LST tokens.

Thus, the value of LST tokens is now different. However, we still pass the cached balance variable—which doesn't reflect the updated LST token amount of the splitter—to the withdraw function. This leads to an underflow error because the principalDeposits is less than _amount, or the total balance of LST tokens, causing an unexpected revert.

POC

Step-by-Step Scenario to Trigger the Bug:

1. User Interaction:

Alice, a user of the protocol, has deposited 1,000 LST into her splitter contract, managed by the protocol. This 1,000 LST is the principal deposit. Over time, Alice earns rewards of 100 LST, bringing the total balance of her splitter to 1,100 LST.

  • Principal Deposits: 1,000 LST

  • Accumulated Rewards: 100 LST

  • Total Balance: 1,100 LST

2. Protocol Action - Removing Splitter:

At some point, the protocol decides to remove Alice’s splitter contract. The controller contract calls the removeSplitter function to:

  • Withdraw the entire balance from the splitter.

  • Transfer the funds back to Alice’s wallet.

Before calling withdraw, the removeSplitter function checks whether the balance and the principal deposits match. Since the balance is 1,100 LST and the principal is 1,000 LST, the contract calls splitRewards to distribute the 100 LST rewards to the fee receivers.

3. Bug Triggered - Incorrect Withdraw:

After splitRewards is called, the protocol proceeds to withdraw the balance from Alice’s splitter:

splitter.withdraw(balance, _account);
  • The balance is still 1,100 LST, but the principal deposits are now reduced due to the rewards being split.

  • If the principalDeposits is reduced to a value less than 1,100 LST (perhaps 1,000 LST or lower), calling withdraw(1,100) will underflow when the contract tries to decrease principalDeposits:

principalDeposits -= _amount; // _amount = 1,100 LST

This causes the contract to revert, preventing Alice from withdrawing her funds and completing the removal of the splitter.

4. Impact on Alice:

Due to this bug:

  • Alice’s withdrawal fails, leaving her funds locked in the splitter.

  • The protocol is unable to remove Alice’s splitter contract, and the funds are stuck.

Impact

The improper handling of the balance during the removeSplitter function can result in user funds being locked within the splitter contract. If the Controller incorrectly withdraws the full balance (including rewards) without adjusting for the distributed rewards, it triggers an underflow error and unable to remove Splitters.

Tools Used

Manual Review

Recommendations

After calling the splitRewards function, update the cached balance variable to correctly account for the changes in principal deposits, preventing underflows during withdrawals.

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 != principalDeposits) {
splitter.splitRewards();
+ // Update the balance after splitting rewards to reflect the correct balance
+ balance = IERC20(lst).balanceOf(address(splitter));
}
if (balance != 0) {
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);
}
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.