diff --git a/crates/interpreter/src/instruction_result.rs b/crates/interpreter/src/instruction_result.rs index aed0da331e..3a73d20a8d 100644 --- a/crates/interpreter/src/instruction_result.rs +++ b/crates/interpreter/src/instruction_result.rs @@ -61,6 +61,8 @@ pub enum InstructionResult { EofAuxDataOverflow, /// Aud data is smaller then already present data size. EofAuxDataTooSmall, + /// EXT*CALL target address needs to be padded with 0s. + InvalidEXTCALLTarget, } impl From for InstructionResult { @@ -163,6 +165,7 @@ macro_rules! return_error { | InstructionResult::EOFFunctionStackOverflow | InstructionResult::EofAuxDataTooSmall | InstructionResult::EofAuxDataOverflow + | InstructionResult::InvalidEXTCALLTarget }; } @@ -195,6 +198,8 @@ pub enum InternalResult { InternalCallOrCreate, /// Internal CREATE/CREATE starts with 0xEF00 CreateInitCodeStartingEF00, + /// Check for target address validity is only done inside subcall. + InvalidEXTCALLTarget, } #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] @@ -303,6 +308,9 @@ impl From for SuccessOrHalt { InstructionResult::ReturnContract => Self::Success(SuccessReason::EofReturnContract), InstructionResult::EofAuxDataOverflow => Self::Halt(HaltReason::EofAuxDataOverflow), InstructionResult::EofAuxDataTooSmall => Self::Halt(HaltReason::EofAuxDataTooSmall), + InstructionResult::InvalidEXTCALLTarget => { + Self::Internal(InternalResult::InvalidEXTCALLTarget) + } } } } diff --git a/crates/interpreter/src/instructions/contract.rs b/crates/interpreter/src/instructions/contract.rs index 7266e28c4e..b8de980c13 100644 --- a/crates/interpreter/src/instructions/contract.rs +++ b/crates/interpreter/src/instructions/contract.rs @@ -6,7 +6,7 @@ use crate::{ gas::{self, cost_per_word, EOF_CREATE_GAS, KECCAK256WORD}, interpreter::Interpreter, primitives::{ - eof::EofHeader, keccak256, Address, BerlinSpec, Bytes, Eof, Spec, SpecId::*, U256, + eof::EofHeader, keccak256, Address, BerlinSpec, Bytes, Eof, Spec, SpecId::*, B256, U256, }, CallInputs, CallScheme, CallValue, CreateInputs, CreateScheme, EOFCreateInputs, Host, InstructionResult, InterpreterAction, InterpreterResult, LoadAccountResult, MAX_INITCODE_SIZE, @@ -195,11 +195,29 @@ pub fn extcall_gas_calc( Some(gas_limit) } +/// Pop target address from stack and check if it is valid. +/// +/// Valid address has first 12 bytes as zeroes. +#[inline] +pub fn pop_extcall_target_address(interpreter: &mut Interpreter) -> Option
{ + pop_ret!(interpreter, target_address, None); + let target_address = B256::from(target_address); + // Check if target is left padded with zeroes. + if target_address[..12].iter().any(|i| *i != 0) { + interpreter.instruction_result = InstructionResult::InvalidEXTCALLTarget; + return None; + } + // discard first 12 bytes. + Some(Address::from_word(target_address)) +} + pub fn extcall(interpreter: &mut Interpreter, host: &mut H) { require_eof!(interpreter); - pop_address!(interpreter, target_address); - // TODO check if target is left padded with zeroes. + // pop target address + let Some(target_address) = pop_extcall_target_address(interpreter) else { + return; + }; // input call let Some(input) = extcall_input(interpreter) else { @@ -234,9 +252,11 @@ pub fn extcall(interpreter: &mut Interpreter, host pub fn extdelegatecall(interpreter: &mut Interpreter, host: &mut H) { require_eof!(interpreter); - pop_address!(interpreter, target_address); - // TODO check if target is left paddded with zeroes. + // pop target address + let Some(target_address) = pop_extcall_target_address(interpreter) else { + return; + }; // input call let Some(input) = extcall_input(interpreter) else { @@ -269,9 +289,11 @@ pub fn extdelegatecall(interpreter: &mut Interpret pub fn extstaticcall(interpreter: &mut Interpreter, host: &mut H) { require_eof!(interpreter); - pop_address!(interpreter, target_address); - // TODO check if target is left padded with zeroes. + // pop target address + let Some(target_address) = pop_extcall_target_address(interpreter) else { + return; + }; // input call let Some(input) = extcall_input(interpreter) else { diff --git a/crates/revm/src/context/inner_evm_context.rs b/crates/revm/src/context/inner_evm_context.rs index 32c2826a05..8475d1dea4 100644 --- a/crates/revm/src/context/inner_evm_context.rs +++ b/crates/revm/src/context/inner_evm_context.rs @@ -423,7 +423,8 @@ impl InnerEvmContext { } // Prague EOF - if spec_id.is_enabled_in(PRAGUE_EOF) && inputs.init_code.get(..2) == Some(&[0xEF, 00]) { + if spec_id.is_enabled_in(PRAGUE_EOF) && inputs.init_code.get(..2) == Some(&EOF_MAGIC_BYTES) + { return return_error(InstructionResult::CreateInitCodeStartingEF00); } diff --git a/crates/revm/src/evm.rs b/crates/revm/src/evm.rs index 9c67a8e984..a7ceec6ba9 100644 --- a/crates/revm/src/evm.rs +++ b/crates/revm/src/evm.rs @@ -7,7 +7,7 @@ use crate::{ }, primitives::{ specification::SpecId, BlockEnv, CfgEnv, EVMError, EVMResult, EnvWithHandlerCfg, - ExecutionResult, HandlerCfg, ResultAndState, TxEnv, TxKind, + ExecutionResult, HandlerCfg, ResultAndState, TxEnv, TxKind, EOF_MAGIC_BYTES, }, Context, ContextWithHandlerCfg, Frame, FrameOrResult, FrameResult, }; @@ -350,13 +350,7 @@ impl Evm<'_, EXT, DB> { TxKind::Create => { // if first byte of data is magic 0xEF00, then it is EOFCreate. if spec_id.is_enabled_in(SpecId::PRAGUE_EOF) - && ctx - .env() - .tx - .data - .get(0..2) - .filter(|&t| t == [0xEF, 00]) - .is_some() + && ctx.env().tx.data.get(0..2) == Some(&EOF_MAGIC_BYTES) { exec.eofcreate( ctx,