From c92344272b9638c094d841215e3e2b17cbc66f66 Mon Sep 17 00:00:00 2001 From: vms Date: Thu, 17 Sep 2020 20:12:53 +0300 Subject: [PATCH] add check memory border while reading/writing memory; improve parsing wat --- src/decoders/wat.rs | 32 +++++++++----- src/encoders/wat.rs | 6 +-- src/interpreter/instructions/records.rs | 18 ++++++-- src/interpreter/instructions/records/utils.rs | 44 +++++++++++++++---- 4 files changed, 74 insertions(+), 26 deletions(-) diff --git a/src/decoders/wat.rs b/src/decoders/wat.rs index 876b222..2cffa78 100644 --- a/src/decoders/wat.rs +++ b/src/decoders/wat.rs @@ -158,22 +158,34 @@ impl Parse<'_> for RecordType { fn parse(parser: Parser<'_>) -> Result { parser.parse::()?; - parser.parse::()?; - let record_name = parser.parse()?; + let record_name = parser + .step(|cursor| { + cursor + .id() + .ok_or_else(|| cursor.error("expecting argument identifier")) + })? + .to_string(); let mut fields = vec![]; - while !parser.is_empty() { - fields.push(parser.parens(|parser| { - parser.parse::()?; - let name = parser.parse()?; - + parser.parens(|parser| { + while !parser.is_empty() { parser.parse::()?; + + let name = parser + .step(|cursor| { + cursor + .id() + .ok_or_else(|| cursor.error("expecting argument identifier")) + })? + .to_string(); + let ty = parser.parse()?; - Ok(RecordFieldType { name, ty }) - })?); - } + fields.push(RecordFieldType { name, ty }); + } + Ok(()) + })?; Ok(RecordType { name: record_name, diff --git a/src/encoders/wat.rs b/src/encoders/wat.rs index 9c3d5a4..65d3ff0 100644 --- a/src/encoders/wat.rs +++ b/src/encoders/wat.rs @@ -77,7 +77,7 @@ impl ToString for &InterfaceType { InterfaceType::Anyref => "anyref".to_string(), InterfaceType::I32 => "i32".to_string(), InterfaceType::I64 => "i64".to_string(), - InterfaceType::Record(record_type) => record_type.to_string(), + InterfaceType::Record(record_type_id) => format!("record {}", record_type_id), } } } @@ -85,7 +85,7 @@ impl ToString for &InterfaceType { impl ToString for &RecordType { fn to_string(&self) -> String { format!( - "record {} {{\n{fields}}}", + "record ${} {{\n{fields}}}", self.name, fields = self .fields @@ -93,7 +93,7 @@ impl ToString for &RecordType { .fold(String::new(), |mut accumulator, field_type| { accumulator.push(' '); accumulator.push_str(&format!( - "{}: {}\n", + "field ${}: {}\n", field_type.name, (&field_type.ty).to_string() )); diff --git a/src/interpreter/instructions/records.rs b/src/interpreter/instructions/records.rs index 8f6a301..c32d85e 100644 --- a/src/interpreter/instructions/records.rs +++ b/src/interpreter/instructions/records.rs @@ -284,6 +284,12 @@ where ) })?; + log::trace!( + "record.lift_memory: record {:?} resolved for type id {}", + record_type, + record_type_id + ); + let record = record_lift_memory_(&**instance, record_type, offset, instruction)?; log::trace!("record.lift_memory: pushing {:?} on the stack", record); @@ -327,7 +333,7 @@ where let string_pointer = if !value.is_empty() { write_to_instance_mem(instance, instruction, value.as_bytes())? } else { - 0i32 + 0 }; result.push(string_pointer as _); @@ -338,7 +344,7 @@ where let byte_array_pointer = if !value.is_empty() { write_to_instance_mem(instance, instruction, &value)? } else { - 0i32 + 0 }; result.push(byte_array_pointer as _); @@ -348,7 +354,6 @@ where InterfaceValue::Record(values) => { let record_ptr = record_lower_memory_(instance, instruction, values)?; - log::trace!("record.lower_memory: pushing {:?} on the stack", record_ptr); result.push(record_ptr as _); } } @@ -357,7 +362,7 @@ where let result = safe_transmute::transmute_to_bytes::(&result); let result_pointer = write_to_instance_mem(instance, instruction, &result)?; - Ok(result_pointer) + Ok(result_pointer as _) } pub(crate) fn record_lower_memory( @@ -386,7 +391,12 @@ where &record_fields, instruction, )?; + + log::trace!("record.lower_memory: obtained {:?} values on the stack for record type = {}", record_fields, record_type_id); + let offset = record_lower_memory_(*instance, instruction, record_fields)?; + + log::trace!("record.lower_memory: pushing {:?} on the stack", offset); runtime.stack.push(InterfaceValue::I32(offset)); Ok(()) diff --git a/src/interpreter/instructions/records/utils.rs b/src/interpreter/instructions/records/utils.rs index 99a9f30..3c59a6c 100644 --- a/src/interpreter/instructions/records/utils.rs +++ b/src/interpreter/instructions/records/utils.rs @@ -35,6 +35,19 @@ where })? .view(); + log::info!("reading {} bytes from offset {}", size, offset); + + let right = offset + size; + if right < offset || right >= memory_view.len() { + return Err(InstructionError::new( + instruction, + InstructionErrorKind::MemoryOutOfBoundsAccess { + index: right, + length: memory_view.len(), + }, + )); + } + Ok((&memory_view[offset..offset + size]) .iter() .map(std::cell::Cell::get) @@ -45,7 +58,7 @@ pub(super) fn write_to_instance_mem<'instance, Instance, Export, LocalImport, Me instance: &'instance Instance, instruction: Instruction, bytes: &[u8], -) -> Result +) -> Result where Export: wasm::structures::Export + 'instance, LocalImport: wasm::structures::LocalImport + 'instance, @@ -53,7 +66,7 @@ where MemoryView: wasm::structures::MemoryView, Instance: wasm::structures::Instance, { - let mem_pointer = allocate(instance, instruction, bytes.len() as _)?; + let offset = allocate(instance, instruction, bytes.len() as _)?; let memory_index: u32 = 0; let memory_view = instance @@ -66,18 +79,31 @@ where })? .view(); - for (byte_id, byte) in bytes.iter().enumerate() { - memory_view[mem_pointer as usize + byte_id].set(*byte); + log::info!("writing {} bytes from offset {}", bytes.len(), offset); + + let right = offset + bytes.len(); + if right < offset || right >= memory_view.len() { + return Err(InstructionError::new( + instruction, + InstructionErrorKind::MemoryOutOfBoundsAccess { + index: right, + length: memory_view.len(), + }, + )); } - Ok(mem_pointer) + for (byte_id, byte) in bytes.iter().enumerate() { + memory_view[offset + byte_id].set(*byte); + } + + Ok(offset) } pub(super) fn allocate<'instance, Instance, Export, LocalImport, Memory, MemoryView>( instance: &'instance Instance, instruction: Instruction, - size: i32, -) -> Result + size: usize, +) -> Result where Export: wasm::structures::Export + 'instance, LocalImport: wasm::structures::LocalImport + 'instance, @@ -89,7 +115,7 @@ where instance, ALLOCATE_FUNC_INDEX, instruction, - vec![InterfaceValue::I32(size)], + vec![InterfaceValue::I32(size as _)], )?; if values.len() != 1 { return Err(InstructionError::new( @@ -101,7 +127,7 @@ where }, )); } - to_native::(&values[0], instruction) + to_native::(&values[0], instruction).map(|v| v as usize) } pub(super) fn deallocate<'instance, Instance, Export, LocalImport, Memory, MemoryView>(