c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-10-zksync/blob/main/code/system-contracts/contracts/ContractDeployer.sol#L254

Vulnerability details

Impact

The simultaneous upgrade of multiple system contracts (including ContractDeployer) in zkSync may lead to compatibility issues, as there is no enforcement mechanism to ensure that upgraded contracts are fully compatible with their older versions. This could potentially disrupt the proper functioning of the system and lead to unexpected behavior.

Proof of Concept

Let's consider a scenario where certain system contracts, including ContractDeployer, are scheduled for an upgrade. To facilitate this, the function forceDeployOnAddresses is invoked by FORCE_DEPLOYER, and an array of ForceDeployment structs is passed as an argument. Each element in this array corresponds to a specific system contract set for upgrade. https://github.com/code-423n4/2023-10-zksync/blob/main/code/system-contracts/contracts/ContractDeployer.sol#L238 https://github.com/code-423n4/2023-10-zksync/blob/main/code/system-contracts/contracts/ContractDeployer.sol#L198

For instance, assume that only two system contracts, ContractDeployer and SystemContext, are for an upgrade. The first element of the array pertains to the new version of ContractDeployer, while the second element relates to the new version of SystemContext.

During the process of forcing deployment, an external high-level call is made to the forceDeployOnAddress function, as can be observed in line 214 of the code. https://github.com/code-423n4/2023-10-zksync/blob/main/code/system-contracts/contracts/ContractDeployer.sol#L254 https://github.com/code-423n4/2023-10-zksync/blob/main/code/system-contracts/contracts/ContractDeployer.sol#L214

This call is responsible for deploying the new version of ContractDeployer to the same address address(0x8006) with updated bytecode and functionality. After this deployment, the code execution returns to the forceDeployOnAddresses function to proceed with the deployment of the second element of the _deployments array. In this case, the second external high-level call invokes the new version of ContractDeployer to upgrade SystemContext.

The potential issue arises if the upgraded ContractDeployer is not fully compatible with its previous version, for instance, due to changes in the ForceDeployment struct or other differences in functionality. The problem stems from the fact that there is no enforcement to ensure that if ContractDeployer is included in the list of system contracts to be upgraded, it must be the last element in the _deployments array. This arrangement can inadvertently impact subsequent upgrades in the array.

Tools Used

Recommended Mitigation Steps

It is recommended to revise the line 254 as:

        for (uint256 i = 0; i < deploymentsLength; ++i) {
            if(_deployments[i].newAddress == address(this)){
               require( i == deploymentsLength - 1, "it is not the last upgrade").
            }

            this.forceDeployOnAddress{value: _deployments[i].value}(_deployments[i], msg.sender);
        }

https://github.com/code-423n4/2023-10-zksync/blob/main/code/system-contracts/contracts/ContractDeployer.sol#L254