From 7dd9fa5413eac6fdf7c545e3003b2426d0da9262 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Wed, 30 Oct 2024 17:06:51 +0100 Subject: [PATCH 1/2] Add panic handler function --- libwasmvm/src/cache.rs | 10 ++++++++++ libwasmvm/src/calls.rs | 3 +++ libwasmvm/src/handle_vm_panic.rs | 24 ++++++++++++++++++++++++ libwasmvm/src/lib.rs | 1 + 4 files changed, 38 insertions(+) create mode 100644 libwasmvm/src/handle_vm_panic.rs diff --git a/libwasmvm/src/cache.rs b/libwasmvm/src/cache.rs index 0ed0ea1b1..bada6e54b 100644 --- a/libwasmvm/src/cache.rs +++ b/libwasmvm/src/cache.rs @@ -9,6 +9,7 @@ use serde::Serialize; use crate::api::GoApi; use crate::args::{CACHE_ARG, CHECKSUM_ARG, CONFIG_ARG, WASM_ARG}; use crate::error::{handle_c_error_binary, handle_c_error_default, handle_c_error_ptr, Error}; +use crate::handle_vm_panic::handle_vm_panic; use crate::memory::{ByteSliceView, UnmanagedVector}; use crate::querier::GoQuerier; use crate::storage::GoStorage; @@ -32,6 +33,7 @@ pub extern "C" fn init_cache( ) -> *mut cache_t { let r = catch_unwind(|| do_init_cache(config)).unwrap_or_else(|err| { eprintln!("Panic in do_init_cache: {err:?}"); + handle_vm_panic(); Err(Error::panic()) }); handle_c_error_ptr(r, error_msg) as *mut cache_t @@ -57,6 +59,7 @@ pub extern "C" fn save_wasm( Some(c) => catch_unwind(AssertUnwindSafe(move || do_save_wasm(c, wasm, unchecked))) .unwrap_or_else(|err| { eprintln!("Panic in do_save_wasm: {err:?}"); + handle_vm_panic(); Err(Error::panic()) }), None => Err(Error::unset_arg(CACHE_ARG)), @@ -89,6 +92,7 @@ pub extern "C" fn remove_wasm( Some(c) => catch_unwind(AssertUnwindSafe(move || do_remove_wasm(c, checksum))) .unwrap_or_else(|err| { eprintln!("Panic in do_remove_wasm: {err:?}"); + handle_vm_panic(); Err(Error::panic()) }), None => Err(Error::unset_arg(CACHE_ARG)), @@ -118,6 +122,7 @@ pub extern "C" fn load_wasm( Some(c) => catch_unwind(AssertUnwindSafe(move || do_load_wasm(c, checksum))) .unwrap_or_else(|err| { eprintln!("Panic in do_load_wasm: {err:?}"); + handle_vm_panic(); Err(Error::panic()) }), None => Err(Error::unset_arg(CACHE_ARG)), @@ -148,6 +153,7 @@ pub extern "C" fn pin( Some(c) => { catch_unwind(AssertUnwindSafe(move || do_pin(c, checksum))).unwrap_or_else(|err| { eprintln!("Panic in do_pin: {err:?}"); + handle_vm_panic(); Err(Error::panic()) }) } @@ -178,6 +184,7 @@ pub extern "C" fn unpin( Some(c) => { catch_unwind(AssertUnwindSafe(move || do_unpin(c, checksum))).unwrap_or_else(|err| { eprintln!("Panic in do_unpin: {err:?}"); + handle_vm_panic(); Err(Error::panic()) }) } @@ -286,6 +293,7 @@ pub extern "C" fn analyze_code( Some(c) => catch_unwind(AssertUnwindSafe(move || do_analyze_code(c, checksum))) .unwrap_or_else(|err| { eprintln!("Panic in do_analyze_code: {err:?}"); + handle_vm_panic(); Err(Error::panic()) }), None => Err(Error::unset_arg(CACHE_ARG)), @@ -364,6 +372,7 @@ pub extern "C" fn get_metrics( Some(c) => { catch_unwind(AssertUnwindSafe(move || do_get_metrics(c))).unwrap_or_else(|err| { eprintln!("Panic in do_get_metrics: {err:?}"); + handle_vm_panic(); Err(Error::panic()) }) } @@ -419,6 +428,7 @@ pub extern "C" fn get_pinned_metrics( Some(c) => { catch_unwind(AssertUnwindSafe(move || do_get_pinned_metrics(c))).unwrap_or_else(|err| { eprintln!("Panic in do_get_pinned_metrics: {err:?}"); + handle_vm_panic(); Err(Error::panic()) }) } diff --git a/libwasmvm/src/calls.rs b/libwasmvm/src/calls.rs index 8618aef8c..382b18c7a 100644 --- a/libwasmvm/src/calls.rs +++ b/libwasmvm/src/calls.rs @@ -19,6 +19,7 @@ use crate::args::{ARG1, ARG2, ARG3, CACHE_ARG, CHECKSUM_ARG, GAS_REPORT_ARG}; use crate::cache::{cache_t, to_cache}; use crate::db::Db; use crate::error::{handle_c_error_binary, Error}; +use crate::handle_vm_panic::handle_vm_panic; use crate::memory::{ByteSliceView, UnmanagedVector}; use crate::querier::GoQuerier; use crate::storage::GoStorage; @@ -529,6 +530,7 @@ fn call_2_args( })) .unwrap_or_else(|err| { eprintln!("Panic in do_call_2_args: {err:?}"); + handle_vm_panic(); Err(Error::panic()) }), None => Err(Error::unset_arg(CACHE_ARG)), @@ -623,6 +625,7 @@ fn call_3_args( })) .unwrap_or_else(|err| { eprintln!("Panic in do_call_3_args: {err:?}"); + handle_vm_panic(); Err(Error::panic()) }), None => Err(Error::unset_arg(CACHE_ARG)), diff --git a/libwasmvm/src/handle_vm_panic.rs b/libwasmvm/src/handle_vm_panic.rs new file mode 100644 index 000000000..6fb11d69f --- /dev/null +++ b/libwasmvm/src/handle_vm_panic.rs @@ -0,0 +1,24 @@ +/// A function to process cases in which the VM panics. +/// +/// We want to provide as much debug information as possible +/// as those cases are not expated to happen during healthy operations. +pub fn handle_vm_panic() { + eprintln!( + "This indicates a panic in during the operations of libwasmvm/cosmwasm-vm. +Such panics must not happen and are considered bugs. If you see this in any real-world or +close-to-real-world usage of wasmvm, please consider filing a security report, +no matter if it can be abused or not: +(https://github.com/CosmWasm/advisories/blob/main/SECURITY.md#reporting-a-vulnerability). +Thank you for your help keeping CosmWasm safe and secure 💚" + ); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn handle_vm_panic_works() { + handle_vm_panic(); + } +} diff --git a/libwasmvm/src/lib.rs b/libwasmvm/src/lib.rs index 8edf84e20..9db5cb79f 100644 --- a/libwasmvm/src/lib.rs +++ b/libwasmvm/src/lib.rs @@ -9,6 +9,7 @@ mod db; mod error; mod gas_meter; mod gas_report; +mod handle_vm_panic; mod iterator; mod memory; mod querier; From b785826785433c3d1537851b4e66996fa40915c3 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Wed, 30 Oct 2024 17:20:42 +0100 Subject: [PATCH 2/2] Move called function and error into handle_vm_panic --- libwasmvm/src/cache.rs | 27 +++++++++------------------ libwasmvm/src/calls.rs | 6 ++---- libwasmvm/src/handle_vm_panic.rs | 14 ++++++++++++-- 3 files changed, 23 insertions(+), 24 deletions(-) diff --git a/libwasmvm/src/cache.rs b/libwasmvm/src/cache.rs index bada6e54b..02c8d5e48 100644 --- a/libwasmvm/src/cache.rs +++ b/libwasmvm/src/cache.rs @@ -32,8 +32,7 @@ pub extern "C" fn init_cache( error_msg: Option<&mut UnmanagedVector>, ) -> *mut cache_t { let r = catch_unwind(|| do_init_cache(config)).unwrap_or_else(|err| { - eprintln!("Panic in do_init_cache: {err:?}"); - handle_vm_panic(); + handle_vm_panic("do_init_cache", err); Err(Error::panic()) }); handle_c_error_ptr(r, error_msg) as *mut cache_t @@ -58,8 +57,7 @@ pub extern "C" fn save_wasm( let r = match to_cache(cache) { Some(c) => catch_unwind(AssertUnwindSafe(move || do_save_wasm(c, wasm, unchecked))) .unwrap_or_else(|err| { - eprintln!("Panic in do_save_wasm: {err:?}"); - handle_vm_panic(); + handle_vm_panic("do_save_wasm", err); Err(Error::panic()) }), None => Err(Error::unset_arg(CACHE_ARG)), @@ -91,8 +89,7 @@ pub extern "C" fn remove_wasm( let r = match to_cache(cache) { Some(c) => catch_unwind(AssertUnwindSafe(move || do_remove_wasm(c, checksum))) .unwrap_or_else(|err| { - eprintln!("Panic in do_remove_wasm: {err:?}"); - handle_vm_panic(); + handle_vm_panic("do_remove_wasm", err); Err(Error::panic()) }), None => Err(Error::unset_arg(CACHE_ARG)), @@ -121,8 +118,7 @@ pub extern "C" fn load_wasm( let r = match to_cache(cache) { Some(c) => catch_unwind(AssertUnwindSafe(move || do_load_wasm(c, checksum))) .unwrap_or_else(|err| { - eprintln!("Panic in do_load_wasm: {err:?}"); - handle_vm_panic(); + handle_vm_panic("do_load_wasm", err); Err(Error::panic()) }), None => Err(Error::unset_arg(CACHE_ARG)), @@ -152,8 +148,7 @@ pub extern "C" fn pin( let r = match to_cache(cache) { Some(c) => { catch_unwind(AssertUnwindSafe(move || do_pin(c, checksum))).unwrap_or_else(|err| { - eprintln!("Panic in do_pin: {err:?}"); - handle_vm_panic(); + handle_vm_panic("do_pin", err); Err(Error::panic()) }) } @@ -183,8 +178,7 @@ pub extern "C" fn unpin( let r = match to_cache(cache) { Some(c) => { catch_unwind(AssertUnwindSafe(move || do_unpin(c, checksum))).unwrap_or_else(|err| { - eprintln!("Panic in do_unpin: {err:?}"); - handle_vm_panic(); + handle_vm_panic("do_unpin", err); Err(Error::panic()) }) } @@ -292,8 +286,7 @@ pub extern "C" fn analyze_code( let r = match to_cache(cache) { Some(c) => catch_unwind(AssertUnwindSafe(move || do_analyze_code(c, checksum))) .unwrap_or_else(|err| { - eprintln!("Panic in do_analyze_code: {err:?}"); - handle_vm_panic(); + handle_vm_panic("do_analyze_code", err); Err(Error::panic()) }), None => Err(Error::unset_arg(CACHE_ARG)), @@ -371,8 +364,7 @@ pub extern "C" fn get_metrics( let r = match to_cache(cache) { Some(c) => { catch_unwind(AssertUnwindSafe(move || do_get_metrics(c))).unwrap_or_else(|err| { - eprintln!("Panic in do_get_metrics: {err:?}"); - handle_vm_panic(); + handle_vm_panic("do_get_metrics", err); Err(Error::panic()) }) } @@ -427,8 +419,7 @@ pub extern "C" fn get_pinned_metrics( let r = match to_cache(cache) { Some(c) => { catch_unwind(AssertUnwindSafe(move || do_get_pinned_metrics(c))).unwrap_or_else(|err| { - eprintln!("Panic in do_get_pinned_metrics: {err:?}"); - handle_vm_panic(); + handle_vm_panic("do_get_pinned_metrics", err); Err(Error::panic()) }) } diff --git a/libwasmvm/src/calls.rs b/libwasmvm/src/calls.rs index 382b18c7a..b49ac142a 100644 --- a/libwasmvm/src/calls.rs +++ b/libwasmvm/src/calls.rs @@ -529,8 +529,7 @@ fn call_2_args( ) })) .unwrap_or_else(|err| { - eprintln!("Panic in do_call_2_args: {err:?}"); - handle_vm_panic(); + handle_vm_panic("do_call_2_args", err); Err(Error::panic()) }), None => Err(Error::unset_arg(CACHE_ARG)), @@ -624,8 +623,7 @@ fn call_3_args( ) })) .unwrap_or_else(|err| { - eprintln!("Panic in do_call_3_args: {err:?}"); - handle_vm_panic(); + handle_vm_panic("do_call_3_args", err); Err(Error::panic()) }), None => Err(Error::unset_arg(CACHE_ARG)), diff --git a/libwasmvm/src/handle_vm_panic.rs b/libwasmvm/src/handle_vm_panic.rs index 6fb11d69f..0eb0dca9d 100644 --- a/libwasmvm/src/handle_vm_panic.rs +++ b/libwasmvm/src/handle_vm_panic.rs @@ -1,8 +1,12 @@ +use std::any::Any; + /// A function to process cases in which the VM panics. /// /// We want to provide as much debug information as possible /// as those cases are not expated to happen during healthy operations. -pub fn handle_vm_panic() { +pub fn handle_vm_panic(what: &str, err: Box) { + eprintln!("Panic in {what}:"); + eprintln!("{err:?}"); // Does not show useful information, see https://users.rust-lang.org/t/return-value-from-catch-unwind-is-a-useless-any/89134/6 eprintln!( "This indicates a panic in during the operations of libwasmvm/cosmwasm-vm. Such panics must not happen and are considered bugs. If you see this in any real-world or @@ -17,8 +21,14 @@ Thank you for your help keeping CosmWasm safe and secure 💚" mod tests { use super::*; + use std::panic::catch_unwind; + #[test] fn handle_vm_panic_works() { - handle_vm_panic(); + fn nice_try() { + panic!("oh no!"); + } + let err = catch_unwind(nice_try).unwrap_err(); + handle_vm_panic("nice_try", err); } }