From 5d0478bbbb6e2a6ac9d70569bd917a790b9ad11c Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Wed, 14 Mar 2018 15:27:56 +0300 Subject: [PATCH] more dos protection (#8104) --- ethcore/vm/src/schedule.rs | 6 +++++ ethcore/wasm/src/lib.rs | 46 ++++++++++++++++++------------------- ethcore/wasm/src/runtime.rs | 13 ++++++++--- ethcore/wasm/src/tests.rs | 32 +++++++++++++------------- 4 files changed, 55 insertions(+), 42 deletions(-) diff --git a/ethcore/vm/src/schedule.rs b/ethcore/vm/src/schedule.rs index cc184825c8c..7a7a5ac6b74 100644 --- a/ethcore/vm/src/schedule.rs +++ b/ethcore/vm/src/schedule.rs @@ -135,6 +135,10 @@ pub struct WasmCosts { pub initial_mem: u32, /// Grow memory cost, per page (64kb) pub grow_mem: u32, + /// Memory copy cost, per byte + pub memcpy: u32, + /// Max stack height (native WebAssembly stack limiter) + pub max_stack_height: u32, /// Cost of wasm opcode is calculated as TABLE_ENTRY_COST * `opcodes_mul` / `opcodes_div` pub opcodes_mul: u32, /// Cost of wasm opcode is calculated as TABLE_ENTRY_COST * `opcodes_mul` / `opcodes_div` @@ -153,6 +157,8 @@ impl Default for WasmCosts { static_address: 40, initial_mem: 4096, grow_mem: 8192, + memcpy: 1, + max_stack_height: 64*1024, opcodes_mul: 3, opcodes_div: 8, } diff --git a/ethcore/wasm/src/lib.rs b/ethcore/wasm/src/lib.rs index fcc2a669653..830a82f6c99 100644 --- a/ethcore/wasm/src/lib.rs +++ b/ethcore/wasm/src/lib.rs @@ -66,6 +66,12 @@ impl From for vm::Error { } } +enum ExecutionOutcome { + Suicide, + Return, + NotSpecial, +} + impl vm::Vm for WasmInterpreter { fn exec(&mut self, params: ActionParams, ext: &mut vm::Ext) -> vm::Result { @@ -117,31 +123,25 @@ impl vm::Vm for WasmInterpreter { let module_instance = module_instance.run_start(&mut runtime).map_err(Error)?; - match module_instance.invoke_export("call", &[], &mut runtime) { - Ok(_) => { }, - Err(InterpreterError::Host(boxed)) => { - match boxed.downcast_ref::() { - None => { - return Err(vm::Error::Wasm("Invalid user error used in interpreter".to_owned())); - } - Some(runtime_err) => { - match *runtime_err { - runtime::Error::Suicide => { - // Suicide uses trap to break execution - } - ref any_err => { - trace!(target: "wasm", "Error executing contract: {:?}", boxed); - return Err(vm::Error::from(Error::from(InterpreterError::Host(Box::new(any_err.clone()))))); - } - } - } - } - }, - Err(err) => { - trace!(target: "wasm", "Error executing contract: {:?}", err); - return Err(vm::Error::from(Error::from(err))) + let invoke_result = module_instance.invoke_export("call", &[], &mut runtime); + + let mut execution_outcome = ExecutionOutcome::NotSpecial; + if let Err(InterpreterError::Host(ref boxed)) = invoke_result { + let ref runtime_err = boxed.downcast_ref::() + .expect("Host errors other than runtime::Error never produced; qed"); + + match **runtime_err { + runtime::Error::Suicide => { execution_outcome = ExecutionOutcome::Suicide; }, + runtime::Error::Return => { execution_outcome = ExecutionOutcome::Return; }, + _ => {} } } + + if let (ExecutionOutcome::NotSpecial, Err(e)) = (execution_outcome, invoke_result) { + trace!(target: "wasm", "Error executing contract: {:?}", e); + return Err(vm::Error::from(Error::from(e))); + } + ( runtime.gas_left().expect("Cannot fail since it was not updated since last charge"), runtime.into_result(), diff --git a/ethcore/wasm/src/runtime.rs b/ethcore/wasm/src/runtime.rs index 0251c97eadb..a2b8d1bda21 100644 --- a/ethcore/wasm/src/runtime.rs +++ b/ethcore/wasm/src/runtime.rs @@ -52,6 +52,8 @@ pub enum Error { MemoryAccessViolation, /// Native code resulted in suicide Suicide, + /// Native code requested execution to finish + Return, /// Suicide was requested but coudn't complete SuicideAbort, /// Invalid gas state inside interpreter @@ -98,6 +100,7 @@ impl ::std::fmt::Display for Error { Error::InvalidGasState => write!(f, "Invalid gas state"), Error::BalanceQueryError => write!(f, "Balance query resulted in an error"), Error::Suicide => write!(f, "Suicide result"), + Error::Return => write!(f, "Return result"), Error::Unknown => write!(f, "Unknown runtime function invoked"), Error::AllocationFailed => write!(f, "Memory allocation failed (OOM)"), Error::BadUtf8 => write!(f, "String encoding is bad utf-8 sequence"), @@ -273,7 +276,7 @@ impl<'a> Runtime<'a> { self.result = self.memory.get(ptr, len as usize)?; - Ok(()) + Err(Error::Return) } /// Destroy the runtime, returning currently recorded result of the execution. @@ -305,6 +308,10 @@ impl<'a> Runtime<'a> { /// Write input bytes to the memory location using the passed pointer fn fetch_input(&mut self, args: RuntimeArgs) -> Result<()> { let ptr: u32 = args.nth(0)?; + + let args_len = self.args.len() as u64; + self.charge(|s| args_len * s.wasm().memcpy as u64)?; + self.memory.set(ptr, &self.args[..])?; Ok(()) } @@ -525,8 +532,8 @@ impl<'a> Runtime<'a> { fn debug(&mut self, args: RuntimeArgs) -> Result<()> { trace!(target: "wasm", "Contract debug message: {}", { - let msg_ptr: u32 = args.nth_checked(0)?; - let msg_len: u32 = args.nth_checked(1)?; + let msg_ptr: u32 = args.nth(0)?; + let msg_len: u32 = args.nth(1)?; String::from_utf8(self.memory.get(msg_ptr, msg_len as usize)?) .map_err(|_| Error::BadUtf8)? diff --git a/ethcore/wasm/src/tests.rs b/ethcore/wasm/src/tests.rs index 0083877c0ea..73288d7077c 100644 --- a/ethcore/wasm/src/tests.rs +++ b/ethcore/wasm/src/tests.rs @@ -209,7 +209,7 @@ fn dispersion() { result, vec![0u8, 0, 125, 11, 197, 7, 255, 8, 19, 0] ); - assert_eq!(gas_left, U256::from(93_972)); + assert_eq!(gas_left, U256::from(94_013)); } #[test] @@ -237,7 +237,7 @@ fn suicide_not() { result, vec![0u8] ); - assert_eq!(gas_left, U256::from(94_970)); + assert_eq!(gas_left, U256::from(94_984)); } #[test] @@ -269,7 +269,7 @@ fn suicide() { }; assert!(ext.suicides.contains(&refund)); - assert_eq!(gas_left, U256::from(94_933)); + assert_eq!(gas_left, U256::from(94_925)); } #[test] @@ -299,7 +299,7 @@ fn create() { assert!(ext.calls.contains( &FakeCall { call_type: FakeCallType::Create, - gas: U256::from(60_917), + gas: U256::from(60_914), sender_address: None, receive_address: None, value: Some(1_000_000_000.into()), @@ -307,7 +307,7 @@ fn create() { code_address: None, } )); - assert_eq!(gas_left, U256::from(60_903)); + assert_eq!(gas_left, U256::from(60_900)); } #[test] @@ -467,7 +467,7 @@ fn realloc() { } }; assert_eq!(result, vec![0u8; 2]); - assert_eq!(gas_left, U256::from(94_352)); + assert_eq!(gas_left, U256::from(94_372)); } #[test] @@ -543,7 +543,7 @@ fn keccak() { }; assert_eq!(H256::from_slice(&result), H256::from("68371d7e884c168ae2022c82bd837d51837718a7f7dfb7aa3f753074a35e1d87")); - assert_eq!(gas_left, U256::from(84_223)); + assert_eq!(gas_left, U256::from(84_240)); } // math_* tests check the ability of wasm contract to perform big integer operations @@ -572,7 +572,7 @@ fn math_add() { U256::from_dec_str("1888888888888888888888888888887").unwrap(), (&result[..]).into() ); - assert_eq!(gas_left, U256::from(93_818)); + assert_eq!(gas_left, U256::from(93_814)); } // multiplication @@ -594,7 +594,7 @@ fn math_mul() { U256::from_dec_str("888888888888888888888888888887111111111111111111111111111112").unwrap(), (&result[..]).into() ); - assert_eq!(gas_left, U256::from(93_304)); + assert_eq!(gas_left, U256::from(93_300)); } // subtraction @@ -616,7 +616,7 @@ fn math_sub() { U256::from_dec_str("111111111111111111111111111111").unwrap(), (&result[..]).into() ); - assert_eq!(gas_left, U256::from(93_831)); + assert_eq!(gas_left, U256::from(93_826)); } // subtraction with overflow @@ -658,7 +658,7 @@ fn math_div() { U256::from_dec_str("1125000").unwrap(), (&result[..]).into() ); - assert_eq!(gas_left, U256::from(90_607)); + assert_eq!(gas_left, U256::from(90_603)); } #[test] @@ -686,7 +686,7 @@ fn storage_metering() { }; // 0 -> not 0 - assert_eq!(gas_left, U256::from(74_410)); + assert_eq!(gas_left, U256::from(74_338)); // #2 @@ -705,7 +705,7 @@ fn storage_metering() { }; // not 0 -> not 0 - assert_eq!(gas_left, U256::from(89_410)); + assert_eq!(gas_left, U256::from(89_338)); } // This test checks the ability of wasm contract to invoke @@ -793,7 +793,7 @@ fn externs() { "Gas limit requested and returned does not match" ); - assert_eq!(gas_left, U256::from(92_089)); + assert_eq!(gas_left, U256::from(92_110)); } #[test] @@ -819,7 +819,7 @@ fn embedded_keccak() { }; assert_eq!(H256::from_slice(&result), H256::from("68371d7e884c168ae2022c82bd837d51837718a7f7dfb7aa3f753074a35e1d87")); - assert_eq!(gas_left, U256::from(84_223)); + assert_eq!(gas_left, U256::from(84_240)); } /// This test checks the correctness of log extern @@ -854,5 +854,5 @@ fn events() { assert_eq!(&log_entry.data, b"gnihtemos"); assert_eq!(&result, b"gnihtemos"); - assert_eq!(gas_left, U256::from(81_235)); + assert_eq!(gas_left, U256::from(81_292)); }