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

Frontrunning vulnerability in LSTRewardsSplitterController removal process

ragnarok

Summary

The owner of the LSTRewardsSplitterController contract will be unable to remove an LSTRewardsSplitter because the LSTRewardsSplitter::withdraw function reverts.

Vulnerability Detail

In the LSTRewardsSplitterController::removeSplitter function, rewards are distributed to the fee receivers of the LSTRewardsSplitter contract being removed (referred to as Contract A) if the Liquid Staking Token (LST) balance of Contract A is greater than the principal amount deposited. After the rewards are split, the LST balance of Contract A becomes lower than the initial LST balance. As a result, when the LSTRewardsSplitter::withdraw function is called with the _amount parameter equal to the initial LST balance, it reverts, causing the LSTRewardsSplitterController::removeSplitter function to fail.

Malicious users can exploit this behavior by frontrunning the LSTRewardsSplitterController::removeSplitter function, preventing the removal of an LSTRewardsSplitter. The following is an example of a potential attack:

  1. The contract owner attempts to call the LSTRewardsSplitterController::removeSplitter function to remove an LSTRewardsSplitter.

  2. If the LST balance of the LSTRewardsSplitter contract is equal to the principal amount deposited, a malicious user can frontrun the owner's transaction by sending a small amount of LST (e.g., 1000 wei) to the LSTRewardsSplitter contract. This increases the LST balance above the principal amount.

  3. As a result, the LSTRewardsSplitterController::removeSplitter transaction will fail and be reverted.

LSTRewardsSplitterController::removeSplitter function:

function removeSplitter(address _account) external onlyOwner {
...
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];
...
}

LSTRewardsSplitter::withdraw function:

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

Impact

The owner of the LSTRewardsSplitterController contract will be unable to remove an LSTRewardsSplitter.

Tools Used

Manual Review

Recommendations

Update the LSTRewardsSplitterController::removeSplitter function as follows:

function removeSplitter(address _account) external onlyOwner {
...
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);
}
​
delete splitters[_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.