Nonce Behavior Discrepancy Between zkSync Era and EIP-161

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/system-contracts/contracts/ContractDeployer.sol#L322

Vulnerability details

Impact

The discrepancy in deployment nonce behavior between zkSync Era and EVM can cause problems for contract factories and developers. zkSync Era starts the deployment nonce at zero, unlike the EVM, where it starts at one. This difference may lead to incorrect predictions of child contract addresses.

Proof of Concept

As per EIP-161, it's specified that account creation transactions and the CREATE operation should increase the nonce beyond its initial value by one.
https://github.com/ethereum/EIPs/blob/master/EIPS/eip-161.md#specification

Account creation transactions and the CREATE operation SHALL, prior to the execution of the initialisation code, increment the nonce over and above its normal starting value by one (for normal networks, this will be simply 1, however test-nets with non-zero default starting nonces will be different).

In other words, when an EOA (for example with nonce 100) deploys a contract (named as "contract X"), the nonces will be (please note that the nonce of the newly-deployed contract X is also incremented by one):

nonce(EOA): 100 -> 101 
nonce(contract X): 0 -> 1

And when in another transaction, this contract X deploys another contract (named as "contract Y"), the nonces will be (again please note that the nonce of the newly-deployed contract Y is also incremented by one):

nonce(EOA): 101 -> 102 
nonce(contract X): 1 -> 2
nonce(contract Y): 0 -> 1

However, during the zkSync Era, there is a divergence from the Ethereum standard. In this context, the deployment nonce for a newly created contract initiates at zero. This deviation from the EVM standard can impact factories that anticipate the addresses of child contracts and make decisions based on these assumptions. Such factories might mistakenly assume that their nonce starts at 1, mirroring the EVM, leading to discrepancies between anticipated and actual addresses of the deployed child contracts.

Tools Used

Recommended Mitigation Steps

It is advisable to increment the deployment nonce of a contract by one before invoking its constructor. Moreover, this contrast should be documented to provide clarity for developers.

function _constructContract(
        address _sender,
        address _newAddress,
        bytes32 _bytecodeHash,
        bytes calldata _input,
        bool _isSystem,
        bool _callConstructor
    ) internal {
        NONCE_HOLDER_SYSTEM_CONTRACT.incrementDeploymentNonce(_newAddress);
        //...
    }

https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/system-contracts/contracts/ContractDeployer.sol#L322

Assessed type

Context

[c4-pre-sort]

bytes032 marked the issue as low quality report

[c4-pre-sort]

bytes032 marked the issue as primary issue

[miladpiri]

Medium/Low. The impact can be high (depending on the context), and probality is medium to high (any factory can be affected).
I am thinking that if a factory creates a child, and child also creates second contract, and factory already trasnfers ETH to the second contract address (assuming that nonce of child is one, leading to predict the second contract address wrongly). Fund is lost.

[c4-sponsor]

miladpiri (sponsor) acknowledged

[GalloDaSballo]

We have an historical record of awarding non-evm equivalence that can cause damage as med, so I'm inclined to maintain the severity
Will double check

[GalloDaSballo]

The Warden has shown a discrepancy in the CREATE opcode for Contracts deployed on the zkEVM

While impact should be low in many scenarios, this highlights a discrepancy between zkEVM and EVM, which is notable, for this reason Medium Severity seems most appropriate

[c4-judge]

GalloDaSballo marked the issue as selected for report

[HE1M]

@GalloDaSballo Thank you for your effort.

The identified issue could potentially affect numerous widely-used projects and libraries. As an illustration:

The create3 library facilitates EVM contract creation, resembling CREATE2 but differing in that it excludes the contract initCode from the address derivation formula. The process involves employing the CREATE2 method to deploy a new proxy contract, which subsequently deploys the child contract using CREATE. This keyless deployment approach removes reliance on the account owner who deployed the factory contract.

Examining the CREATE3 library in solmate or solady, the child contract's address is computed based on the address of the proxy contract and its nonce. Notably, the nonce of the proxy contract is hardcoded as hex"01". This choice aligns with EIP-161, specifying that account creation transactions and the CREATE operation should increment the nonce beyond its initial value by one. However, in the zkSync Era, the nonce does not increase by one, so unexpectedly this mechanism does not work on zkSync Era as on EVM.

Given that this library or a similar mechanism is widely used (as seen in AxelarNetwork and MeanFinance), any deviation from the expected behavior could impact numerous contracts dependent on the correct address of the child contract.

Consequently, I assert that the significance of this bug should be classified as high.

[miladpiri]

@HE1M thanks.

Agree. This is an issue.

[GalloDaSballo]

After reviewing the issue and consulting with another Judge

While the issue is notable, I believe that the finding is limited in its impact in the sense that it shows a discrepancy against the EVM

Integrators would be subject to an incorrect functionality which would be found rationally through the first usage or an integration test

In the case in which said factory was in scope, then a High Severity would have been appropriate

But in lack of it, the finding impacts the compatibility of zkSync with the EVM, meaning Medium Severity is most appropriate

[vladbochok]

1. Nonce is not visible inside the VM execution, the only resulted address will be derived from the nonce 1, not 0.
2. The rule of address derivation on Era is different. Not only by a specific different formula but also by using two separate nonces - deployment nonce and min nonce. One is used for deployment and another for the contract address derivation. That is different from Ethereum itself, and has much bigger impact on the address prediction than just nonce that nonce value been used in the derivation.

I do think that the maximum impact is low, due to the reason that only infra is affected, I would like to see the real contract deployed on Ethereum that would affected rather than very hypothetical assumptions about developers and users. The likelihood is also very low. All in all, I don't think this issue has proven to have some severity.