Refine sandbox errors (#860)

substrate/core/executor/src/sandbox.rs

@@ -181,21 +181,21 @@ pub struct GuestExternals<'a, FE: SandboxCapabilities + Externals + 'a> {
 	state: u32,
 }
 
-fn trap() -> Trap {
-	TrapKind::Host(Box::new(UserError("Sandbox error"))).into()
+fn trap(msg: &'static str) -> Trap {
+	TrapKind::Host(Box::new(UserError(msg))).into()
 }
 
 fn deserialize_result(serialized_result: &[u8]) -> Result<Option<RuntimeValue>, Trap> {
 	use self::sandbox_primitives::{HostError, ReturnValue};
 	let result_val = Result::<ReturnValue, HostError>::decode(&mut &serialized_result[..])
-		.ok_or_else(|| trap())?;
+		.ok_or_else(|| trap("Decoding Result<ReturnValue, HostError> failed!"))?;
 
 	match result_val {
 		Ok(return_value) => Ok(match return_value {
 			ReturnValue::Unit => None,
 			ReturnValue::Value(typed_value) => Some(RuntimeValue::from(typed_value)),
 		}),
-		Err(HostError) => Err(trap()),
+		Err(HostError) => Err(trap("Supervisor function returned sandbox::HostError")),
 	}
 }
 
@@ -257,7 +257,8 @@ impl<'a, FE: SandboxCapabilities + Externals + 'a> Externals for GuestExternals<
 				let len = (v & 0xFFFFFFFF) as u32;
 				(ptr, len)
 			}
-			_ => return Err(trap()),
+			Ok(_) => return Err(trap("Supervisor function returned unexpected result!")),
+			Err(_) => return Err(trap("Supervisor function trapped!")),
 		};
 
 		let serialized_result_val = self.supervisor_externals
@@ -474,7 +475,12 @@ impl Store {
 		};
 
 		let mem =
-			MemoryInstance::alloc(Pages(initial as usize), maximum).map_err(|_| UserError("Sandbox error"))?;
+			MemoryInstance::alloc(
+				Pages(initial as usize),
+				maximum,
+			)
+			.map_err(|_| UserError("Sandboxed memory allocation error"))?;
+
 		let mem_idx = self.memories.len();
 		self.memories.push(Some(mem));
 		Ok(mem_idx as u32)
@@ -490,8 +496,8 @@ impl Store {
 		self.instances
 			.get(instance_idx as usize)
 			.cloned()
-			.ok_or_else(|| UserError("Sandbox error"))?
-			.ok_or_else(|| UserError("Sandbox error"))
+			.ok_or_else(|| UserError("Trying to access a non-existent instance"))?
+			.ok_or_else(|| UserError("Trying to access a torndown instance"))
 	}
 
 	/// Returns reference to a memory instance by `memory_idx`.
