Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

Ro - Address collision in cross - chain contract creation (breaks tooling) #204

Open
github-actions bot opened this issue Feb 20, 2023 · 5 comments
Open
Labels
Escalation Resolved This issue's escalations have been approved/rejected Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Reward A payout will be made for this issue Specification An issue related to the specification (low severity)

Comments

@github-actions
Copy link

Ro

medium

Address collision in cross - chain contract creation (breaks tooling)

Summary

When creating a contract from L1 (mainnet) to L2 (optimism) the nonce of the sender is kept at 0 in the context of the transaction receipt and thus the existing infrastructure (Etherscan) will incorrectly interpret the address of the new contract (creating a collision).

Vulnerability Detail

It is possible to create a contract in Layer 2 from Layer 1 by sending a transaction to the optimism portal.
This is the function of interest:

 function depositTransaction(
        address _to,
        uint256 _value,
        uint64 _gasLimit,
        bool _isCreation,
        bytes memory _data
    ) public payable metered(_gasLimit)

To create a contract the "_to" address needs to be set to "address(0x0), " the "_isCreation" needs to be set to "true" and the bytecode needs to be passed in the "_data" field.

  • The contract is then created through the "create" opcode, therefore the address is computed by hashing the nonce and the address of the sender.

If the nonce is kept at 0 during some time of the execution, the new contract address will always be the same (nonce is always 0).

NOTE: In reality, the new contract is created with the proper nonce (and the nonce is updated), but the receipt displays it wrongly and during a point in the execution is also kept at 0 so it breaks Etherscan, Blockscout and the rest of the block exploreres.

Impact

Infrastructure tooling like Etherscan is the source of truth for most users, therefore having inconsistent data between these tools and Optimism is not recommended.

This bug causes existing tooling like Etherscan and Blockscout to display wrong data in the following ways:

  1. The first contract created will always appear as the new contract in contract creation.
    See this link for reference: https://goerli-optimism.etherscan.io/address/0x5d7ee88447367b9212fa655dc863a1e83f7e189d

When the account 0x7d.. creates a new contract from Layer 1, the address "0x5d.." will always appear as the new contract created.

  1. It incorrectly displays contracts as EOA's
    See this address on etherscan goerli optimism:
    https://goerli-optimism.etherscan.io/address/0x0a1c3c13c35275d030ff9fb660946cf2fca74ece
    It appears to be a regular EOA, while in reality, it is a contract.
    Run the following command to verify:
cast code 0x0a1c3c13c35275d030ff9fb660946cf2fca74ece --rpc-url https://goerli.optimism.io

Code Snippet

https://github.com/sherlock-audit/2023-01-optimism/blob/main/optimism/packages/contracts-bedrock/contracts/L1/OptimismPortal.sol#L358

Tool used

Manual Review

Recommendation

The nonce should be set to the sender's nonce from the op-node.

@rcstanciu
Copy link
Contributor

Comment from Optimism


Description: Address collision in cross - chain contract creation (breaks tooling)

Reason: This is correct, but is unexpected behavior external to the scope of the code. Thus, this is a low issue.

Action: Increment the nonce in the op-node to make transaction receipts use the correct nonce for contract creation on L1 -> L2 bridge.

@sherlock-admin sherlock-admin added the Non-Reward This issue will not receive a payout label Feb 21, 2023
@rodrigoherrerai
Copy link

rodrigoherrerai commented Feb 23, 2023

Escalate for 250 USDC

This bug clearly breaks the specifications from Optimism. The debate here is if this is marked as a Medium or as a spec.

It could be marked as medium because it breaks EVM complete equivalence (as shown below):

Bedrock’s design aims for EVM equivalence, meaning that smart contract developers should be able to safely deploy the same contracts to Optimism as they would on Ethereum, without concern for differences in the execution.

Here are concrete examples where this bug causes the specifications to differ:

Just to recap, this issue is manifested when creating contracts from L1 -> L2. The nonce is not incremented and thus the contract created in the transaction receipt will always be the same (nonce is always 0).

When creating a contract, the address is computed as follows:

