rvierdiiev - SystemDictator.finalize can be called before step5, which will make step5 not possible to call

[github-actions[bot]]

rvierdiiev

medium

SystemDictator.finalize can be called before step5, which will make step5 not possible to call

Summary

SystemDictator.finalize can be called before step5, which will make step5 not possible to call

Vulnerability Detail

SystemDictator contract if important for upgrading to bedrock version. This contract is initialized with some configs and then do step by step upgrade of system.

currentStep variable is responsible for storing current step of upgrade. Using step modifier SystemDictator restricts calls to further steps. Everything should be called step by step.

Also SystemDictator has finalize function which will transfer ownership of some contracts, such as proxyAdmin, l1CrossDomainMessengerProxy to finalOwner. Once it's done, that means that SystemDictator is not owner of that contracts anymore.

finalize doesn't have any step protection, which means that it can be called any time. In case if it will be called before step5 function, which main task is to upgrade proxyAdmin with new implementations, that mean that it will be not possible to execute step5 anymore because of transferred ownership, so function will always revert.

As result, deployment will not be finished and protocol will need to transfer ownership back to dictator in order to proceed next.

Impact

Upgrading to bedrock can be blocked.

Code Snippet

https://github.com/sherlock-audit/2023-01-optimism/blob/main/optimism/packages/contracts-bedrock/contracts/deployment/SystemDictator.sol#L380-L384 https://github.com/sherlock-audit/2023-01-optimism/blob/main/optimism/packages/contracts-bedrock/contracts/deployment/SystemDictator.sol#L277-L319

Tool used

Manual Review

Recommendation

Restrict call to finalize function to be possible only after agreed steps.

[rcstanciu]

Comment from Optimism

---

Reason: Seems like a reasonable point and is maybe worth fixing, but not going to lead to vuln