From ffac2d6daa67bcb79782a3a15b9917d5de711e7f Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Wed, 5 Jan 2022 16:54:22 +0100 Subject: [PATCH] Don't panic across FFI Panicking across FFI was UB in older Rust versions and thus because of MSRV it's safer to avoid it. This replaces the panic with print+abort on `std` and double panic on no-std. Closes #354 --- contrib/test.sh | 2 +- secp256k1-sys/src/lib.rs | 33 ++++++++++++++++++++++++++++++--- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/contrib/test.sh b/contrib/test.sh index dc936ab75..b96918472 100755 --- a/contrib/test.sh +++ b/contrib/test.sh @@ -80,7 +80,7 @@ if [ "$DO_ASAN" = true ]; then fi # Test if panic in C code aborts the process (either with a real panic or with SIGILL) -cargo test -- --ignored --exact 'tests::test_panic_raw_ctx_should_terminate_abnormally' 2>&1 | tee /dev/stderr | grep "SIGILL\\|panicked at '\[libsecp256k1\]" +cargo test -- --ignored --exact 'tests::test_panic_raw_ctx_should_terminate_abnormally' 2>&1 | tee /dev/stderr | grep "SIGABRT\\|SIGILL\\|panicked at '\[libsecp256k1\]" # Bench if [ "$DO_BENCH" = true ]; then diff --git a/secp256k1-sys/src/lib.rs b/secp256k1-sys/src/lib.rs index 4f0f0d5de..d9bf511b3 100644 --- a/secp256k1-sys/src/lib.rs +++ b/secp256k1-sys/src/lib.rs @@ -569,6 +569,33 @@ pub unsafe fn secp256k1_context_destroy(ctx: *mut Context) { rustsecp256k1_v0_4_1_context_destroy(ctx) } +/// FFI-safe replacement for panic +/// +/// Prints to stderr and aborts with `std`, double panics without `std`. +#[cfg_attr(not(feature = "std"), allow(unused))] +fn ffi_abort(msg: impl core::fmt::Display) -> ! { + #[cfg(feature = "std")] + { + eprintln!("[libsecp256k1] {}", msg); + std::process::abort() + } + #[cfg(not(feature = "std"))] + { + use core::fmt::Display; + + // Abort by double panic + struct PanicOnDrop(M); + + impl Drop for PanicOnDrop { + fn drop(&mut self) { + panic!("[libsecp256k1] {}", self.0); + } + } + + let _bomb = PanicOnDrop(&msg); + panic!("[libsecp256k1] {}", &msg) + } +} /// **This function is an override for the C function, this is the an edited version of the original description:** /// @@ -594,7 +621,7 @@ pub unsafe extern "C" fn rustsecp256k1_v0_4_1_default_illegal_callback_fn(messag use core::str; let msg_slice = slice::from_raw_parts(message as *const u8, strlen(message)); let msg = str::from_utf8_unchecked(msg_slice); - panic!("[libsecp256k1] illegal argument. {}", msg); + ffi_abort(format_args!("illegal argument. {}", msg)); } /// **This function is an override for the C function, this is the an edited version of the original description:** @@ -617,7 +644,7 @@ pub unsafe extern "C" fn rustsecp256k1_v0_4_1_default_error_callback_fn(message: use core::str; let msg_slice = slice::from_raw_parts(message as *const u8, strlen(message)); let msg = str::from_utf8_unchecked(msg_slice); - panic!("[libsecp256k1] internal consistency check failed {}", msg); + ffi_abort(format_args!("internal consistency check failed {}", msg)); } #[cfg(not(rust_secp_no_symbol_renaming))] @@ -826,7 +853,7 @@ mod fuzz_dummy { *output = 4; ptr::copy((*pk).0.as_ptr(), output.offset(1), 64); } else { - panic!("Bad flags"); + ffi_abort(format_args!("Bad flags")); } 1 }