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

All splitters can be removed

sabit

Summary

All splitters can be removed.

Vulnerability Details

The removeSplitter function in the LSTRewardsSplitterController contract allows the owner to remove any splitter without restrictions. \

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);
}

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

There are no checks to ensure that at least one splitter remains active. This can lead to a situation where all splitters are removed.

The contract uses the onTokenTransfer function to handle incoming token transfers:

/**
* @notice ERC677 implementation to receive an LST deposit
**/
function onTokenTransfer(address _sender, uint256 _value, bytes calldata) external {
if (msg.sender != lst) revert InvalidToken();
if (address(splitters[_sender]) == address(0)) revert SenderNotAuthorized();
​
splitters[_sender].deposit(_value);
}

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

If all splitters are removed, any attempt to transfer tokens to the contract will be rejected. The onTokenTransfer function will revert with SenderNotAuthorized error if the sender doesn't have an associated splitter.

Also, calling the withdraw funxtion will revert with SenderNotAuthorized error.

function withdraw(uint256 _amount) external {
if (address(splitters[msg.sender]) == address(0)) revert SenderNotAuthorized();
splitters[msg.sender].withdraw(_amount, msg.sender);
}

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

Impact

If all splitters are removed, any attempt to transfer tokens to the contract or withdraw tokens will be rejected.

Tools Used

Recommendations

Implement a check to ensure at least one splitter always remains:

function removeSplitter(address _account) external onlyOwner {
require(accounts.length > 1, "Cannot remove last splitter");
// ... (rest of the function)
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.