feat(EOF): disallow ExtDelegateCall to legacy bytecode (bluealloy#1572)

* Wip call ext * feat(EOF): disallow ExtDelegateCall to legacy bytecode
ch4ns1q1 · Jun 27, 2024 · d3ff9cf · d3ff9cf
1 parent 27b67a0
commit d3ff9cf
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 5 deletions.
diff --git a/crates/interpreter/src/instruction_result.rs b/crates/interpreter/src/instruction_result.rs
@@ -20,6 +20,8 @@ pub enum InstructionResult {
     CreateInitCodeStartingEF00,
     /// Invalid EOF initcode,
     InvalidEOFInitCode,
+    /// ExtDelegateCall calling a non EOF contract.
+    InvalidExtDelegateCallTarget,
 
     // Actions
     CallOrCreate = 0x20,
@@ -132,6 +134,7 @@ macro_rules! return_revert {
             | InstructionResult::OutOfFunds
             | InstructionResult::InvalidEOFInitCode
             | InstructionResult::CreateInitCodeStartingEF00
+            | InstructionResult::InvalidExtDelegateCallTarget
     };
 }
 
@@ -200,6 +203,8 @@ pub enum InternalResult {
     CreateInitCodeStartingEF00,
     /// Check for target address validity is only done inside subcall.
     InvalidEXTCALLTarget,
+    /// Internal to ExtDelegateCall
+    InvalidExtDelegateCallTarget,
 }
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
@@ -311,6 +316,9 @@ impl From<InstructionResult> for SuccessOrHalt {
             InstructionResult::InvalidEXTCALLTarget => {
                 Self::Internal(InternalResult::InvalidEXTCALLTarget)
             }
+            InstructionResult::InvalidExtDelegateCallTarget => {
+                Self::Internal(InternalResult::InvalidExtDelegateCallTarget)
+            }
         }
     }
 }

diff --git a/crates/interpreter/src/instructions/contract.rs b/crates/interpreter/src/instructions/contract.rs
@@ -241,7 +241,7 @@ pub fn extcall<H: Host + ?Sized, SPEC: Spec>(interpreter: &mut Interpreter, host
             caller: interpreter.contract.target_address,
             bytecode_address: target_address,
             value: CallValue::Transfer(value),
-            scheme: CallScheme::Call,
+            scheme: CallScheme::ExtCall,
             is_static: interpreter.is_static,
             is_eof: true,
             return_memory_offset: 0..0,
@@ -277,8 +277,7 @@ pub fn extdelegatecall<H: Host + ?Sized, SPEC: Spec>(interpreter: &mut Interpret
             caller: interpreter.contract.caller,
             bytecode_address: target_address,
             value: CallValue::Apparent(interpreter.contract.call_value),
-            // TODO(EOF) should be EofDelegateCall?
-            scheme: CallScheme::DelegateCall,
+            scheme: CallScheme::ExtDelegateCall,
             is_static: interpreter.is_static,
             is_eof: true,
             return_memory_offset: 0..0,
@@ -313,7 +312,7 @@ pub fn extstaticcall<H: Host + ?Sized>(interpreter: &mut Interpreter, host: &mut
             caller: interpreter.contract.target_address,
             bytecode_address: target_address,
             value: CallValue::Transfer(U256::ZERO),
-            scheme: CallScheme::Call,
+            scheme: CallScheme::ExtStaticCall,
             is_static: true,
             is_eof: true,
             return_memory_offset: 0..0,

diff --git a/crates/interpreter/src/interpreter_action/call_inputs.rs b/crates/interpreter/src/interpreter_action/call_inputs.rs
@@ -130,6 +130,27 @@ pub enum CallScheme {
     DelegateCall,
     /// `STATICCALL`
     StaticCall,
+    /// `EXTCALL`
+    ExtCall,
+    /// `EXTSTATICCALL`
+    ExtStaticCall,
+    /// `EXTDELEGATECALL`
+    ExtDelegateCall,
+}
+
+impl CallScheme {
+    /// Returns true if it is EOF EXT*CALL.
+    pub fn is_ext(&self) -> bool {
+        matches!(
+            self,
+            Self::ExtCall | Self::ExtStaticCall | Self::ExtDelegateCall
+        )
+    }
+
+    /// Returns true if it is ExtDelegateCall.
+    pub fn is_ext_delegate_call(&self) -> bool {
+        matches!(self, Self::ExtDelegateCall)
+    }
 }
 
 /// Call value.

diff --git a/crates/revm/src/context/evm_context.rs b/crates/revm/src/context/evm_context.rs
@@ -7,7 +7,7 @@ use crate::{
     interpreter::{
         return_ok, CallInputs, Contract, Gas, InstructionResult, Interpreter, InterpreterResult,
     },
-    primitives::{Address, Bytes, EVMError, Env, U256},
+    primitives::{Address, Bytes, EVMError, Env, EOF_MAGIC_BYTES, U256},
     ContextPrecompiles, FrameOrResult, CALL_STACK_LIMIT,
 };
 use core::{
@@ -170,9 +170,17 @@ impl<DB: Database> EvmContext<DB> {
             .inner
             .journaled_state
             .load_code(inputs.bytecode_address, &mut self.inner.db)?;
+
         let code_hash = account.info.code_hash();
         let bytecode = account.info.code.clone().unwrap_or_default();
 
+        // ExtDelegateCall is not allowed to call non-EOF contracts.
+        if inputs.scheme.is_ext_delegate_call()
+            && bytecode.bytes_slice().get(..2) != Some(&EOF_MAGIC_BYTES)
+        {
+            return return_result(InstructionResult::InvalidExtDelegateCallTarget);
+        }
+
         // Create subroutine checkpoint
         let checkpoint = self.journaled_state.checkpoint();