`ERC20PermitPermissionedMint:constructor` and `OperatorRegistry:constructor` both don't check whether `_timelock_address` is a zero address.

[code423n4]

Lines of code

https://github.com/code-423n4/2022-09-frax/blob/55ea6b1ef3857a277e2f47d42029bc0f3d6f9173/src/ERC20/ERC20PermitPermissionedMint.sol#L34 https://github.com/code-423n4/2022-09-frax/blob/55ea6b1ef3857a277e2f47d42029bc0f3d6f9173/src/OperatorRegistry.sol#L41

Vulnerability details

Impact

The timelock_address can be set to a zero address.

Proof of Concept

When ERC20PermitPermissionedMint:constructor is called, the _timelock_address is not being checked for a zero address. See code snippet below. Thus if you pass in a zero address for the _timelock_address, the ERC20PermitPermissionedMint:constructorwill set timelock_address to a zero address. The same applies to the OperatorRegistry:constructor where _timelock_address is also not being checked for a zero address. See code snippet below.

// ERC20PermitPermissionedMint
// constructor
24    constructor(
25        address _creator_address,
26        address _timelock_address,
27        string memory _name,
28        string memory _symbol
29    ) 
30    ERC20(_name, _symbol)
31    ERC20Permit(_name) 
32    Owned(_creator_address)
33    {
34      timelock_address = _timelock_address;
35    }

// OperatorRegistry
// constructor
40    constructor(address _owner, address _timelock_address, bytes memory _withdrawal_pubkey) Owned(_owner) {
41        timelock_address = _timelock_address;
42        curr_withdrawal_pubkey = _withdrawal_pubkey;
43    }

Tools Used

None

Recommended Mitigation Steps

Check whether _timelock_address is a zero address inside the ERC20PermitPermissionedMint:constructor and inside the OperatorRegistry:constructor.

[FortisFortuna]

We are well aware of the permission structure. The owner will most likely be a large multisig. We mentioned the Frax Multisig in the scope too. If moving funds, it is assumed someone in the multisig would catch an invalid or malicious address.

[joestakey]

Zero-address checks, should be QA?

[0xean]

closing as invalid, low quality.