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

LSTRewardsSplitter::addFee - by accident, the same fee receiver could be added several times to the same RewardsSplitter

0xrs

Summary

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

The is no control on the addFee function that would prevent the contract owner from accidentially adding the same fee receiver several times.

Vulnerability Details

Proof of Concept:

Add the following test to lst-rewards-splitter.test.ts:

it.only('the same feeReceiver can be added several times to to a RewardsSplitter', async () => {
const { accounts, controller, token, splitter0 } = await loadFixture(deployFixture)
​
await token.transferAndCall(controller.target, toEther(100), '0x')
await token.transfer(splitter0.target, toEther(100))
​
//by accident, account6 is added a second time as fee receiver
await splitter0.addFee(accounts[6], 2000)
​
await splitter0.splitRewards()
​
//the same fee receiver is added twice to the fees array
console.log("Fees: ", await splitter0.getFees())
​
//the balance of splitter0 should be 170, but because account6 got added a second time, it is only 150
assert.equal(fromEther(await splitter0.principalDeposits()), 150)
assert.equal(fromEther(await token.balanceOf(splitter0.target)), 150)
assert.equal(fromEther(await token.balanceOf(accounts[5])), 10)
//account6 should only receive a 20% fee, but it gets an accumulated fee of 40%
assert.equal(fromEther(await token.balanceOf(accounts[6])), 40)
})

Impact

This would cause an indirect loss of funds, because the feeReceiver address (that got added more than once to the fees array) would receive more funds that it is entitled to.

Tools Used

Manual Review

Recommendations

Modify the addFee function in the following way:

error FeeReceiverAlreadyExists();
​
function addFee(address _receiver, uint256 _feeBasisPoints) external onlyOwner {
for (uint256 i = 0; i < fees.length; ++i) {
if (fees[i].receiver == _receiver) revert FeeReceiverAlreadyExists();
}
​
fees.push(Fee(_receiver, _feeBasisPoints));
if (_totalFeesBasisPoints() > 10000) revert FeesExceedLimit();
}
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.