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

Double Fee Accounting

Dup1337

Summary

StakingPool's and OperatorVCS's calculation of total fees in the getStrategyRewards function results in double-counting of fees on the depositChange.

It´s because getPendingFees in the OperatorVCS strategy already includes fees calculated on the depositChange, and the getStrategyRewards function in StakingPool adds both the depositChange and getPendingFees together and then applies additional fees on the totalRewards.

This leads to the same amount being included twice in the fee calculations, resulting in incorrect total fees.

Vulnerability Details

The getStrategyRewards function in StakingPool is intended to return the total rewards and total fees for a set of strategies.
It calculates totalRewards by summing up the depositChange from each strategy and totalFees by summing up the getPendingFee from each strategy.

Contract: StakingPool.sol
​
376: /**
377: * @notice Returns the amount of rewards earned since the last call to updateStrategyRewards and the
378: * amount of fees that will be paid on the rewards
379: * @param _strategyIdxs indexes of strategies to sum rewards/fees for
380: * @return total rewards
381: * @return total fees
382: **/
383: function getStrategyRewards(
384: uint256[] calldata _strategyIdxs
385: ) external view returns (int256, uint256) {
386: int256 totalRewards;
387: uint256 totalFees;
388:
389: for (uint256 i = 0; i < _strategyIdxs.length; i++) {
390: IStrategy strategy = IStrategy(strategies[_strategyIdxs[i]]);
391: >>> totalRewards += strategy.getDepositChange();
392: totalFees += strategy.getPendingFees(); <<<
393: }
394:
395: if (totalRewards > 0) {
396: for (uint256 i = 0; i < fees.length; i++) {
397: totalFees += (uint256(totalRewards) * fees[i].basisPoints) / 10000;
398: }
399: }
400:
401: if (totalFees >= totalStaked) {
402: totalFees = 0;
403: }
404:
405: return (totalRewards, totalFees);
406: }

However, the issue arises because getPendingFees in OperatorVCS already includes fees calculated on the depositChange (L:143).
Specifically, it adds the fees calculated on depositChange to totalFees:

Contract: OperatorVCS.sol
​
135: function getPendingFees() external view override returns (uint256) {
136: uint256 totalFees;
137:
138: uint256 vaultCount = vaults.length;
139: for (uint256 i = 0; i < vaultCount; ++i) {
140: totalFees += IOperatorVault(address(vaults[i])).getPendingRewards();
141: }
142:
143: >>> int256 depositChange = getDepositChange(); // @audit called again
144: if (depositChange > 0) {
145: for (uint256 i = 0; i < fees.length; ++i) {
146: totalFees += (uint256(depositChange) * fees[i].basisPoints) / 10000;
147: }
148: }
149: return totalFees;
150: }

In getStrategyRewards above, after summing up totalRewards and totalFees from all strategies, it again calculates fees on totalRewards and adds them to totalFees:

Contract: StakingPool.sol
​
395: if (totalRewards > 0) {
396: for (uint256 i = 0; i < fees.length; i++) {
397: totalFees += (uint256(totalRewards) * fees[i].basisPoints) / 10000;
398: }
399: }

This results in the fees on depositChange being calculated twice.

Impact

Incorrect fee distribution - loss of funds due to double accounting

Tools Used

Manual Review

Recommendations

Exclude the strategy-level fees from totalFees when calculating getStrategyRewards

OR

Adjust the strategy's getPendingFees to exclude fees on depositChange

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!