Description

Both the init and setParams, migrate functions are used for modifying the contract's most important state variables, such as the _eraSeconds, the _minStakeAmount, and more. However, both functions lack proper checks to ensure that the parameters are within acceptable ranges or that they are not set to zero.

For example, despite the eraSeconds being checked against zero in the setParams function, there is no upper bound check to ensure that the _eraSeconds is not set to a value that is too large to be handled by the contract.

function setParams(
    uint256 _unstakeFeeCommission,
    uint256 _protocolFeeCommission,
    uint256 _minStakeAmount,
    uint256 _unbondingDuration,
    uint256 _rateChangeLimit,
    uint256 _eraSeconds,
    uint256 _eraOffset
) external onlyAdmin {
    unstakeFeeCommission = _unstakeFeeCommission == 1 ? unstakeFeeCommission : _unstakeFeeCommission;
    protocolFeeCommission = _protocolFeeCommission == 1 ? protocolFeeCommission : _protocolFeeCommission;
    minStakeAmount = _minStakeAmount == 0 ? minStakeAmount : _minStakeAmount;
    rateChangeLimit = _rateChangeLimit == 0 ? rateChangeLimit : _rateChangeLimit;
    eraSeconds = _eraSeconds == 0 ? eraSeconds : _eraSeconds;
    eraOffset = _eraOffset == 0 ? eraOffset : _eraOffset;
    if (_unbondingDuration > 0) {
        unbondingDuration = _unbondingDuration;
        emit SetUnbondingDuration(_unbondingDuration);
    }
}

Impact

The lack of checks on the parameters may result in the contract being set to an invalid state or a state that is not expected by the contract's users. For example, setting the _eraSeconds to a very large value may result in the contract being unable to handle eras properly, since it would take too long for the contract to progress to the next era.

Recommendations

We recommend ensuring that all parameters are comprehensively checked, in a transparent way. One way to do this is to implement input validation checks in the setParams, migrate and init functions to ensure that only valid and expected values are accepted for modification.

Remediation

This issue has been acknowledged by StaFi Protocol, and fixes were implemented in the following commits:

• 1f980d34↗
• c2053dc3↗