From fb59f4d91e8b95daa82e554cc0815182167bf21d Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Mon, 13 Jul 2020 15:15:38 +0200 Subject: [PATCH 1/2] Add validate_region --- packages/vm/src/errors.rs | 56 ++++++++++++++++ packages/vm/src/memory.rs | 137 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 193 insertions(+) diff --git a/packages/vm/src/errors.rs b/packages/vm/src/errors.rs index 48f483cfb5..8ce9abaafb 100644 --- a/packages/vm/src/errors.rs +++ b/packages/vm/src/errors.rs @@ -245,6 +245,26 @@ pub enum CommunicationError { max_length: usize, backtrace: snafu::Backtrace, }, + #[snafu(display( + "Region length exceeds capacity. Length {}, capacity {}", + length, + capacity + ))] + RegionLengthExceedsCapacity { + length: u32, + capacity: u32, + backtrace: snafu::Backtrace, + }, + #[snafu(display( + "Region exceeds address space. Offset {}, capacity {}", + offset, + capacity + ))] + RegionOutOfRange { + offset: u32, + capacity: u32, + backtrace: snafu::Backtrace, + }, #[snafu(display("Region too small. Got {}, required {}", size, required))] RegionTooSmall { size: usize, @@ -281,6 +301,14 @@ impl CommunicationError { RegionLengthTooBig { length, max_length }.build() } + pub(crate) fn region_length_exceeds_capacity(length: u32, capacity: u32) -> Self { + RegionLengthExceedsCapacity { length, capacity }.build() + } + + pub(crate) fn region_out_of_range(offset: u32, capacity: u32) -> Self { + RegionOutOfRange { offset, capacity }.build() + } + pub(crate) fn region_too_small(size: usize, required: usize) -> Self { RegionTooSmall { size, required }.build() } @@ -547,6 +575,34 @@ mod test { } } + #[test] + fn communication_error_region_length_exceeds_capacity_works() { + let error = CommunicationError::region_length_exceeds_capacity(50, 20); + match error { + CommunicationError::RegionLengthExceedsCapacity { + length, capacity, .. + } => { + assert_eq!(length, 50); + assert_eq!(capacity, 20); + } + e => panic!("Unexpected error: {:?}", e), + } + } + + #[test] + fn communication_error_region_out_of_range_works() { + let error = CommunicationError::region_out_of_range(u32::MAX, 1); + match error { + CommunicationError::RegionOutOfRange { + offset, capacity, .. + } => { + assert_eq!(offset, u32::MAX); + assert_eq!(capacity, 1); + } + e => panic!("Unexpected error: {:?}", e), + } + } + #[test] fn communication_error_region_too_small_works() { let error = CommunicationError::region_too_small(12, 33); diff --git a/packages/vm/src/memory.rs b/packages/vm/src/memory.rs index 8413680241..0b51de30e4 100644 --- a/packages/vm/src/memory.rs +++ b/packages/vm/src/memory.rs @@ -145,6 +145,27 @@ fn get_region(ctx: &Ctx, ptr: u32) -> CommunicationResult { } } +/// Performs plausibility checks in the given Region. Regions are always created by the +/// contract and this can be used to detect problems in the standard library of the contract. +fn validate_region(region: &Region) -> CommunicationResult<()> { + if region.offset == 0 { + return Err(CommunicationError::zero_address()); + } + if region.length > region.capacity { + return Err(CommunicationError::region_length_exceeds_capacity( + region.length, + region.capacity, + )); + } + if region.capacity > (u32::MAX - region.offset) { + return Err(CommunicationError::region_out_of_range( + region.offset, + region.capacity, + )); + } + Ok(()) +} + /// Overrides a Region at ptr in wasm memory with data fn set_region(ctx: &Ctx, ptr: u32, data: Region) -> CommunicationResult<()> { let memory = ctx.memory(0); @@ -161,3 +182,119 @@ fn set_region(ctx: &Ctx, ptr: u32, data: Region) -> CommunicationResult<()> { )), } } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn validate_region_passes_for_valid_region() { + // empty + let region = Region { + offset: 23, + capacity: 500, + length: 0, + }; + validate_region(®ion).unwrap(); + + // half full + let region = Region { + offset: 23, + capacity: 500, + length: 250, + }; + validate_region(®ion).unwrap(); + + // full + let region = Region { + offset: 23, + capacity: 500, + length: 500, + }; + validate_region(®ion).unwrap(); + + // at end of linear memory (1) + let region = Region { + offset: u32::MAX, + capacity: 0, + length: 0, + }; + validate_region(®ion).unwrap(); + + // at end of linear memory (2) + let region = Region { + offset: 1, + capacity: u32::MAX - 1, + length: 0, + }; + validate_region(®ion).unwrap(); + } + + #[test] + fn validate_region_fails_for_zero_offset() { + let region = Region { + offset: 0, + capacity: 500, + length: 250, + }; + let result = validate_region(®ion); + match result.unwrap_err() { + CommunicationError::ZeroAddress { .. } => {} + e => panic!("Got unexpected error: {:?}", e), + } + } + + #[test] + fn validate_region_fails_for_length_exceeding_capacity() { + let region = Region { + offset: 23, + capacity: 500, + length: 501, + }; + let result = validate_region(®ion); + match result.unwrap_err() { + CommunicationError::RegionLengthExceedsCapacity { + length, capacity, .. + } => { + assert_eq!(length, 501); + assert_eq!(capacity, 500); + } + e => panic!("Got unexpected error: {:?}", e), + } + } + + #[test] + fn validate_region_fails_when_exceeding_address_space() { + let region = Region { + offset: 23, + capacity: u32::MAX, + length: 501, + }; + let result = validate_region(®ion); + match result.unwrap_err() { + CommunicationError::RegionOutOfRange { + offset, capacity, .. + } => { + assert_eq!(offset, 23); + assert_eq!(capacity, u32::MAX); + } + e => panic!("Got unexpected error: {:?}", e), + } + + let region = Region { + offset: u32::MAX, + capacity: 1, + length: 0, + }; + let result = validate_region(®ion); + match result.unwrap_err() { + CommunicationError::RegionOutOfRange { + offset, capacity, .. + } => { + assert_eq!(offset, u32::MAX); + assert_eq!(capacity, 1); + } + e => panic!("Got unexpected error: {:?}", e), + } + } +} From 2d6cbad0cf32443c842cae0d2190efcc1090aa1c Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Tue, 14 Jul 2020 06:44:47 +0200 Subject: [PATCH 2/2] Use validate_region in get_region --- CHANGELOG.md | 3 +++ packages/vm/src/memory.rs | 6 +++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a46429d4b0..9014f677ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,9 @@ value. - Increase the max. supported value for gas limit from 10_000_000_000 to 0x7FFFFFFFFFFFFFFF. +- Add checks to `get_region` for failing early when the contract sends a Region + pointer to the VM that is not backed by a plausible Region. This helps + development of standard libraries. ## 0.9.3 (2020-07-08) diff --git a/packages/vm/src/memory.rs b/packages/vm/src/memory.rs index 0b51de30e4..90f2c7cf04 100644 --- a/packages/vm/src/memory.rs +++ b/packages/vm/src/memory.rs @@ -137,7 +137,11 @@ fn get_region(ctx: &Ctx, ptr: u32) -> CommunicationResult { let memory = ctx.memory(0); let wptr = WasmPtr::::new(ptr); match wptr.deref(memory) { - Some(cell) => Ok(cell.get()), + Some(cell) => { + let region = cell.get(); + validate_region(®ion)?; + Ok(region) + } None => Err(CommunicationError::deref_err( ptr, "Could not dereference this pointer to a Region",