Description

The migrate function is responsible for migrating the state of the StakeManager contract when it is bridged to the Ethereum Mainnet. However, the current implementation lacks proper checks, allowing for the _rate to be set to zero, which would allow the function to be called again.

function migrate(
    address _poolAddress,
    uint256 _validatorId,
    uint256 _govDelegated,
    uint256 _bond,
    uint256 _unbond,
    uint256 _rate,
    uint256 _totalRTokenSupply,
    uint256 _totalProtocolFee,
    uint256 _era
) external onlyAdmin {
    require(rate == 0, "already migrate");
    require(bondedPools.add(_poolAddress), "already exist");

    validatorIdsOf[_poolAddress].add(_validatorId);
    poolInfoOf[_poolAddress] = PoolInfo(
        {
        bond: _bond, 
        unbond: _unbond, 
        active: _govDelegated
        });

    rate = _rate;
    totalRTokenSupply = _totalRTokenSupply;
    totalProtocolFee = _totalProtocolFee;
    latestEra = _era;
    eraRate[_era] = _rate;
}

Impact

In addition to the obvious impact of the contract being migrated with incorrect values, if the _rate in the migrate function is set to zero, it opens the possibility of the function being called again, potentially causing unintended consequences for the contract. The limited severity in this case is due to the fact that the function can only be called by the contract's admin, and the admin is a trusted entity.

Recommendations

We recommend ensuring that all parameters are comprehensively checked before the migration is allowed to proceed. One way to do this is to implement input validation checks in the migrate function to ensure that only valid and expected values are accepted for migration. Furthermore, we highly recommend to explicitly check the _rate parameter to ensure that it is not set to zero.

Remediation

This issue has been acknowledged by StaFi Protocol, and a fix was implemented in commit 1f980d34↗.