Liquid Staking

Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Submission Details
Severity: medium
Invalid

Adding Or Updating `StakingPool` Fees Does Not Update Strategy Rewards

Summary

Modifications to the fee configuration for a StakingPool can make inadvertently make retroactive claims on due rewards.

Vulnerability Details

Calling addFee or updateFee makes stepwise changes to the fee distribution model of the StakingPool which influence outstanding rewards.

Notice how the fees are directly written without settling fee accounting:

function addFee(address _receiver, uint256 _feeBasisPoints) external onlyOwner {
@> fees.push(Fee(_receiver, _feeBasisPoints));
require(_totalFeesBasisPoints() <= 4000, "Total fees must be <= 40%");
}

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L347C5-L350C6

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;
}
require(_totalFeesBasisPoints() <= 4000, "Total fees must be <= 40%");
}

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L358C5-L374C6

This changes the share of the rewards distribution for pending due rewards, resulting in undue gains or losses for fees accrued against the terms of the previous configuration.

Conversely, we note that the VaultControllerStrategy is careful to avoid this:

function addFee(address _receiver, uint256 _feeBasisPoints) external onlyOwner {
@> _updateStrategyRewards(); /// @audit:ok
fees.push(Fee(_receiver, _feeBasisPoints));
if (_totalFeesBasisPoints() > 3000) revert FeesTooLarge();
}

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/linkStaking/base/VaultControllerStrategy.sol#L649C5-L653C6

function updateFee(
uint256 _index,
address _receiver,
uint256 _feeBasisPoints
) external onlyOwner {
@> _updateStrategyRewards(); /// @audit:ok
if (_feeBasisPoints == 0) {
fees[_index] = fees[fees.length - 1];
fees.pop();
} else {
fees[_index].receiver = _receiver;
fees[_index].basisPoints = _feeBasisPoints;
}
if (_totalFeesBasisPoints() > 3000) revert FeesTooLarge();
}

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/linkStaking/base/VaultControllerStrategy.sol#L663C5-L679C6

Impact

Leads to the overestimation or underestimation of rewards resulting in:

  1. Undue claims on pending fees.

  2. The loss of due fees for affected fee recipients (i.e. those either removed or assigned a lower fee bps).

Tools Used

Manual Review

Recommendations

Claim the rewards via _updateStrategyRewards before applying fee updates.

Updates

Lead Judging Commences

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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