@@ -499,35 +505,47 @@ impl Store {
 	/// # Errors
 	///
 	/// Returns `Err` If `memory_idx` isn't a valid index of an memory or
-	/// memory is already torndown.
+	/// if memory has been torn down.
 	pub fn memory(&self, memory_idx: u32) -> Result<MemoryRef, UserError> {
 		self.memories
 			.get(memory_idx as usize)
 			.cloned()
-			.ok_or_else(|| UserError("Sandbox error"))?
-			.ok_or_else(|| UserError("Sandbox error"))
+			.ok_or_else(|| UserError("Trying to access a non-existent sandboxed memory"))?
+			.ok_or_else(|| UserError("Trying to access a torndown sandboxed memory"))
 	}
 
-	/// Teardown the memory at the specified index.
+	/// Tear down the memory at the specified index.
 	///
 	/// # Errors
 	///
-	/// Returns `Err` if `memory_idx` isn't a valid index of an memory.
+	/// Returns `Err` if `memory_idx` isn't a valid index of an memory or
+	/// if it has been torn down.
 	pub fn memory_teardown(&mut self, memory_idx: u32) -> Result<(), UserError> {
-		if memory_idx as usize >= self.memories.len() {
-			return Err(UserError("Sandbox error"));
+		match self.memories.get_mut(memory_idx as usize) {
+			None => Err(UserError("Trying to teardown a non-existent sandboxed memory")),
+			Some(None) => Err(UserError("Double teardown of a sandboxed memory")),
+			Some(memory) => {
+				*memory = None;
+				Ok(())
+			}
 		}
-		self.memories[memory_idx as usize] = None;
-		Ok(())
 	}
 
-	/// Teardown the instance at the specified index.
+	/// Tear down the instance at the specified index.
+	///
+	/// # Errors
+	///
+	/// Returns `Err` if `instance_idx` isn't a valid index of an instance or
+	/// if it has been torn down.
 	pub fn instance_teardown(&mut self, instance_idx: u32) -> Result<(), UserError> {
-		if instance_idx as usize >= self.instances.len() {
-			return Err(UserError("Sandbox error"));
+		match self.instances.get_mut(instance_idx as usize) {
+			None => Err(UserError("Trying to teardown a non-existent instance")),
+			Some(None) => Err(UserError("Double teardown of an instance")),
+			Some(instance) => {
+				*instance = None;
+				Ok(())
+			}
 		}
-		self.instances[instance_idx as usize] = None;
-		Ok(())
 	}
 
 	fn register_sandbox_instance(&mut self, sandbox_instance: Rc<SandboxInstance>) -> u32 {

substrate/core/executor/src/wasm_executor.rs

@@ -352,15 +352,18 @@ impl_function_executor!(this: FunctionExecutor<'e, E>,
 		imports_len: usize,
 		state: usize
 	) -> u32 => {
-		let wasm = this.memory.get(wasm_ptr, wasm_len as usize).map_err(|_| UserError("Sandbox error"))?;
-		let raw_env_def = this.memory.get(imports_ptr, imports_len as usize).map_err(|_| UserError("Sandbox error"))?;
+		let wasm = this.memory.get(wasm_ptr, wasm_len as usize)
+			.map_err(|_| UserError("OOB while ext_sandbox_instantiate: wasm"))?;
+		let raw_env_def = this.memory.get(imports_ptr, imports_len as usize)
+			.map_err(|_| UserError("OOB while ext_sandbox_instantiate: imports"))?;
 
 		// Extract a dispatch thunk from instance's table by the specified index.
 		let dispatch_thunk = {
-			let table = this.table.as_ref().ok_or_else(|| UserError("Sandbox error"))?;
+			let table = this.table.as_ref()
+				.ok_or_else(|| UserError("Runtime doesn't have a table; sandbox is unavailable"))?;
 			table.get(dispatch_thunk_idx)
-				.map_err(|_| UserError("Sandbox error"))?
-				.ok_or_else(|| UserError("Sandbox error"))?
+				.map_err(|_| UserError("dispatch_thunk_idx is out of the table bounds"))?
+				.ok_or_else(|| UserError("dispatch_thunk_idx points on an empty table entry"))?
 				.clone()
 		};
 
@@ -382,17 +385,17 @@ impl_function_executor!(this: FunctionExecutor<'e, E>,
 
 		trace!(target: "sr-sandbox", "invoke, instance_idx={}", instance_idx);
 		let export = this.memory.get(export_ptr, export_len as usize)
-			.map_err(|_| UserError("Sandbox error"))
+			.map_err(|_| UserError("OOB while ext_sandbox_invoke: export"))
 			.and_then(|b|
 				String::from_utf8(b)
-					.map_err(|_| UserError("Sandbox error"))
+					.map_err(|_| UserError("export name should be a valid utf-8 sequence"))
 			)?;
 
 		// Deserialize arguments and convert them into wasmi types.
 		let serialized_args = this.memory.get(args_ptr, args_len as usize)
-			.map_err(|_| UserError("Sandbox error"))?;
+			.map_err(|_| UserError("OOB while ext_sandbox_invoke: args"))?;
 		let args = Vec::<sandbox_primitives::TypedValue>::decode(&mut &serialized_args[..])
-			.ok_or_else(|| UserError("Sandbox error"))?
+			.ok_or_else(|| UserError("Can't decode serialized arguments for the invocation"))?
 			.into_iter()
 			.map(Into::into)
 			.collect::<Vec<_>>();
@@ -406,11 +409,11 @@ impl_function_executor!(this: FunctionExecutor<'e, E>,
 				// Serialize return value and write it back into the memory.
 				sandbox_primitives::ReturnValue::Value(val.into()).using_encoded(|val| {
 					if val.len() > return_val_len as usize {
-						Err(UserError("Sandbox error"))?;
+						Err(UserError("Return value buffer is too small"))?;
 					}
 					this.memory
 						.set(return_val_ptr, val)
-						.map_err(|_| UserError("Sandbox error"))?;
+						.map_err(|_| UserError("Return value buffer is OOB"))?;
 					Ok(sandbox_primitives::ERR_OK)
 				})
 			}