[UOF-01M] Insufficient Validation of Period Configurations

[Haypierre]

UOF-01M: Insufficient Validation of Period Configurations

Type	Severity	Location
Input Sanitization		UpgradeableOpenfortFactory.sol:L49-L54

Description:

Per the relevant finding in the BaseRecoverableAccount, the periods require further sanitization so as to ensure that the security period is less than the security window.

Impact:

The UpgradeableOpenfortAccount deployer factory can be presently misconfigured, causing discrepancies in the BaseRecoverableAccount implementation.

Example:

constructor(
    address _owner,
    bytes32 _upgradeableProxyCodeHash,
    address _accountImplementation,
    uint256 _recoveryPeriod,
    uint256 _securityPeriod,
    uint256 _securityWindow,
    uint256 _lockPeriod,
    address _initialGuardian
) BaseOpenfortFactory(_owner, _accountImplementation) {

    upgradeableProxyCodeHash = _upgradeableProxyCodeHash;

    if (
        _lockPeriod < _recoveryPeriod ||
        _recoveryPeriod < _securityPeriod + _securityWindow
    ) {
        revert InsecurePeriod();
    }
    recoveryPeriod = _recoveryPeriod;
    securityPeriod = _securityPeriod;
    securityWindow = _securityWindow;
    lockPeriod = _lockPeriod;
    if (_initialGuardian == address(0)) revert ZeroAddressNotAllowed();
    initialGuardian = _initialGuardian;
}

Recommendation:

We advise such validation to be imposed in the UpgradeableOpenfortFactory::constructor as well, preventing misconfiguration of deployed UpgradeableOpenfortAccount contracts.

[Haypierre]

Needs clarification:

InsecurePeriod() is raised under the same condition in the UpgradeableOpenfortFactory:constructor as in the BaseRecoverableAccoun initialize.