Lines of code

Vulnerability details

Governor ownership can be lost because of no checks

Impact

Sanity checks are important to not affect reputation / flows and users of the protocol when a mistake is done. 0 address should be checked for important address assignments (in this case, only done in the constructor) or function calls.

PoC

l2TokenFactory calls transfer ownership to a _governor in 1 step without checks. This can force a redeployment and affect the protocol.
l2TokenFactory.transferOwnership(_governor);

Also there are no sanity checks in L2ERC20Bridge.sol constructor in:
l1Bridge = _l1Bridge;
only assigned in the constructor and not checked for 0 address.

If wrongly assigned to 0 finalizeDeposit would get blocked.

<https://github.com/code-423n4/2022-10-zksync/blob/5a31c9db8ab32175dbd7264b05ce84931b6c0428/zksync/contracts/bridge/L2ERC20Bridge.sol#L31-L42&gt;

Mitigation

• Check for 0 address
• Consider using msg.sender for _governor initial ownership.

The text was updated successfully, but these errors were encountered:

All reactions