Merge pull request #2344 from CosmWasm/aw/memory-safety-doc

Add safety documentation
CosmWasm · Jan 29, 2025 · 9d5bfef · 9d5bfef
2 parents c365711 + 56d9d29
commit 9d5bfef
Show file tree

Hide file tree

Showing 4 changed files with 137 additions and 69 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,23 +13,29 @@ and this project adheres to
 - cosmwasm-std: Implement `From<Uint64> for u{64,128}`,
   `From<Uint128> for u128`, `From<Int64> for i{64,128}`, and
   `From<Int128> 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
@@ -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<Owned>) };
+    let _ =
+        unsafe { Region::from_heap_ptr(ptr::NonNull::new(pointer as *mut Region<Owned>).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<u8> = unsafe { Region::from_heap_ptr(env_ptr).into_vec() };
-    let info: Vec<u8> = unsafe { Region::from_heap_ptr(info_ptr).into_vec() };
-    let msg: Vec<u8> = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() };
+    let env: Vec<u8> =
+        unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() };
+    let info: Vec<u8> =
+        unsafe { Region::from_heap_ptr(ptr::NonNull::new(info_ptr).unwrap()).into_vec() };
+    let msg: Vec<u8> =
+        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<u8> = unsafe { Region::from_heap_ptr(env_ptr).into_vec() };
-    let info: Vec<u8> = unsafe { Region::from_heap_ptr(info_ptr).into_vec() };
-    let msg: Vec<u8> = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() };
+    let env: Vec<u8> =
+        unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() };
+    let info: Vec<u8> =
+        unsafe { Region::from_heap_ptr(ptr::NonNull::new(info_ptr).unwrap()).into_vec() };
+    let msg: Vec<u8> =
+        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<u8> = unsafe { Region::from_heap_ptr(env_ptr).into_vec() };
-    let msg: Vec<u8> = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() };
+    let env: Vec<u8> =
+        unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() };
+    let msg: Vec<u8> =
+        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<u8> = unsafe { Region::from_heap_ptr(env_ptr).into_vec() };
-    let msg: Vec<u8> = 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<u8> =
+        unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() };
+    let msg: Vec<u8> =
+        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<u8> = unsafe { Region::from_heap_ptr(env_ptr).into_vec() };
-    let msg: Vec<u8> = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() };
+    let env: Vec<u8> =
+        unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() };
+    let msg: Vec<u8> =
+        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<u8> = unsafe { Region::from_heap_ptr(env_ptr).into_vec() };
-    let msg: Vec<u8> = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() };
+    let env: Vec<u8> =
+        unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() };
+    let msg: Vec<u8> =
+        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<u8> = unsafe { Region::from_heap_ptr(env_ptr).into_vec() };
-    let msg: Vec<u8> = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() };
+    let env: Vec<u8> =
+        unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() };
+    let msg: Vec<u8> =
+        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<u8> = unsafe { Region::from_heap_ptr(env_ptr).into_vec() };
-    let msg: Vec<u8> = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() };
+    let env: Vec<u8> =
+        unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() };
+    let msg: Vec<u8> =
+        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<u8> = unsafe { Region::from_heap_ptr(env_ptr).into_vec() };
-    let msg: Vec<u8> = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() };
+    let env: Vec<u8> =
+        unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() };
+    let msg: Vec<u8> =
+        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<u8> = unsafe { Region::from_heap_ptr(env_ptr).into_vec() };
-    let msg: Vec<u8> = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() };
+    let env: Vec<u8> =
+        unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() };
+    let msg: Vec<u8> =
+        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<u8> = unsafe { Region::from_heap_ptr(env_ptr).into_vec() };
-    let msg: Vec<u8> = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() };
+    let env: Vec<u8> =
+        unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() };
+    let msg: Vec<u8> =
+        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<u8> = unsafe { Region::from_heap_ptr(env_ptr).into_vec() };
-    let msg: Vec<u8> = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() };
+    let env: Vec<u8> =
+        unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() };
+    let msg: Vec<u8> =
+        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<u8> = unsafe { Region::from_heap_ptr(env_ptr).into_vec() };
-    let msg: Vec<u8> = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() };
+    let env: Vec<u8> =
+        unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() };
+    let msg: Vec<u8> =
+        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<u8> = unsafe { Region::from_heap_ptr(env_ptr).into_vec() };
-    let msg: Vec<u8> = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() };
+    let env: Vec<u8> =
+        unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() };
+    let msg: Vec<u8> =
+        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<u8> = unsafe { Region::from_heap_ptr(env_ptr).into_vec() };
-    let msg: Vec<u8> = unsafe { Region::from_heap_ptr(msg_ptr).into_vec() };
+    let env: Vec<u8> =
+        unsafe { Region::from_heap_ptr(ptr::NonNull::new(env_ptr).unwrap()).into_vec() };
+    let msg: Vec<u8> =
+        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
@@ -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<Owned>;
-        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<Owned>;
-        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<Self::Item> {
         let next_result = unsafe { db_next(self.iterator_id) };
         let kv_region_ptr = next_result as *mut Region<Owned>;
-        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<Owned>) };
+        unsafe { Region::from_heap_ptr(ptr::NonNull::new(region as *mut Region<Owned>).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<Owned>).into_vec() };
+                let pubkey = unsafe {
+                    Region::from_heap_ptr(
+                        ptr::NonNull::new(pubkey_ptr as *mut Region<Owned>).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<Owned>).into_vec() };
+                let pubkey = unsafe {
+                    Region::from_heap_ptr(
+                        ptr::NonNull::new(pubkey_ptr as *mut Region<Owned>).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<Owned>) -> 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<Owned>).into_vec() };
+        let response = unsafe {
+            Region::from_heap_ptr(ptr::NonNull::new(response_ptr as *mut Region<Owned>).unwrap())
+                .into_vec()
+        };
 
         from_json(&response).unwrap_or_else(|parsing_err| {
             SystemResult::Err(SystemError::InvalidResponse {