computed_address = keccak256(rlp.encode([address, nonce]))
    canonical_address = computed_address[-20:]
    padded_address = left_pad_zero_bytes(canonical_address, 20)
    return Address(padded_address)

If the nonce is kept at 0 during the receipt, then the new contract address will always be the same.
https://github.com/ethereum/execution-specs/blob/master/src/ethereum/london/utils/address.py#L42

This issue breaks the following specifications:

1:

Nonce Handling
Despite the lack of signature validation, we still increment the nonce of the from account when a deposit transaction is executed. In the context of a deposit-only roll up, this is not necessary for transaction ordering or replay prevention, however it maintains consistency with the use of nonces during contract creation. It may also simplify integration with downstream tooling (such as wallets and block explorers).

Link: https://github.com/ethereum-optimism/optimism/blob/develop/specs/deposits.md#nonce-handling

Quote: "however it maintains consistency with the use of nonces during contract creation. It may also simplify integration with downstream tooling (such as wallets and block explorers)."

This issue completely breaks the integration with downstream tooling:

  • Etherscan and the rest of the block explorers show contract accounts without code (as EOA's):

This is an account with code (contract) on Optimism goerli: 0x0a1c3c13c35275d030ff9fb660946cf2fca74ece

But Etherscan and the rest of the block explorers display it as an EOA:
Screenshot 2023-02-23 at 10 12 06

  • Etherscan and the rest of the block explorers incorrectly show the contract creation as the first contract will always appear to be the contract created:

Take this transaction for example:

Screenshot 2023-02-23 at 10 53 34

It appears as if the contract created was "0x5d7..", while in reality it is incorrect. Note that the nonce also appears as "0".

It breaks complete "EVM Equivalence"

From the Optimism blog post:

…What is EVM equivalence?
In short: EVM equivalence is complete compliance with the Ethereum yellow paper, the formal definition of the protocol. By definition, L1 Ethereum software must comply with this specification.

This means that — down to the very deepest depths — the existing Ethereum stack will now integrate with the L2 system, too. Every debugger. Every toolchain. Every node implementation. We believe that any L2 offering any EVM experience will have to meet this bar — anything less is unacceptable.

Quote: "This means that — down to the very deepest depths — the existing Ethereum stack will now integrate with the L2 system, too. Every debugger. Every toolchain. Every node implementation. We believe that any L2 offering any EVM experience will have to meet this bar — anything less is unacceptable."
Link: https://medium.com/ethereum-optimism/introducing-evm-equivalence-5c2021deb306

Apart from breaking downstream tooling like block explorers and debuggers, it can also break frontend / backend applications that rely on the transaction receipt as the source of truth.

For example, when the address 0x7dF608C479d2D2a7df677fEEc6f2535Eb268da01 creates a contract from L1 to L2, the contract address in the receipt will always be the same, because the nonce is kept at 0:

{
  to: null,
  from: '0x7dF608C479d2D2a7df677fEEc6f2535Eb268da01',
  contractAddress: '0x5d7Ee88447367b9212FA655dc863A1e83f7e189D',
...

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Feb 23, 2023

Escalate for 250 USDC

This bug clearly breaks the specifications from Optimism. The debate here is if this is marked as a Medium or as a spec.

It could be marked as medium because it breaks EVM complete equivalence (as shown below):

Bedrock’s design aims for EVM equivalence, meaning that smart contract developers should be able to safely deploy the same contracts to Optimism as they would on Ethereum, without concern for differences in the execution.

Here are concrete examples where this bug causes the specifications to differ:

Just to recap, this issue is manifested when creating contracts from L1 -> L2. The nonce is not incremented and thus the contract created in the transaction receipt will always be the same (nonce is always 0).

When creating a contract, the address is computed as follows:

computed_address = keccak256(rlp.encode([address, nonce]))
    canonical_address = computed_address[-20:]
    padded_address = left_pad_zero_bytes(canonical_address, 20)
    return Address(padded_address)

If the nonce is kept at 0 during the receipt, then the new contract address will always be the same.
https://github.com/ethereum/execution-specs/blob/master/src/ethereum/london/utils/address.py#L42

This issue breaks the following specifications:

1:

Nonce Handling
Despite the lack of signature validation, we still increment the nonce of the from account when a deposit transaction is executed. In the context of a deposit-only roll up, this is not necessary for transaction ordering or replay prevention, however it maintains consistency with the use of nonces during contract creation. It may also simplify integration with downstream tooling (such as wallets and block explorers).

Link: https://github.com/ethereum-optimism/optimism/blob/develop/specs/deposits.md#nonce-handling

Quote: "however it maintains consistency with the use of nonces during contract creation. It may also simplify integration with downstream tooling (such as wallets and block explorers)."

This issue completely breaks the integration with downstream tooling:

  • Etherscan and the rest of the block explorers show contract accounts without code (as EOA's):

This is an account with code (contract) on Optimism goerli: 0x0a1c3c13c35275d030ff9fb660946cf2fca74ece

But Etherscan and the rest of the block explorers display it as an EOA:
Screenshot 2023-02-23 at 10 12 06

  • Etherscan and the rest of the block explorers incorrectly show the contract creation as the first contract will always appear to be the contract created:

Take this transaction for example:

Screenshot 2023-02-23 at 10 53 34

It appears as if the contract created was "0x5d7..", while in reality it is incorrect. Note that the nonce also appears as "0".

It breaks complete "EVM Equivalence"

From the Optimism blog post:

…What is EVM equivalence?
In short: EVM equivalence is complete compliance with the Ethereum yellow paper, the formal definition of the protocol. By definition, L1 Ethereum software must comply with this specification.

This means that — down to the very deepest depths — the existing Ethereum stack will now integrate with the L2 system, too. Every debugger. Every toolchain. Every node implementation. We believe that any L2 offering any EVM experience will have to meet this bar — anything less is unacceptable.

Quote: "This means that — down to the very deepest depths — the existing Ethereum stack will now integrate with the L2 system, too. Every debugger. Every toolchain. Every node implementation. We believe that any L2 offering any EVM experience will have to meet this bar — anything less is unacceptable."
Link: https://medium.com/ethereum-optimism/introducing-evm-equivalence-5c2021deb306

Apart from breaking downstream tooling like block explorers and debuggers, it can also break frontend / backend applications that rely on the transaction receipt as the source of truth.

For example, when the address 0x7dF608C479d2D2a7df677fEEc6f2535Eb268da01 creates a contract from L1 to L2, the contract address in the receipt will always be the same, because the nonce is kept at 0:

{
  to: null,
  from: '0x7dF608C479d2D2a7df677fEEc6f2535Eb268da01',
  contractAddress: '0x5d7Ee88447367b9212FA655dc863A1e83f7e189D',
...

You've created a valid escalation for 250 USDC!

To remove the escalation from consideration: Delete your comment.
To change the amount you've staked on this escalation: Edit your comment (do not create a new comment).

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

@sherlock-admin sherlock-admin added the Escalated This issue contains a pending escalation label Feb 23, 2023
@Evert0x
Copy link
Contributor

Evert0x commented Mar 2, 2023

Escalation accepted.

Comment provides sufficient evidence for a specification issue.

Labeling #176 as duplicate as it describes the same issue

@Evert0x Evert0x reopened this Mar 2, 2023
@sherlock-admin
Copy link
Contributor

Escalation accepted.

Comment provides sufficient evidence for a specification issue.

Labeling #176 as duplicate as it describes the same issue

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.

@sherlock-admin sherlock-admin added Escalation Resolved This issue's escalations have been approved/rejected and removed Escalated This issue contains a pending escalation labels Mar 2, 2023
@Evert0x Evert0x added the Specification An issue related to the specification (low severity) label Mar 2, 2023
@rcstanciu rcstanciu added Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Reward A payout will be made for this issue and removed Non-Reward This issue will not receive a payout labels Mar 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Escalation Resolved This issue's escalations have been approved/rejected Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Reward A payout will be made for this issue Specification An issue related to the specification (low severity)
Projects
None yet
Development

No branches or pull requests

4 participants