Description

Currently, there is no maximum-value check for the fee in the constructor:

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);
}

function addFee(address _receiver, uint256 _feeBasisPoints) external onlyOwner {
    // [...]
    if (_totalFeesBasisPoints() > 10000) revert FeesExceedLimit();
}

function updateFee(
    uint256 _index,
    address _receiver,
    uint256 _feeBasisPoints
) external onlyOwner {
    // [...]
    if (_totalFeesBasisPoints() > 10000) revert FeesExceedLimit();
}

Since the 10000 threshold is not checked in the constructor, arbitrarily large amounts can be assigned to the fee at that time. If the assigned fees are so large that removing one fee does not fix the problem, then this permanently bricks fee updates, because no single call to addFee or updateFee will be able to satisfy the threshold to not revert with FeesExceedLimit.

Impact

Incorrect contract construction can lead to unintended and excessive fees charged. In extreme cases, these cannot be fixed except by upgrading the vault.

Recommendations

Add a _totalFeesBasisPoints() > 10000 check to the constructor.

Remediation

This issue has been acknowledged by Stake.link, and a fix was implemented in commit 429503fd↗.