From 1bad20ae38457314fb5441f3b19c8b4e63b96230 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 | 3 +++ ethcore/wasm/src/lib.rs | 22 +++++++++++++++------- ethcore/wasm/src/runtime.rs | 9 ++++++++- ethcore/wasm/src/tests.rs | 32 ++++++++++++++++---------------- 4 files changed, 42 insertions(+), 24 deletions(-) diff --git a/ethcore/vm/src/schedule.rs b/ethcore/vm/src/schedule.rs index c64f6159f..6cfabed0d 100644 --- a/ethcore/vm/src/schedule.rs +++ b/ethcore/vm/src/schedule.rs @@ -135,6 +135,8 @@ 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` @@ -154,6 +156,7 @@ 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 d6082e018..10e14caeb 100644 --- a/ethcore/wasm/src/lib.rs +++ b/ethcore/wasm/src/lib.rs @@ -77,6 +77,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 { @@ -130,21 +136,23 @@ impl vm::Vm for WasmInterpreter { let invoke_result = module_instance.invoke_export("call", &[], &mut runtime); - let mut suicide = false; + let mut execution_outcome = ExecutionOutcome::NotSpecial; if let Err(InterpreterError::Trap(ref trap)) = invoke_result { if let wasmi::TrapKind::Host(ref boxed) = *trap.kind() { let ref runtime_err = boxed.downcast_ref::() .expect("Host errors other than runtime::Error never produced; qed"); - if let runtime::Error::Suicide = **runtime_err { suicide = true; } + match **runtime_err { + runtime::Error::Suicide => { execution_outcome = ExecutionOutcome::Suicide; }, + runtime::Error::Return => { execution_outcome = ExecutionOutcome::Return; }, + _ => {} + } } } - if !suicide { - if let Err(e) = invoke_result { - trace!(target: "wasm", "Error executing contract: {:?}", e); - return Err(vm::Error::from(Error::from(e))); - } + 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))); } ( diff --git a/ethcore/wasm/src/runtime.rs b/ethcore/wasm/src/runtime.rs index 4a2cf7d12..bc639bea1 100644 --- a/ethcore/wasm/src/runtime.rs +++ b/ethcore/wasm/src/runtime.rs @@ -32,6 +32,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 @@ -103,6 +105,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"), @@ -289,7 +292,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 @@ -321,6 +324,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_checked(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(()) } diff --git a/ethcore/wasm/src/tests.rs b/ethcore/wasm/src/tests.rs index af2bbe367..2b71a1768 100644 --- a/ethcore/wasm/src/tests.rs +++ b/ethcore/wasm/src/tests.rs @@ -207,7 +207,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] @@ -235,7 +235,7 @@ fn suicide_not() { result, vec![0u8] ); - assert_eq!(gas_left, U256::from(94_970)); + assert_eq!(gas_left, U256::from(94_984)); } #[test] @@ -267,7 +267,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] @@ -297,7 +297,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()), @@ -305,7 +305,7 @@ fn create() { code_address: None, } )); - assert_eq!(gas_left, U256::from(60_903)); + assert_eq!(gas_left, U256::from(60_900)); } #[test] @@ -465,7 +465,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] @@ -541,7 +541,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 @@ -570,7 +570,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 @@ -592,7 +592,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 @@ -614,7 +614,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 @@ -656,7 +656,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] @@ -684,7 +684,7 @@ fn storage_metering() { }; // 0 -> not 0 - assert_eq!(gas_left, U256::from(74_410)); + assert_eq!(gas_left, U256::from(74_338)); // #2 @@ -703,7 +703,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 @@ -791,7 +791,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] @@ -817,7 +817,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 @@ -852,5 +852,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)); }