sherlock-audit / 2023-09-Gitcoin-judging

11 stars 7 forks source link

carrotsmuggler - Incorrect CREATE3 implementation for zksync era #866

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

carrotsmuggler

high

Incorrect CREATE3 implementation for zksync era

Incorrect CREATE3 implementation for zksync era.

Vulnerability Detail

The protocol wishes to deploy the contracts to zksync era chain as well. While EVM equivalent, zksync era has certain differences with other EVM chains which breaks some of the contract functionality.

According to the zksync docs, There a number of differences between the two chains, and the relevant one here is the calculatio of create2 based addresses.

In all other EVM chains, create2 addresses are calculated in the following manner:

address predictedAddress = address(uint160(uint(keccak256(abi.encodePacked(
    bytes1(0xff),
    address(this),
    salt,
    keccak256(abi.encodePacked(
        type(D).creationCode,
        abi.encode(arg)
    ))
)))));

This segment is taken from the solidity docs. However for zksync, as stated here, create2 addresses are calculated in a different manner.

export function create2Address(
    sender: Address,
    bytecodeHash: BytesLike,
    salt: BytesLike,
    input: BytesLike
) {
    const prefix = ethers.utils.keccak256(
        ethers.utils.toUtf8Bytes("zksyncCreate2")
    )
    const inputHash = ethers.utils.keccak256(input)
    const addressBytes = ethers.utils
        .keccak256(
            ethers.utils.concat([
                prefix,
                ethers.utils.zeroPad(sender, 32),
                salt,
                bytecodeHash,
                inputHash,
            ])
        )
        .slice(26)
    return ethers.utils.getAddress(addressBytes)
}

The min difference is the prefix. On normal EVM chains, this prefix is 0xff, but on zksync this prefix is ethers.utils.keccak256(ethers.utils.toUtf8Bytes("zksyncCreate2"). For this reason, the create2 addresses will be different on the 2 chains.

The contract Registry.sol uses CREATE3 from solady, which was written for the normal EVMS. In the CREATE3 deploy function, we can see how the addresses are calculated.

assembly {
    // Cache the free memory pointer.
    let m := mload(0x40)
    // Store `address(this)`.
    mstore(0x00, address())
    // Store the prefix.
    mstore8(0x0b, 0xff)
    // Store the salt.
    mstore(0x20, salt)
    // Store the bytecode hash.
    mstore(0x40, _PROXY_BYTECODE_HASH)

    // Store the proxy's address.
    mstore(0x14, keccak256(0x0b, 0x55))
    // Restore the free memory pointer.
    mstore(0x40, m)
    // 0xd6 = 0xc0 (short RLP prefix) + 0x16 (length of: 0x94 ++ proxy ++ 0x01).
    // 0x94 = 0x80 + 0x14 (0x14 = the length of an address, 20 bytes, in hex).
    mstore(0x00, 0xd694)
    // Nonce of the proxy contract (1).
    mstore8(0x34, 0x01)

    deployed := keccak256(0x1e, 0x17)
}

Here the 0xff prefix is used, showing that this is not compatible with zksync era chains. So the entire anchor system will break on deploying to the zksync chain.

Fork testing on foundry does not raise an error, since the foundry EVM is based on the normal EVMS, and not zksync. For specifically zksync contracts, plugins like foundry-zksync-era is available. The official docs also list out some hardhat plugins here. These plugins take into consideration the abnormal behaviour of create2 on zksync era chains.

Impact

Entire system will break, since CREATE3 will not work. This is beacause the contract will be deployed at a different address than calculated by the create3 contract due to the different prefix.

Code Snippet

https://github.com/sherlock-audit/2023-09-Gitcoin/blob/main/allo-v2/contracts/core/Registry.sol#L335-L352

Tool used

Manual Review

Recommendation

Solady create3 needs to be re-written with the correct prefix as mentioned above. Also, the testing framework needs to be changed by including plugins which take into consideration the zksync era chain differences.

Duplicate of #862