code-423n4/2024-08-phi-validation

Arts cannot be created if protocol is deployed on zkEVMs or zkSync

c4-bot-10 opened this issue · 0 comments

Lines of code

https://github.com/code-423n4/2024-08-phi/blob/8c0985f7a10b231f916a51af5d506dd6b0c54120/src/PhiFactory.sol#L631-L634

Vulnerability details

Impact

Protocol plans to deploy on various EVM chains, according to the readme.

But on zkSync and other zkEVMs, there ways of processing the create and create2 opcodes differs from other EVMs. It requires that the compiler be aware of the bytecode beforehand. Arts are deployed with solady's cloneDeterministic which uses create2 and doesn't declare the bytecode beforehand. Therefore, new arts cannot be deployed.

Proof of Concept

In the _createNewNFTContract new arts are created using the cloneDeterministic method from solady.

    function _createNewNFTContract(
//...
        address payable newArt =
            payable(erc1155ArtAddress.cloneDeterministic(keccak256(abi.encodePacked(block.chainid, newArtId, credId))));

//...

The cloneDeterministic method creates the instance using the create2 method.
So, the instance is created via this: instance := create2(value, 0x0c, 0x35, salt).

But, in the official zkSync docs we can see that
To guarantee that create/create2 functions operate correctly, the compiler must be aware of the bytecode of the deployed contract in advance.

In our case the function doesn't know the bytecode of the deployed contract it advance, and this will lead to the problems during the deployment on the zkSync and further revert or incorrect address creation.

    /// @dev Deploys a deterministic clone of `implementation` with `salt`.
    function cloneDeterministic(address implementation, bytes32 salt)
        internal
        returns (address instance)
    {
        instance = cloneDeterministic(0, implementation, salt);
    }

    /// @dev Deploys a deterministic clone of `implementation` with `salt`.
    function cloneDeterministic(uint256 value, address implementation, bytes32 salt)
        internal
        returns (address instance)
    {
        /// @solidity memory-safe-assembly
        assembly {
            mstore(0x21, 0x5af43d3d93803e602a57fd5bf3)
            mstore(0x14, implementation)
            mstore(0x00, 0x602c3d8160093d39f33d3d3d3d363d3d37363d73)
            instance := create2(value, 0x0c, 0x35, salt)
            if iszero(instance) {
                mstore(0x00, 0x30116425) // `DeploymentFailed()`.
                revert(0x1c, 0x04)
            }
            mstore(0x21, 0) // Restore the overwritten part of the free memory pointer.
        }
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Recommend using the actual CREATE2 or ensuring that the compiler is aware of the bytecode beforehand.

For example

bytes memory bytecode = type(MyContract).creationCode;
assembly {
    addr := create2(0, add(bytecode, 32), mload(bytecode), salt)
}

Assessed type

Other