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 about 1 year 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.

Give us feedback!