code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/ContractDeployer.sol#L232-L247

Vulnerability details

Impact

System upgrade is as the name suggests used for upgrading system parts. In case of the address named as FORCE_DEPLOYER it has the ability to change/upgrade system contracts.

Any other user can not use metamorphic contracts (contracts that can be redeployed with new code to the same address) because selfdestruct opcode is not supported.

But the FORCE_DEPLOYER user can still redeploy new contract logic on existing/any address, not just the system contract ones.

Proof of Concept

FORCE_DEPLOYER address can re-deploy new contract version not only for upgrading system contracts but also change any other contract code:

https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/ContractDeployer.sol#L232-L247

The call off forceDeployOnAddress

        for (uint256 i = 0; i < deploymentsLength; ++i) {
            this.forceDeployOnAddress{value: _deployments[i].value}(_deployments[i], msg.sender);
        }

can re/create contracts on any/existing addresses. The code for creation is identical to the normal code the user calls when deploys contracts:

https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/ContractDeployer.sol#L212-L227

The code other users goes trough on create/create2 :

https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/ContractDeployer.sol#L274-L291

Tools Used

Manual review

Recommended Mitigation Steps

Don`t allow the FORCE_DEPLOYER address to alter other (non system) contracts.

function forceDeployOnAddress(ForceDeployment calldata _deployment, address _sender) external payable onlySelf {
+   require(uint160(_deployment.newAddress) <= uint160(MAX_SYSTEM_CONTRACT_ADDRESS), "not system contract");
    _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);
    }

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