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

stting a new fee receiver will cause the old one to lose his already generated fees

pyro

Summary

Updating the fee will cause the already generated rewards for receivers to be lowered/increased based on the fee change.

Vulnerability Details

Unlike removeSplitter which splits rewards before making any changes to the splitters:

function removeSplitter(address _account) external onlyOwner {
...
if (balance != 0) {
// Splitting rewards
if (balance != principalDeposits) splitter.splitRewards();
splitter.withdraw(balance, _account);
}

updateFee, doesn't.

function updateFee(
uint256 _index,
address _receiver,
uint256 _feeBasisPoints
) external onlyOwner {
require(_index < fees.length, "Fee does not exist");
​
if (_feeBasisPoints == 0) {
fees[_index] = fees[fees.length - 1];
fees.pop();
} else {
fees[_index].receiver = _receiver;
fees[_index].basisPoints = _feeBasisPoints;
}
​
if (_totalFeesBasisPoints() > 10000) revert FeesExceedLimit();
}

Changing the receives or their amounts will cause some of them to lose a part of their already generated rewards. This shouldn't be the case, as future changes to the basisPoints should not effect past generated rewards.

Example:

  1. 3 users are splitting the fees the following manner Alice (owner) - 40%, Bob - 30%, Eve- 30%

  2. Some time passes and 100 are generated as rewards, that will be split with these fee percentages - 30 for Alice, 30 for Bob and 30 for Eve.

  3. For one reason or another Alice lowers the _feeBasisPoints for Bob to 10% and increases hers to 50% and Eve to 40%.

  4. Now when fees are split Bob will receive 10 tokens instead of 30, even thought his assets contributed to the rewards and if _splitRewards was called before the change made by Alice he would have received 30.

Impact

User losses funds.

Tools Used

Manual review

Recommendations

Call _splitRewards before changing the fee percentages or removing fee receivers. Note that this is already implemented inside removeSplitter, just not inside updateFee.

function removeSplitter(address _account) external onlyOwner {
...
if (balance != 0) {
// Splitting rewards
if (balance != principalDeposits) splitter.splitRewards();
splitter.withdraw(balance, _account);
}
Updates

Lead Judging Commences

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

Appeal created

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

`updateStrategyRewards` is not called before adding & updating the fees

It should be called with try and catch to avoid DOS by receiver.

Support

FAQs

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