updateOperator() can be called before an operator is set in proxy

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-aura/blob/main/contracts/Aura.sol#L82

Vulnerability details

Impact

In Aura.sol the updateOperator() function can be called by anyone and it sets a new operator based on the address returned from IStaker(vecrvProxy).operator(). The problem is that anyone can call this function even if the operator on vecrvProxy is not yet set. If this is the case the operator in Aura.sol would be set to a zero address breaking the contract since functions like init() and mint() rely on the msg.sender being the operator. Even the minterMint() function relies on the operator since only the operator can set the minter which is the only one who can call minterMinter().

Proof of Concept

https://github.com/code-423n4/2022-05-aura/blob/main/contracts/Aura.sol#L82

Tools Used

Manual code review

Recommended Mitigation Steps

The updateOperator() function should not be a public function and should only be callable by an admin or the operator inside Aura.sol. Also in the updateOperator() function, there should be a check ensuring that the newOperator address is not a zero address to prevent breaking the contract by setting the operator to a zero address.

[phijfry]

The voter proxy is already deployed and the operator is not set to address(0). So this can't happen.

[0xMaharishi]

This is certainly not a high severity issue, I would say a 1 at most.

The only problem that could happen here is if someone is watching the Aura deployment, and then calls updateOperator immediately after Aura has been deployed, but before the init fn has been called.

Solution is to add some protections.. a) checking system has been initialised and b) non zero address

[dmvt]

Agree that this is not high severity. I'm going to downgrade it to gas, since the impact would be that they would need to redeploy the contracts. No fund loss is possible with this issue.