code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

Function forceDeployOnAddress() is to be used only during an upgrade to set bytecodes on specific addresses. but there is no check that the supplied address is not and EOA address or it's related to the System and zkSync protocol. so it's possible to deploy code to users and other protocols addresses which can result in total fund loss for EOAs and other protocols.

Proof of Concept

This is forceDeployOnAddress() code:

    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);
    }

As you can see There is no whitelist for newAddress and there is no check that newAddress does not belong to and EOA address. so if deployer specify wrong address when calling this function then code would get deployed to that address. This can cause fund loss for example if code get deployed to other protocol addresses in L2 or EOA addresses. it would break the other protocols and their users may lose funds. for EOA it would be possible to execute code in the context of the EOA address and steal or lock the funds(for example call withdraw if there is withdraw function in the code).

Tools Used

VIM

Recommended Mitigation Steps

code should check and make sure that newAddress is in kernel space or there should be a whitelist of addresses that it is only possible to upgrade those addresses.

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #66

c4-judge commented 1 year ago

GalloDaSballo changed the severity to QA (Quality Assurance)

GalloDaSballo commented 1 year ago

L

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