From 5e415b60651037dabfd069e32d6792e408e7feea Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Fri, 23 Aug 2019 17:46:44 -0400 Subject: [PATCH 1/2] Add a static mut bool to prevent accidentally using fuzz functions --- secp256k1-sys/src/lib.rs | 28 +++++++++++++++++++++++++++- secp256k1-sys/src/recovery.rs | 1 + 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/secp256k1-sys/src/lib.rs b/secp256k1-sys/src/lib.rs index a7792351e..0dccfe64e 100644 --- a/secp256k1-sys/src/lib.rs +++ b/secp256k1-sys/src/lib.rs @@ -84,6 +84,7 @@ pub type EcdhHashFn = unsafe extern "C" fn( #[cfg(feature = "fuzztarget")] impl Context { pub fn flags(&self) -> u32 { + unsafe {assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); } self.0 as u32 } } @@ -405,7 +406,7 @@ unsafe fn strlen(mut str_ptr: *const c_char) -> usize { /// A trait for producing pointers that will always be valid in C. (assuming NULL pointer is a valid no-op) /// Rust doesn't promise what pointers does it give to ZST (https://doc.rust-lang.org/nomicon/exotic-sizes.html#zero-sized-types-zsts) /// In case the type is empty this trait will give a NULL pointer, which should be handled in C. -/// +/// pub trait CPtr { type Target; fn as_c_ptr(&self) -> *const Self::Target; @@ -447,6 +448,9 @@ mod fuzz_dummy { #[allow(non_upper_case_globals)] pub static secp256k1_context_no_precomp: &Context = &Context(0); + pub static mut UNSAFE_CRYPTO_FUZZING: bool = false; + pub const UNSAFE_CRYPTO_WARNING: &str = "Tried fuzzing without setting the UNSAFE_CRYPTO_FUZZING variable"; + extern "C" { #[cfg_attr(not(feature = "external-symbols"), link_name = "rustsecp256k1_v0_1_1_ecdh_hash_function_default")] pub static secp256k1_ecdh_hash_function_default: EcdhHashFn; @@ -457,22 +461,26 @@ mod fuzz_dummy { // Contexts /// Creates a dummy context, tracking flags to ensure proper calling semantics pub unsafe fn secp256k1_context_preallocated_create(_ptr: *mut c_void, flags: c_uint) -> *mut Context { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); let b = Box::new(Context(flags as i32)); Box::into_raw(b) } /// Return dummy size of context struct. pub unsafe fn secp256k1_context_preallocated_size(_flags: c_uint) -> size_t { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); mem::size_of::() } /// Return dummy size of context struct. pub unsafe fn secp256k1_context_preallocated_clone_size(_cx: *mut Context) -> size_t { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); mem::size_of::() } /// Copies a dummy context pub unsafe fn secp256k1_context_preallocated_clone(cx: *const Context, prealloc: *mut c_void) -> *mut Context { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); let ret = prealloc as *mut Context; *ret = (*cx).clone(); ret @@ -480,6 +488,7 @@ mod fuzz_dummy { /// "Destroys" a dummy context pub unsafe fn secp256k1_context_preallocated_destroy(cx: *mut Context) { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); (*cx).0 = 0; } @@ -487,6 +496,7 @@ mod fuzz_dummy { pub unsafe fn secp256k1_context_randomize(cx: *mut Context, _seed32: *const c_uchar) -> c_int { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); assert!(!cx.is_null() && (*cx).0 as u32 & !(SECP256K1_START_NONE | SECP256K1_START_VERIFY | SECP256K1_START_SIGN) == 0); 1 } @@ -496,6 +506,7 @@ mod fuzz_dummy { pub unsafe fn secp256k1_ec_pubkey_parse(cx: *const Context, pk: *mut PublicKey, input: *const c_uchar, in_len: size_t) -> c_int { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); assert!(!cx.is_null() && (*cx).0 as u32 & !(SECP256K1_START_NONE | SECP256K1_START_VERIFY | SECP256K1_START_SIGN) == 0); match in_len { 33 => { @@ -524,6 +535,7 @@ mod fuzz_dummy { out_len: *mut size_t, pk: *const PublicKey, compressed: c_uint) -> c_int { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); assert!(!cx.is_null() && (*cx).0 as u32 & !(SECP256K1_START_NONE | SECP256K1_START_VERIFY | SECP256K1_START_SIGN) == 0); if test_pk_validate(cx, pk) != 1 { return 0; } if compressed == SECP256K1_SER_COMPRESSED { @@ -555,6 +567,7 @@ mod fuzz_dummy { pub unsafe fn secp256k1_ecdsa_signature_parse_compact(cx: *const Context, sig: *mut Signature, input64: *const c_uchar) -> c_int { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); assert!(!cx.is_null() && (*cx).0 as u32 & !(SECP256K1_START_NONE | SECP256K1_START_VERIFY | SECP256K1_START_SIGN) == 0); if secp256k1_ec_seckey_verify(cx, input64.offset(32)) != 1 { return 0; } // sig should be msg32||sk ptr::copy(input64, (*sig).0[..].as_mut_ptr(), 64); @@ -571,6 +584,7 @@ mod fuzz_dummy { pub unsafe fn secp256k1_ecdsa_signature_serialize_der(cx: *const Context, output: *mut c_uchar, out_len: *mut size_t, sig: *const Signature) -> c_int { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); assert!(!cx.is_null() && (*cx).0 as u32 & !(SECP256K1_START_NONE | SECP256K1_START_VERIFY | SECP256K1_START_SIGN) == 0); let mut len_r = 33; @@ -609,6 +623,7 @@ mod fuzz_dummy { pub unsafe fn secp256k1_ecdsa_signature_serialize_compact(cx: *const Context, output64: *mut c_uchar, sig: *const Signature) -> c_int { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); assert!(!cx.is_null() && (*cx).0 as u32 & !(SECP256K1_START_NONE | SECP256K1_START_VERIFY | SECP256K1_START_SIGN) == 0); ptr::copy((*sig).0[..].as_ptr(), output64, 64); 1 @@ -627,6 +642,7 @@ mod fuzz_dummy { msg32: *const c_uchar, pk: *const PublicKey) -> c_int { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); assert!(!cx.is_null() && (*cx).0 as u32 & !(SECP256K1_START_NONE | SECP256K1_START_VERIFY | SECP256K1_START_SIGN) == 0); assert!((*cx).0 as u32 & SECP256K1_START_VERIFY == SECP256K1_START_VERIFY); if test_pk_validate(cx, pk) != 1 { return 0; } @@ -650,6 +666,7 @@ mod fuzz_dummy { _noncefn: NonceFn, _noncedata: *const c_void) -> c_int { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); assert!(!cx.is_null() && (*cx).0 as u32 & !(SECP256K1_START_NONE | SECP256K1_START_VERIFY | SECP256K1_START_SIGN) == 0); assert!((*cx).0 as u32 & SECP256K1_START_SIGN == SECP256K1_START_SIGN); if secp256k1_ec_seckey_verify(cx, sk) != 1 { return 0; } @@ -662,6 +679,7 @@ mod fuzz_dummy { /// Checks that pk != 0xffff...ffff and pk[0..32] == pk[32..64] pub unsafe fn test_pk_validate(cx: *const Context, pk: *const PublicKey) -> c_int { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); assert!(!cx.is_null() && (*cx).0 as u32 & !(SECP256K1_START_NONE | SECP256K1_START_VERIFY | SECP256K1_START_SIGN) == 0); if (*pk).0[0..32] != (*pk).0[32..64] || secp256k1_ec_seckey_verify(cx, (*pk).0[0..32].as_ptr()) == 0 { 0 @@ -673,6 +691,7 @@ mod fuzz_dummy { /// Checks that sk != 0xffff...ffff pub unsafe fn secp256k1_ec_seckey_verify(cx: *const Context, sk: *const c_uchar) -> c_int { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); assert!(!cx.is_null() && (*cx).0 as u32 & !(SECP256K1_START_NONE | SECP256K1_START_VERIFY | SECP256K1_START_SIGN) == 0); let mut res = 0; for i in 0..32 { @@ -684,6 +703,7 @@ mod fuzz_dummy { /// Sets pk to sk||sk pub unsafe fn secp256k1_ec_pubkey_create(cx: *const Context, pk: *mut PublicKey, sk: *const c_uchar) -> c_int { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); assert!(!cx.is_null() && (*cx).0 as u32 & !(SECP256K1_START_NONE | SECP256K1_START_VERIFY | SECP256K1_START_SIGN) == 0); if secp256k1_ec_seckey_verify(cx, sk) != 1 { return 0; } ptr::copy(sk, (*pk).0[0..32].as_mut_ptr(), 32); @@ -699,6 +719,7 @@ mod fuzz_dummy { sk: *mut c_uchar, tweak: *const c_uchar) -> c_int { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); assert!(!cx.is_null() && (*cx).0 as u32 & !(SECP256K1_START_NONE | SECP256K1_START_VERIFY | SECP256K1_START_SIGN) == 0); if secp256k1_ec_seckey_verify(cx, sk) != 1 { return 0; } ptr::copy(tweak.offset(16), sk.offset(16), 16); @@ -711,6 +732,7 @@ mod fuzz_dummy { pk: *mut PublicKey, tweak: *const c_uchar) -> c_int { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); assert!(!cx.is_null() && (*cx).0 as u32 & !(SECP256K1_START_NONE | SECP256K1_START_VERIFY | SECP256K1_START_SIGN) == 0); if test_pk_validate(cx, pk) != 1 { return 0; } ptr::copy(tweak.offset(16), (*pk).0[16..32].as_mut_ptr(), 16); @@ -725,6 +747,7 @@ mod fuzz_dummy { sk: *mut c_uchar, tweak: *const c_uchar) -> c_int { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); assert!(!cx.is_null() && (*cx).0 as u32 & !(SECP256K1_START_NONE | SECP256K1_START_VERIFY | SECP256K1_START_SIGN) == 0); if secp256k1_ec_seckey_verify(cx, sk) != 1 { return 0; } ptr::copy(tweak.offset(16), sk.offset(16), 16); @@ -737,6 +760,7 @@ mod fuzz_dummy { pk: *mut PublicKey, tweak: *const c_uchar) -> c_int { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); assert!(!cx.is_null() && (*cx).0 as u32 & !(SECP256K1_START_NONE | SECP256K1_START_VERIFY | SECP256K1_START_SIGN) == 0); if test_pk_validate(cx, pk) != 1 { return 0; } ptr::copy(tweak.offset(16), (*pk).0[16..32].as_mut_ptr(), 16); @@ -751,6 +775,7 @@ mod fuzz_dummy { ins: *const *const PublicKey, n: c_int) -> c_int { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); assert!(!cx.is_null() && (*cx).0 as u32 & !(SECP256K1_START_NONE | SECP256K1_START_VERIFY | SECP256K1_START_SIGN) == 0); assert!(n <= 32 && n >= 0); //TODO: Remove this restriction? for i in 0..n { @@ -772,6 +797,7 @@ mod fuzz_dummy { _hashfp: EcdhHashFn, _data: *mut c_void, ) -> c_int { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); assert!(!cx.is_null() && (*cx).0 as u32 & !(SECP256K1_START_NONE | SECP256K1_START_VERIFY | SECP256K1_START_SIGN) == 0); if secp256k1_ec_seckey_verify(cx, scalar) != 1 { return 0; } diff --git a/secp256k1-sys/src/recovery.rs b/secp256k1-sys/src/recovery.rs index 4e5d2b983..9b1127c6a 100644 --- a/secp256k1-sys/src/recovery.rs +++ b/secp256k1-sys/src/recovery.rs @@ -108,6 +108,7 @@ mod fuzz_dummy { _noncefn: NonceFn, _noncedata: *const c_void) -> c_int { + assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING); assert!(!cx.is_null() && (*cx).flags() & !(SECP256K1_START_NONE | SECP256K1_START_VERIFY | SECP256K1_START_SIGN) == 0); assert!((*cx).flags() & SECP256K1_START_SIGN == SECP256K1_START_SIGN); if secp256k1_ec_seckey_verify(cx, sk) != 1 { return 0; } From d58822ebea0875e832d8ef44a57aeba52923f1ed Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Fri, 23 Aug 2019 17:50:17 -0400 Subject: [PATCH 2/2] Add tests for the UNSAFE_CRYPTO_FUZZING variable --- .travis.yml | 1 + secp256k1-sys/src/recovery.rs | 7 +-- src/ecdh.rs | 2 +- src/key.rs | 2 +- src/lib.rs | 81 ++++++++++++++++++++++++++++++++++- src/recovery.rs | 2 +- 6 files changed, 88 insertions(+), 7 deletions(-) diff --git a/.travis.yml b/.travis.yml index 0b57c159f..714a6ddbc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -38,6 +38,7 @@ script: - cargo test --verbose --features="rand rand-std" - cargo test --verbose --features="rand serde" - cargo test --verbose --features="rand serde recovery endomorphism" + - cargo test --verbose --lib --features=fuzztarget - cargo build --verbose - cargo test --verbose - cargo build --verbose --release diff --git a/secp256k1-sys/src/recovery.rs b/secp256k1-sys/src/recovery.rs index 9b1127c6a..710fd595b 100644 --- a/secp256k1-sys/src/recovery.rs +++ b/secp256k1-sys/src/recovery.rs @@ -79,9 +79,10 @@ mod fuzz_dummy { use self::std::ptr; use super::RecoverableSignature; use types::*; - use ::{Signature, Context, PublicKey, NonceFn, secp256k1_ec_seckey_verify, - SECP256K1_START_NONE, SECP256K1_START_VERIFY, SECP256K1_START_SIGN}; - + use ::{ + secp256k1_ec_seckey_verify, Context, NonceFn, PublicKey, Signature, SECP256K1_START_NONE, + SECP256K1_START_SIGN, SECP256K1_START_VERIFY, UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING, + }; pub unsafe fn secp256k1_ecdsa_recoverable_signature_parse_compact(_cx: *const Context, _sig: *mut RecoverableSignature, _input64: *const c_uchar, _recid: c_int) -> c_int { diff --git a/src/ecdh.rs b/src/ecdh.rs index 394abe349..6cc47299e 100644 --- a/src/ecdh.rs +++ b/src/ecdh.rs @@ -162,7 +162,7 @@ impl SharedSecret { } } -#[cfg(test)] +#[cfg(all(test, not(feature = "fuzztarget")))] mod tests { use rand::thread_rng; use super::SharedSecret; diff --git a/src/key.rs b/src/key.rs index ba1d22177..6e48f0e1a 100644 --- a/src/key.rs +++ b/src/key.rs @@ -443,7 +443,7 @@ impl<'de> ::serde::Deserialize<'de> for PublicKey { } } -#[cfg(test)] +#[cfg(all(test, not(feature = "fuzztarget")))] mod test { use Secp256k1; use from_hex; diff --git a/src/lib.rs b/src/lib.rs index a60a2b5ca..44b61b1fa 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -707,7 +707,7 @@ fn from_hex(hex: &str, target: &mut [u8]) -> Result { } -#[cfg(test)] +#[cfg(all(test, not(feature = "fuzztarget")))] mod tests { use rand::{RngCore, thread_rng}; use std::str::FromStr; @@ -1160,3 +1160,82 @@ mod benches { }); } } + +#[cfg(all(test, feature = "fuzztarget"))] +mod test_fuzz { + use super::*; + use std::str::FromStr; + use std::panic::{self, UnwindSafe}; + use std::mem; + + + pub fn crashed_for_fuzz R + UnwindSafe, R>(f: F) -> bool { + if let Err(e) = panic::catch_unwind(f) { + if let Some(st) = e.downcast_ref::<&str>() { + return st.contains(ffi::UNSAFE_CRYPTO_WARNING); + } + } + false + } + + fn fuzz_not_set_var() { + unsafe {ffi::UNSAFE_CRYPTO_FUZZING = false}; + let mut raw_fake_context: ffi::Context = unsafe {mem::transmute(VerifyOnly::FLAGS | SignOnly::FLAGS)}; + let fake_context: Secp256k1 = Secp256k1 { + ctx: &mut raw_fake_context, + phantom: PhantomData, + buf: ptr::null_mut::<[u8;0]>() as *mut [u8] + }; + let msg = Message::from_slice(&[1u8; 32]).unwrap(); + let sec = SecretKey::from_str("01010101010101010001020304050607ffff0000ffff00006363636363636363").unwrap(); + let sig: Signature = unsafe { std::mem::transmute::<_, ffi::Signature>([3u8; 64]).into() }; + + assert!(crashed_for_fuzz(|| SecretKey::from_slice(&[2; 32]))); + assert!(crashed_for_fuzz(|| PublicKey::from_slice(&[2; 33]))); + assert!(crashed_for_fuzz(|| Signature::from_compact(&[3; 64]))); + assert!(crashed_for_fuzz(|| Secp256k1::new())); + assert!(crashed_for_fuzz(|| sec.clone().add_assign(&[2u8; 32]))); + assert!(crashed_for_fuzz(|| sec.clone().mul_assign(&[2u8; 32]))); + assert!(crashed_for_fuzz(|| sig.serialize_compact())); + assert!(crashed_for_fuzz(|| fake_context.sign(&msg, &sec))); + #[cfg(feature = "recovery")] + { + assert!(crashed_for_fuzz(||fake_context.sign_recoverable(&msg, &sec))); + } + assert!(crashed_for_fuzz(|| drop(fake_context))); + } + + fn fuzz_set_var_not_crash() { + unsafe {ffi::UNSAFE_CRYPTO_FUZZING = true;} + let context = Secp256k1::new(); + let msg = Message::from_slice(&[1u8; 32]).unwrap(); + let _ = SecretKey::from_slice(&[2; 32]); + let _ = PublicKey::from_slice(&[2; 33]); + let _ = Signature::from_compact(&[3; 64]); + let _ = Secp256k1::new(); + let mut sec = SecretKey::from_str("01010101010101010001020304050607ffff0000ffff00006363636363636363").unwrap(); + let _ = sec.add_assign(&[2u8; 32]); + let mut sec = SecretKey::from_str("01010101010101010001020304050607ffff0000ffff00006363636363636363").unwrap(); + let _ = sec.mul_assign(&[2u8; 32]); + let sig: Signature = unsafe { std::mem::transmute::<_, ffi::Signature>([3u8; 64]).into() }; + let _ = sig.serialize_compact(); + let pubkey1: PublicKey = unsafe { mem::transmute::<_, ffi::PublicKey>([3u8; 64]).into() }; + let pubkey2: PublicKey = unsafe { mem::transmute::<_, ffi::PublicKey>([5u8; 64]).into() }; + let _ = pubkey1.combine(&pubkey2); + let sig = context.sign(&msg, &sec); + assert_eq!(context.verify(&msg, &sig, &pubkey1).unwrap_err(), Error::IncorrectSignature); + + #[cfg(feature = "recovery")] + { + let _ = context.sign_recoverable(&msg, &sec); + } + + unsafe {ffi::UNSAFE_CRYPTO_FUZZING = false;} + } + + #[test] + fn test_fuzz_setting_var() { + fuzz_not_set_var(); + fuzz_set_var_not_crash(); + } +} diff --git a/src/recovery.rs b/src/recovery.rs index 015299419..f759c16dd 100644 --- a/src/recovery.rs +++ b/src/recovery.rs @@ -191,7 +191,7 @@ impl Secp256k1 { } -#[cfg(test)] +#[cfg(all(test, not(feature = "fuzztarget")))] mod tests { use rand::{RngCore, thread_rng};