From 5a7c176ce1526f5ef52771c66e94592070d2057e Mon Sep 17 00:00:00 2001 From: wborgeaud Date: Thu, 18 May 2023 18:18:33 +0200 Subject: [PATCH] Fix issues related to CREATE2 collisions (#1050) * Fix issues related to CREATE2 collisions * Add error functions --- evm/src/cpu/kernel/asm/core/create.asm | 34 ++++++++++++++----- .../asm/core/create_contract_account.asm | 17 +++++++--- evm/src/cpu/kernel/asm/core/transfer.asm | 1 + 3 files changed, 38 insertions(+), 14 deletions(-) diff --git a/evm/src/cpu/kernel/asm/core/create.asm b/evm/src/cpu/kernel/asm/core/create.asm index 9ca6c661..1cb64f8d 100644 --- a/evm/src/cpu/kernel/asm/core/create.asm +++ b/evm/src/cpu/kernel/asm/core/create.asm @@ -81,12 +81,6 @@ global create_common: %checkpoint - // Create the new contract account in the state trie. - DUP1 - // stack: address, address, value, code_offset, code_len, kexit_info - %create_contract_account - // stack: status, address, value, code_offset, code_len, kexit_info - %jumpi(fault_exception) // stack: address, value, code_offset, code_len, kexit_info DUP2 DUP2 %address %transfer_eth %jumpi(panic) // We checked the balance above, so this should never happen. DUP2 DUP2 %address %journal_add_balance_transfer // Add journal entry for the balance transfer. @@ -127,6 +121,12 @@ run_constructor: %set_new_ctx_gas_limit // stack: new_ctx, address, kexit_info + // Create the new contract account in the state trie. + DUP2 + %create_contract_account + // stack: status, new_ctx, address, kexit_info + %jumpi(create_collision) + %enter_new_ctx // (Old context) stack: new_ctx, address, kexit_info @@ -141,13 +141,13 @@ after_constructor: POP // EIP-3541: Reject new contract code starting with the 0xEF byte - PUSH 0 %mload_current(@SEGMENT_RETURNDATA) %eq_const(0xEF) %jumpi(fault_exception) + PUSH 0 %mload_current(@SEGMENT_RETURNDATA) %eq_const(0xEF) %jumpi(create_first_byte_ef) // Charge gas for the code size. // stack: leftover_gas, success, address, kexit_info %returndatasize // Size of the code. // stack: code_size, leftover_gas, success, address, kexit_info - DUP1 %gt_const(@MAX_CODE_SIZE) %jumpi(fault_exception) + DUP1 %gt_const(@MAX_CODE_SIZE) %jumpi(create_code_too_large) // stack: code_size, leftover_gas, success, address, kexit_info %mul_const(@GAS_CODEDEPOSIT) // stack: code_size_cost, leftover_gas, success, address, kexit_info @@ -189,14 +189,30 @@ after_constructor_failed: %jump(after_constructor_contd) create_insufficient_balance: - // stack: address, value, code_offset, code_len, kexit_info + %revert_checkpoint %stack (address, value, code_offset, code_len, kexit_info) -> (kexit_info, 0) EXIT_KERNEL nonce_overflow: + %revert_checkpoint %stack (sender, address, value, code_offset, code_len, kexit_info) -> (kexit_info, 0) EXIT_KERNEL +create_collision: + %revert_checkpoint + %stack (new_ctx, address, kexit_info) -> (kexit_info, 0) + EXIT_KERNEL + +create_first_byte_ef: + %revert_checkpoint + %stack (leftover_gas, success, address, kexit_info) -> (kexit_info, 0) + EXIT_KERNEL + +create_code_too_large: + %revert_checkpoint + %stack (code_size, leftover_gas, success, address, kexit_info) -> (kexit_info, 0) + EXIT_KERNEL + %macro set_codehash %stack (addr, codehash) -> (addr, codehash, %%after) %jump(set_codehash) diff --git a/evm/src/cpu/kernel/asm/core/create_contract_account.asm b/evm/src/cpu/kernel/asm/core/create_contract_account.asm index 905104e0..fe36f066 100644 --- a/evm/src/cpu/kernel/asm/core/create_contract_account.asm +++ b/evm/src/cpu/kernel/asm/core/create_contract_account.asm @@ -1,5 +1,5 @@ // Create a smart contract account with the given address and the given endowment value. -// Pre stack: value, address +// Pre stack: address // Post stack: status %macro create_contract_account // stack: address @@ -10,12 +10,19 @@ // so we can skip ahead, setting existing_balance = existing_account_ptr = 0. DUP1 ISZERO %jumpi(%%add_account) + // Check that the nonce is 0. // stack: existing_account_ptr, address DUP1 %mload_trie_data // nonce = account[0] // stack: nonce, existing_account_ptr, address - %jumpi(%%error_nonzero_nonce) + %jumpi(%%error_collision) // stack: existing_account_ptr, address - %increment %mload_trie_data // balance = account[1] + // Check that the code is empty. + %add_const(3) + // stack: existing_codehash_ptr, address + DUP1 %mload_trie_data // nonce = account[0] + %eq_const(@EMPTY_STRING_HASH) ISZERO %jumpi(%%error_collision) + // stack: existing_codehash_ptr, address + %sub_const(2) %mload_trie_data // balance = account[1] %jump(%%do_insert) %%add_account: @@ -44,10 +51,10 @@ PUSH 0 // success %jump(%%end) -// If the nonce is nonzero, that means a contract has already been deployed to this address. +// If the nonce is nonzero or the code is non-empty, that means a contract has already been deployed to this address. // (This should be impossible with contract creation transactions or CREATE, but possible with CREATE2.) // So we return 1 to indicate an error. -%%error_nonzero_nonce: +%%error_collision: %stack (existing_account_ptr, address) -> (1) %%end: diff --git a/evm/src/cpu/kernel/asm/core/transfer.asm b/evm/src/cpu/kernel/asm/core/transfer.asm index 6b23dd61..e5d8775f 100644 --- a/evm/src/cpu/kernel/asm/core/transfer.asm +++ b/evm/src/cpu/kernel/asm/core/transfer.asm @@ -87,6 +87,7 @@ global add_eth_new_account: POP // stack: addr, amount, retdest DUP2 ISZERO %jumpi(add_eth_new_account_zero) + DUP1 %journal_add_account_created %get_trie_data_size // pointer to new account we're about to create // stack: new_account_ptr, addr, amount, retdest SWAP2