diff --git a/CHANGELOG.md b/CHANGELOG.md index 0180f0bc3..16bbacbf9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,23 +13,29 @@ and this project adheres to - cosmwasm-std: Implement `From for u{64,128}`, `From for u128`, `From for i{64,128}`, and `From for i128` ([#2268]) -- cosmwasm-std: Deprecate `abort` feature. The panic handler is now always - enabled. ([#2337]) - cosmwasm-std: Implement `Uint128::from_{be,le}_bytes` and `Uint64::from_{be,le}_bytes`. ([#2269]) +- cosmwasm-std: Added new `EurekaMsg` and `CosmosMsg::Eureka` variant ([#2340]) -[#2268]: https://github.com/CosmWasm/cosmwasm/issues/2268 -[#2337]: https://github.com/CosmWasm/cosmwasm/issues/2337 -[#2269]: https://github.com/CosmWasm/cosmwasm/issues/2269 +## Changed + +- cosmwasm-std: Deprecate `abort` feature. The panic handler is now always + enabled. ([#2337]) +- cosmwasm-std: Document safety invariants of the internal memory repr ([#2344]) +- cosmwasm-std: Enforce non-null pointers using `ptr::NonNull` in the internal + memory repr ([#2344]) ## Fixed - cosmwasm-schema: The schema export now doesn't overwrite existing `additionalProperties` values anymore ([#2310]) -- cosmwasm-std: Added new `EurekaMsg` and `CosmosMsg::Eureka` variant ([#2340]) +[#2268]: https://github.com/CosmWasm/cosmwasm/issues/2268 +[#2269]: https://github.com/CosmWasm/cosmwasm/issues/2269 [#2310]: https://github.com/CosmWasm/cosmwasm/pull/2310 +[#2337]: https://github.com/CosmWasm/cosmwasm/issues/2337 [#2340]: https://github.com/CosmWasm/cosmwasm/pull/2340 +[#2344]: https://github.com/CosmWasm/cosmwasm/pull/2344 ## [2.2.0] - 2024-12-17 diff --git a/packages/std/src/exports.rs b/packages/std/src/exports.rs index 17e2cf22d..76d9a4367 100644 --- a/packages/std/src/exports.rs +++ b/packages/std/src/exports.rs @@ -8,7 +8,7 @@ //! the contract-specific function pointer. This is done via the `#[entry_point]` //! macro attribute from cosmwasm-derive. use alloc::vec::Vec; -use core::marker::PhantomData; +use core::{marker::PhantomData, ptr}; use serde::de::DeserializeOwned; @@ -95,7 +95,8 @@ extern "C" fn allocate(size: usize) -> u32 { #[no_mangle] extern "C" fn deallocate(pointer: u32) { // auto-drop Region on function end - let _ = unsafe { Region::from_heap_ptr(pointer as *mut Region) }; + let _ = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(pointer as *mut Region).unwrap()) }; } // TODO: replace with https://doc.rust-lang.org/std/ops/trait.Try.html once stabilized @@ -533,9 +534,12 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; - let info: Vec = unsafe { Region::from_heap_ptr(info_ptr).into_vec() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; + let env: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() }; + let info: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(info_ptr).unwrap()).into_vec() }; + let msg: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(msg_ptr).unwrap()).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let info: MessageInfo = try_into_contract_result!(from_json(info)); @@ -557,9 +561,12 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; - let info: Vec = unsafe { Region::from_heap_ptr(info_ptr).into_vec() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; + let env: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() }; + let info: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(info_ptr).unwrap()).into_vec() }; + let msg: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(msg_ptr).unwrap()).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let info: MessageInfo = try_into_contract_result!(from_json(info)); @@ -580,8 +587,10 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; + let env: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() }; + let msg: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(msg_ptr).unwrap()).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: M = try_into_contract_result!(from_json(msg)); @@ -602,9 +611,12 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; - let migrate_info = unsafe { Region::from_heap_ptr(migrate_info_ptr).into_vec() }; + let env: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() }; + let msg: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(msg_ptr).unwrap()).into_vec() }; + let migrate_info = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(migrate_info_ptr).unwrap()).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: M = try_into_contract_result!(from_json(msg)); @@ -625,8 +637,10 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; + let env: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() }; + let msg: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(msg_ptr).unwrap()).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: M = try_into_contract_result!(from_json(msg)); @@ -645,8 +659,10 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; + let env: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() }; + let msg: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(msg_ptr).unwrap()).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: Reply = try_into_contract_result!(from_json(msg)); @@ -665,8 +681,10 @@ where M: DeserializeOwned, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; + let env: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() }; + let msg: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(msg_ptr).unwrap()).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: M = try_into_contract_result!(from_json(msg)); @@ -684,8 +702,10 @@ where Q: CustomQuery, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; + let env: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() }; + let msg: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(msg_ptr).unwrap()).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: IbcChannelOpenMsg = try_into_contract_result!(from_json(msg)); @@ -705,8 +725,10 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; + let env: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() }; + let msg: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(msg_ptr).unwrap()).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: IbcChannelConnectMsg = try_into_contract_result!(from_json(msg)); @@ -726,8 +748,10 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; + let env: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() }; + let msg: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(msg_ptr).unwrap()).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: IbcChannelCloseMsg = try_into_contract_result!(from_json(msg)); @@ -747,8 +771,10 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; + let env: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() }; + let msg: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(msg_ptr).unwrap()).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: IbcPacketReceiveMsg = try_into_contract_result!(from_json(msg)); @@ -768,8 +794,10 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; + let env: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() }; + let msg: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(msg_ptr).unwrap()).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: IbcPacketAckMsg = try_into_contract_result!(from_json(msg)); @@ -789,8 +817,10 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; + let env: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() }; + let msg: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(msg_ptr).unwrap()).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: IbcPacketTimeoutMsg = try_into_contract_result!(from_json(msg)); @@ -809,8 +839,10 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; + let env: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() }; + let msg: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(msg_ptr).unwrap()).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: IbcSourceCallbackMsg = try_into_contract_result!(from_json(msg)); @@ -833,8 +865,10 @@ where C: CustomMsg, E: ToString, { - let env: Vec = unsafe { Region::from_heap_ptr(env_ptr).into_vec() }; - let msg: Vec = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() }; + let env: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() }; + let msg: Vec = + unsafe { Region::from_heap_ptr(ptr::NonNull::new(msg_ptr).unwrap()).into_vec() }; let env: Env = try_into_contract_result!(from_json(env)); let msg: IbcDestinationCallbackMsg = try_into_contract_result!(from_json(msg)); diff --git a/packages/std/src/imports.rs b/packages/std/src/imports.rs index 43a87f656..b31ed76f6 100644 --- a/packages/std/src/imports.rs +++ b/packages/std/src/imports.rs @@ -1,4 +1,5 @@ use alloc::vec::Vec; +use core::ptr; use crate::import_helpers::{from_high_half, from_low_half}; use crate::memory::{Owned, Region}; @@ -133,7 +134,7 @@ impl Storage for ExternalStorage { } let value_ptr = read as *mut Region; - let data = unsafe { Region::from_heap_ptr(value_ptr) }; + let data = unsafe { Region::from_heap_ptr(ptr::NonNull::new(value_ptr).unwrap()) }; Some(data.into_vec()) } @@ -255,7 +256,7 @@ impl Iterator for ExternalPartialIterator { } let data_region = next_result as *mut Region; - let data = unsafe { Region::from_heap_ptr(data_region) }; + let data = unsafe { Region::from_heap_ptr(ptr::NonNull::new(data_region).unwrap()) }; Some(data.into_vec()) } @@ -284,7 +285,7 @@ impl Iterator for ExternalIterator { fn next(&mut self) -> Option { let next_result = unsafe { db_next(self.iterator_id) }; let kv_region_ptr = next_result as *mut Region; - let kv = unsafe { Region::from_heap_ptr(kv_region_ptr) }; + let kv = unsafe { Region::from_heap_ptr(ptr::NonNull::new(kv_region_ptr).unwrap()) }; let (key, value) = decode_sections2(kv.into_vec()); @@ -307,7 +308,7 @@ fn skip_iter(iter_id: u32, count: usize) { } // just deallocate the region - unsafe { Region::from_heap_ptr(region as *mut Region) }; + unsafe { Region::from_heap_ptr(ptr::NonNull::new(region as *mut Region).unwrap()) }; } } @@ -574,8 +575,12 @@ impl Api for ExternalApi { let pubkey_ptr = from_low_half(result); match error_code { 0 => { - let pubkey = - unsafe { Region::from_heap_ptr(pubkey_ptr as *mut Region).into_vec() }; + let pubkey = unsafe { + Region::from_heap_ptr( + ptr::NonNull::new(pubkey_ptr as *mut Region).unwrap(), + ) + .into_vec() + }; Ok(pubkey) } 2 => panic!("MessageTooLong must not happen. This is a bug in the VM."), @@ -631,8 +636,12 @@ impl Api for ExternalApi { let pubkey_ptr = from_low_half(result); match error_code { 0 => { - let pubkey = - unsafe { Region::from_heap_ptr(pubkey_ptr as *mut Region).into_vec() }; + let pubkey = unsafe { + Region::from_heap_ptr( + ptr::NonNull::new(pubkey_ptr as *mut Region).unwrap(), + ) + .into_vec() + }; Ok(pubkey) } 2 => panic!("MessageTooLong must not happen. This is a bug in the VM."), @@ -712,7 +721,7 @@ impl Api for ExternalApi { /// Takes a pointer to a Region and reads the data into a String. /// This is for trusted string sources only. unsafe fn consume_string_region_written_by_vm(from: *mut Region) -> String { - let data = Region::from_heap_ptr(from).into_vec(); + let data = Region::from_heap_ptr(ptr::NonNull::new(from).unwrap()).into_vec(); // We trust the VM/chain to return correct UTF-8, so let's save some gas String::from_utf8_unchecked(data) } @@ -732,8 +741,10 @@ impl Querier for ExternalQuerier { let request_ptr = req.as_ptr() as u32; let response_ptr = unsafe { query_chain(request_ptr) }; - let response = - unsafe { Region::from_heap_ptr(response_ptr as *mut Region).into_vec() }; + let response = unsafe { + Region::from_heap_ptr(ptr::NonNull::new(response_ptr as *mut Region).unwrap()) + .into_vec() + }; from_json(&response).unwrap_or_else(|parsing_err| { SystemResult::Err(SystemError::InvalidResponse { diff --git a/packages/std/src/memory.rs b/packages/std/src/memory.rs index f25da1566..d0a3bf90d 100644 --- a/packages/std/src/memory.rs +++ b/packages/std/src/memory.rs @@ -1,5 +1,5 @@ use alloc::vec::Vec; -use core::{any::TypeId, marker::PhantomData, mem, ops::Deref, slice}; +use core::{any::TypeId, marker::PhantomData, mem, ops::Deref, ptr, slice}; /// This trait is used to indicate whether a region is borrowed or owned pub trait Ownership: 'static {} @@ -37,14 +37,23 @@ const _: () = { impl Region { pub fn from_slice(slice: &[u8]) -> Self { - unsafe { Self::from_parts(slice.as_ptr(), slice.len(), slice.len()) } + // SAFETY: A slice upholds all the safety variants we need to construct a borrowed Region + unsafe { + let ptr: ptr::NonNull = ptr::NonNull::from(slice).cast(); + Self::from_parts(ptr, slice.len(), slice.len()) + } } } impl Region { /// Construct a region from an existing vector - pub fn from_vec(vec: Vec) -> Self { - let region = unsafe { Self::from_parts(vec.as_ptr(), vec.capacity(), vec.len()) }; + pub fn from_vec(mut vec: Vec) -> Self { + // SAFETY: The `std::vec::Vec` type upholds all the safety invariants required to call `from_parts` + let region = unsafe { + let ptr = ptr::NonNull::new_unchecked(vec.as_mut_ptr()); + Self::from_parts(ptr, vec.capacity(), vec.len()) + }; + // Important and load bearing: call `mem::forget` to prevent memory from being freed mem::forget(vec); region } @@ -54,13 +63,11 @@ impl Region { /// /// # Safety /// - /// - The pointer must not be null /// - The pointer must be heap allocated /// - This region must point to a valid memory region /// - The memory region this region points to must be heap allocated as well - pub unsafe fn from_heap_ptr(ptr: *mut Self) -> Box { - assert!(!ptr.is_null(), "Region pointer is null"); - Box::from_raw(ptr) + pub unsafe fn from_heap_ptr(ptr: ptr::NonNull) -> Box { + Box::from_raw(ptr.as_ptr()) } /// Construct a new empty region with *at least* a capacity of what you passed in and a length of 0 @@ -72,6 +79,7 @@ impl Region { /// Transform the region into a vector pub fn into_vec(self) -> Vec { + // SAFETY: Invariants are covered by the safety contract of the constructor let vector = unsafe { Vec::from_raw_parts( self.offset as *mut u8, @@ -88,11 +96,18 @@ impl Region where O: Ownership, { - unsafe fn from_parts(ptr: *const u8, capacity: usize, length: usize) -> Self { + /// # Safety + /// + /// This function requires the following invariants to be upheld: + /// - `length` is smaller or equal to `capacity` + /// - The number of bytes allocated by the pointer must be equal to `capacity` (if the `Ownership` is `Owned`) + /// - The byte range covered by `length` must be initialized + /// - If the generic `Ownership` parameter is set to `Owned`, the `ptr` must point to a memory region allocated by a `Vec` + unsafe fn from_parts(ptr: ptr::NonNull, capacity: usize, length: usize) -> Self { // Well, this technically violates pointer provenance rules. // But there isn't a stable API for it, so that's the best we can do, I guess. Region { - offset: u32::try_from(ptr as usize).expect("pointer doesn't fit in u32"), + offset: u32::try_from(ptr.as_ptr() as usize).expect("pointer doesn't fit in u32"), capacity: u32::try_from(capacity).expect("capacity doesn't fit in u32"), length: u32::try_from(length).expect("length doesn't fit in u32"), @@ -102,6 +117,7 @@ where /// Access the memory region this region points to in form of a byte slice pub fn as_bytes(&self) -> &[u8] { + // SAFETY: Safety contract of constructor requires `length` bytes to be initialized unsafe { slice::from_raw_parts(self.offset as *const u8, self.length as usize) } } @@ -140,15 +156,16 @@ where fn drop(&mut self) { // Since we can't specialize the drop impl we need to perform a runtime check if TypeId::of::() == TypeId::of::() { - let region_start = self.offset as *mut u8; - - // This case is explicitly disallowed by Vec - // "The pointer will never be null, so this type is null-pointer-optimized." - assert!(!region_start.is_null(), "Region starts at null pointer"); + let region_start = ptr::NonNull::new(self.offset as *mut u8).unwrap(); + // SAFETY: Since `from_parts` was required to uphold the invariant that if the parameter is `Owned` + // the memory has been allocated through a `Vec`, we can safely reconstruct the `Vec` and deallocate it. unsafe { - let data = - Vec::from_raw_parts(region_start, self.length as usize, self.capacity as usize); + let data = Vec::from_raw_parts( + region_start.as_ptr(), + self.length as usize, + self.capacity as usize, + ); drop(data); }