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

lack validation on the cumulative value of `_fees.basisPoints`

joicygiore

Summary

The addSplitter() function in LSTRewardsSplitterController and the constructor of LSTRewardsSplitter lack validation on the cumulative value of _fees.basisPoints. This omission could result in failures during reward splitting if the total basisPoints exceeds the allowable limit.

Vulnerability Details

The LSTRewardsSplitterController::addSplitter() function does not verify whether the sum of the basisPoints values from the _fees parameter falls within the acceptable range before deploying the LSTRewardsSplitter contract. This can potentially lead to errors if the sum exceeds the expected maximum.

// LSTRewardsSplitterController::addSplitter()
/**
* @notice Deploys a new splitter
* @param _account address of account to deploy splitter for
* @param _fees list of splitter fees
*/
function addSplitter(
address _account,
@> LSTRewardsSplitter.Fee[] memory _fees
) external onlyOwner {
if (address(splitters[_account]) != address(0)) revert SplitterAlreadyExists();
​
​
@> address splitter = address(new LSTRewardsSplitter(lst, _fees, owner()));
splitters[_account] = ILSTRewardsSplitter(splitter);
accounts.push(_account);
IERC677(lst).safeApprove(splitter, type(uint256).max);
}

Similarly, the constructor of LSTRewardsSplitter does not check the total value of _fees.basisPoints, which could result in errors during the reward splitting process if the cumulative basisPoints exceeds 10,000.

// LSTRewardsSplitter::constructor()
/**
* @notice Inititalizes contract
* @param _lst address of liquid staking token
* @param _fees list of fees to be applied to new rewards
* @param _owner address of owner
*/
constructor(address _lst, Fee[] memory _fees, address _owner) {
controller = ILSTRewardsSplitterController(msg.sender);
lst = IERC677(_lst);
for (uint256 i = 0; i < _fees.length; ++i) {
@> fees.push(_fees[i]);
}
_transferOwnership(_owner);
}

Without these checks, if the total basisPoints value exceeds 10,000, the LSTRewardsSplitter::_splitRewards() function will revert, causing LSTRewardsSplitter::performUpkeep() and splitRewards() to fail, potentially disrupting the reward distribution process.

Code Snippet

https://github.com/Cyfrin/2024-09-stakelink/blob/ea5574ebce3a86d10adc2e1a5f6d5512750f7a72/contracts/core/lstRewardsSplitter/LSTRewardsSplitterController.sol#L109-L124
https://github.com/Cyfrin/2024-09-stakelink/blob/ea5574ebce3a86d10adc2e1a5f6d5512750f7a72/contracts/core/lstRewardsSplitter/LSTRewardsSplitter.sol#L44-L57

Impact

If the sum of _fees.basisPoints exceeds 10,000, the LSTRewardsSplitter::_splitRewards() function will fail, resulting in the reversion of the performUpkeep() and splitRewards() procedures. This could disrupt reward distribution and degrade the functionality of the staking system.

Tools Used

Manual Review

Recommendations

add a validation check in the LSTRewardsSplitter constructor to ensure that the total basisPoints value of the _fees array does not exceed 10,000.

constructor(address _lst, Fee[] memory _fees, address _owner) {
controller = ILSTRewardsSplitterController(msg.sender);
lst = IERC677(_lst);
for (uint256 i = 0; i < _fees.length; ++i) {
fees.push(_fees[i]);
}
_transferOwnership(_owner);
+ if (_totalFeesBasisPoints() > 10000) revert FeesExceedLimit();
}
​
Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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