code-423n4 / 2023-03-zksync-findings

6 stars 1 forks source link

Forceful deployment can overwrite any contract #109

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

Forceful deployment is meant to upgrade only specific addresses as also mentioned in comments

" /// @notice This method is to be used only during an upgrade to set bytecodes on specific addresses. /// @dev We do not require onlySystemCall here, since the method is accessible only /// by FORCE_DEPLOYER. function forceDeployOnAddresses(ForceDeployment[] calldata _deployments) external payable { "

Although it seems that this function fails to check if _deployment.newAddress is one of those specific address only. This means FORCE_DEPLOYER can give any other contract address and that contract bytecode will get upgraded

Proof of Concept

  1. Observe the forceDeployOnAddress function (called by forceDeployOnAddresses)
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);
        }

        emit ContractDeployed(_sender, _deployment.bytecodeHash, _deployment.newAddress);
    }
  1. Observe there is no check to validate that _deployment.newAddress is from a whitelisted set of address

  2. So if this function is called on a random existing contract then that contract will get overwritten with the provided bytecodes

Note: Random bytecode cannot be placed and only whitelisted bytecode can be placed. But existing contract code could get overwritten

Recommended Mitigation Steps

ForceDeployment should only work on whitelisted set of contracts and should not be allowed to upgrade any random address

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #141

c4-judge commented 1 year ago

GalloDaSballo changed the severity to QA (Quality Assurance)

GalloDaSballo commented 1 year ago

L

c4-judge commented 1 year ago

GalloDaSballo marked the issue as grade-c