code-423n4 / 2023-06-lukso-findings

3 stars 1 forks source link

`_deployCreate()`/`_deployCreate2()` will not work on ZKSync Era #121

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/ERC725Alliance/ERC725/blob/v5.1.0/implementations/contracts/ERC725XCore.sol#L221-L245

Vulnerability details

Bug Description

In the contest's Scoping Details, the sponsor states that Universal Profiles might eventually be deployed across multiple chains:

Is it multi-chain?

LUKSO itself is not a multi-chain. The lsp-smart-contracts are initially intended to be used on the LUKSO network. However, the Universal Profile of a user could be deployed at the same address across multiple chains (using for instance Nick's method), to enable the user to use the same UP across multiple networks / chains and offer a different user experience.

According to ZKSync Era's documentation, the bytecode of a contract must be known beforehand by the compiler for the CREATE/CREATE2 opcode to work:

To guarantee that create/create2 functions operate correctly, the compiler must be aware of the bytecode of the deployed contract in advance.

However, in _deployCreate() and _deployCreate2(), the contract's bytecode is provided by the caller as an argument to execute():

ERC725XCore.sol#L221-L245

    function _deployCreate(
        uint256 value,
        bytes memory creationCode
    ) internal virtual returns (bytes memory newContract) {
        // Some code here...

        address contractAddress;
        // solhint-disable-next-line no-inline-assembly
        assembly {
            contractAddress := create(value, add(creationCode, 0x20), mload(creationCode))
        }

        // Some code here...
    }

Therefore, these functions will not work on ZKSync Era.

Impact

If Universal Profiles are deployed on ZKSync Era, users will not be able to deploy contracts through LSP0ERC725Account.

Recommended Mitigation

Unfortunately, this is a limitation of the ZKSync Era chain that cannot be avoided.

Assessed type

Other

c4-pre-sort commented 1 year ago

minhquanym marked the issue as primary issue

c4-sponsor commented 1 year ago

CJ42 marked the issue as sponsor disputed

CJ42 commented 1 year ago

This is invalid, as the issue is related to a specific blockchain environment, not the contracts themselves.

c4-judge commented 1 year ago

trust1995 marked the issue as unsatisfactory: Invalid