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

`LSTRewardsSplitter` contract unintentionally misclassifies undistributed rewards as principal deposits

rhaydden

Summary

LSTRewardsSplitter incorrectly updates the principalDeposits value during reward distribution which could cause a loss of initial deposit information and wrong misclassification of undistributed rewards as principal.

Vulnerability Details

The _splitRewards function overwrites the principalDeposits value with the current contract balance after each reward distribution.

The function doesn't take into account for the possibility that the sum of all fee amounts might be less than the total _rewardsAmount. due to rounding down in the calculation of each fee amount.

For example, if there are multiple small fees, each calculation of amount might round down, leaving some rewards undistributed. These leftover rewards will remain in the contract and be counted as part of the principal deposits in the last line:

principalDeposits = lst.balanceOf(address(this));

This albeit causes two main issues:

-- The contract loses track of the initial deposits made by users.
-- Undistributed rewards (resulting from rounding errors in fee calculations) are incorrectly classified as principal deposits.

This can cause incorrect calculations resulting in over or under-distribution of rewards and loss of user funds.

Coded POC

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

it('PoC: Suspicious behavior in reward distribution and principal tracking', async () => {
const { signers, accounts, controller, token } = await loadFixture(deployFixture)
​
// Let's set up a splitter with fees that don't add up to 10000 basis points
await controller.addSplitter(accounts[2], [
{ receiver: accounts[9], basisPoints: 3333 }, // 33.33%
{ receiver: accounts[10], basisPoints: 3333 }, // 33.33%
{ receiver: accounts[11], basisPoints: 3333 }, // 33.33%
])
const splitter2 = await ethers.getContractAt(
'LSTRewardsSplitter',
await controller.splitters(accounts[2])
)
​
// Deposit some initial amount
await token.transferAndCall(controller.target, toEther(100), ethers.AbiCoder.defaultAbiCoder().encode(['address'], [accounts[2]]))
​
// make a function to simulate rewards and split them
async function simulateRewardsAndSplit(rewardAmount: number) {
await token.transfer(splitter2.target, toEther(rewardAmount))
await splitter2.splitRewards()
}
​
// we now simulate multiple rounds of small rewards
const rounds = 1000
for (let i = 0; i < rounds; i++) {
await simulateRewardsAndSplit(1) // 1 token of reward each time
}
​
// Check balances
const expectedDistributedAmount = rounds // 1000 rounds of 1 token each
const actualDistributedAmount =
fromEther(await token.balanceOf(accounts[9])) +
fromEther(await token.balanceOf(accounts[10])) +
fromEther(await token.balanceOf(accounts[11]))
​
console.log('Expected distributed amount:', expectedDistributedAmount)
console.log('Actual distributed amount:', actualDistributedAmount)
console.log('Undistributed amount:', expectedDistributedAmount - actualDistributedAmount)
​
// Check contract balance
const contractBalance = fromEther(await token.balanceOf(splitter2.target))
console.log('Contract balance:', contractBalance)
​
// Check principal deposits
const initialPrincipalDeposits = 100 // Initial deposit
const actualPrincipalDeposits = fromEther(await splitter2.principalDeposits())
​
console.log('Initial principal deposits:', initialPrincipalDeposits)
console.log('Actual principal deposits:', actualPrincipalDeposits)
​
// Assert the actual behavior
assert(actualDistributedAmount < expectedDistributedAmount, 'Distributed amount should be less than the total rewards due to rounding')
assert(contractBalance > 0, 'Contract should have a non-zero balance due to undistributed rewards')
assert(Math.abs(contractBalance - actualPrincipalDeposits) < 1e-6, 'Contract balance should match actual principal deposits')
assert(actualPrincipalDeposits < initialPrincipalDeposits, 'Actual principal deposits should be less than initial deposits')
})

Run the test using:

yarn test test/core/lst-rewards-splitter.test.ts

Logs:

Expected distributed amount: 1000
Actual distributed amount: 999.9000000000001
Undistributed amount: 0.09999999999990905
Contract balance: 0.1
Initial principal deposits: 100
Actual principal deposits: 0.1

Impact

-- The contract loses track of initial user deposits, potentially leading to fund loss and future reward calculations may be inaccurate due to the misclassification of undistributed rewards as principal.

Tools Used

-- yarn
-- Manual review

Recommendations

Keep track of the total distributed amount and transfer any remainder to a designated address (something like a treasury).
Or, adjust the last fee to account for any rounding errors, ensuring all rewards are distributed.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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