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

The owner of LSTRewardsSplitter was incorrectly set

eason

Summary

The owner of LSTRewardsSplitter is currently set to the controller contract, which is not the intended owner. The owner should be set to the _account variable when calling addSplitter(address _account, LSTRewardsSplitter.Fee[] memory _fees).

Vulnerability Details

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

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

The LSTRewardsSplitter contract is deployed when addSplitter(address _account, LSTRewardsSplitter.Fee[] memory _fees) is called in LSTRewardsSplitterController.sol. In the constructor of LSTRewardsSplitter.sol, the owner is set to the owner() of LSTRewardsSplitterController.sol.

Impact

  • The owner of LSTRewardsSplitter is currently assigned to the controller contract, which is not the intended owner.

  • Users are unable to invoke addFee(address _receiver, uint256 _feeBasisPoints) to add fee receivers.

  • Users are unable to execute updateFee(uint256 _index, address _receiver, uint256 _feeBasisPoints) to update fees.

PoC

it('LSTRewardsSplitter set wrongly owner', async () => {
const { signers, accounts, splitter0, splitter1 } = await loadFixture(deployFixture)
​
// Error: VM Exception while processing transaction: reverted with reason string 'Ownable: caller is not the owner'
await expect(
splitter0.connect(signers[1]).addFee(accounts[9], 1000)
).to.be.revertedWith('Ownable: caller is not the owner')
​
await splitter0.addFee(accounts[9], 1000)
assert.deepEqual(await splitter0.getFees(), [
[accounts[5], 1000n],
[accounts[6], 2000n],
[accounts[9], 1000n],
])
​
await expect(
splitter1.connect(signers[2]).addFee(accounts[10], 1000)
).to.be.revertedWith('Ownable: caller is not the owner')
})

Tools Used

Manual code review

Recommendations

Set the owner of LSTRewardsSplitter to _account when calling addSplitter(address _account, LSTRewardsSplitter.Fee[] memory _fees).

function addSplitter(
address _account,
LSTRewardsSplitter.Fee[] memory _fees
) external onlyOwner {
if (address(splitters[_account]) != address(0)) revert SplitterAlreadyExists();
​
- address splitter = address(new LSTRewardsSplitter(lst, _fees, owner()));
+ address splitter = address(new LSTRewardsSplitter(lst, _fees, _account);
splitters[_account] = ILSTRewardsSplitter(splitter);
accounts.push(_account);
IERC677(lst).safeApprove(splitter, type(uint256).max);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

[INVALID] The owner of LSTRewardsSplitter is not set according to the docs

Appeal created

demorextess Judge
about 1 year ago
inallhonesty Lead Judge
12 months ago
inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

[INVALID] The owner of LSTRewardsSplitter is not set according to the docs

Support

FAQs

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