Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(EOF): Add target address expansion checks #1570

Merged
merged 2 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions crates/interpreter/src/instruction_result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<SuccessReason> for InstructionResult {
Expand Down Expand Up @@ -163,6 +165,7 @@ macro_rules! return_error {
| InstructionResult::EOFFunctionStackOverflow
| InstructionResult::EofAuxDataTooSmall
| InstructionResult::EofAuxDataOverflow
| InstructionResult::InvalidEXTCALLTarget
};
}

Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -303,6 +308,9 @@ impl From<InstructionResult> 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)
}
}
}
}
Expand Down
36 changes: 29 additions & 7 deletions crates/interpreter/src/instructions/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -195,11 +195,29 @@ pub fn extcall_gas_calc<H: Host + ?Sized>(
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<Address> {
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<H: Host + ?Sized, SPEC: Spec>(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 {
Expand Down Expand Up @@ -234,9 +252,11 @@ pub fn extcall<H: Host + ?Sized, SPEC: Spec>(interpreter: &mut Interpreter, host

pub fn extdelegatecall<H: Host + ?Sized, SPEC: Spec>(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 {
Expand Down Expand Up @@ -269,9 +289,11 @@ pub fn extdelegatecall<H: Host + ?Sized, SPEC: Spec>(interpreter: &mut Interpret

pub fn extstaticcall<H: Host + ?Sized>(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 {
Expand Down
3 changes: 2 additions & 1 deletion crates/revm/src/context/inner_evm_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,8 @@ impl<DB: Database> InnerEvmContext<DB> {
}

// 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);
}

Expand Down
10 changes: 2 additions & 8 deletions crates/revm/src/evm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -350,13 +350,7 @@ impl<EXT, DB: Database> 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,
Expand Down
Loading