[Medium - 2] A force deployed contract may be stuck in the constructor forever

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-zksync/blob/main/contracts/ContractDeployer.sol#L212 https://github.com/code-423n4/2023-03-zksync/blob/main/contracts/ContractDeployer.sol#L196-L207 https://github.com/code-423n4/2023-03-zksync/blob/main/contracts/ContractDeployer.sol#L326 https://github.com/code-423n4/2023-03-zksync/blob/main/contracts/ContractDeployer.sol#L214

Vulnerability details

Impact

The forceDeployOnAddress() function in the ContractDeployer contract may be used to redeploy contracts at a specified address. Very useful in the case of precompiles or system contracts upgrades for instance. In the deployment parameters, multiple values can be set by the force deployer. Among those is the callConstructor which is responsible for calling the _constructContract(), which calls the constructor, sets the immutable values for the contract, and finally mark the contract as constructed.

The issue is that if the force deployer does not want to call the constructor, then the contract will be in this "constructing" state forever because the flag was turned on but not turned off in this condition.

Tools Used

Manual inspection

Recommended Mitigation Steps

Turn the flag back off by marking the contract as "constructed" in an else block:

function forceDeployOnAddress(ForceDeployment calldata _deployment, address _sender) external payable onlySelf {
    _ensureBytecodeIsKnown(_deployment.bytecodeHash);
  _storeConstructingByteCodeHashOnAddress(_deployment.newAddress, _deployment.bytecodeHash);

  AccountInfo memory newAccountInfo;
  newAccountInfo.supportedAAVersion = AccountAbstractionVersion.None;
  // Accounts have sequential nonces by default.
  newAccountInfo.nonceOrdering = AccountNonceOrdering.Sequential;
  _storeAccountInfo(_deployment.newAddress, newAccountInfo);

    if (_deployment.callConstructor) {
        _constructContract(_sender, _deployment.newAddress, _deployment.input, false);
    } else {
        ACCOUNT_CODE_STORAGE_SYSTEM_CONTRACT.markAccountCodeHashAsConstructed(_deployment.newAddress);
    }

    emit ContractDeployed(_sender, _deployment.bytecodeHash, _deployment.newAddress);
}

[c4-judge]

GalloDaSballo marked the issue as primary issue

[GalloDaSballo]

Short and sweet, making primary

[c4-judge]

GalloDaSballo marked the issue as duplicate of #167

[c4-judge]

GalloDaSballo marked the issue as satisfactory