Liquid Staking

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

Missing fee check in constructor can cause DoS of updating the fee

Summary

Missing fee check in constructor can cause DoS of updating the fee

Vulnerability Details

In stake.link protocol maximum fee amounts for the receivers are predetermined by the protocol. We saw many fee amount checks while adding and updating fees. But this check is not implemented in constructor in LSTRewardSplitter:

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);
// @audit fee check required here, if it's big enough it's impossible to update fees
}

Normally, admin inputs are not considired valid findings but this problem can make the fees can't be updateable. So, normally if admin sets incorrect fees in the constructor he/she can update it after deployment in update fee function

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(); // @audit-info there is no way to update fee if it's too big
}

But this is not the case in some situations. For instance in following scenario:

  1. Admin put 10 fee receivers and each receiver has 1500 basis points.

  2. It exceed 10000 max point , so it's invalid input from admin.

  3. Admin wants to update the basis of the receivers in order to make it correct.

  4. Admin can change the basis points one by one to 300 level which is valid fee.

  5. It's not possible because updateFee will revert whle updating single fee receiver

  6. 15000 is total basis points will be 13800 after update and it will revert because it's still higher than 10000

Impact

Denial of Services on updateFee()function and some functions will be locked and cause some other DoS.

Tools Used

Manual Review

Recommendations

Implementing a check to constructor will solve it

Updates

Lead Judging Commences

inallhonesty Lead Judge 10 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.