Skip to content

Trying to prevent accidental usage of the fuzztarget functions #154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Trying to prevent accidental usage of the fuzztarget functions #154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5e415b6
Add a static mut bool to prevent accidentally using fuzz functions
elichai Aug 23, 2019
d58822e
Add tests for the UNSAFE_CRYPTO_FUZZING variable
elichai Aug 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 27 additions & 1 deletion secp256k1-sys/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -457,36 +461,42 @@ 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::<Context>()
}

/// 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::<Context>()
}

/// 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
}

/// "Destroys" a dummy context
pub unsafe fn secp256k1_context_preallocated_destroy(cx: *mut Context) {
assert!(UNSAFE_CRYPTO_FUZZING, UNSAFE_CRYPTO_WARNING);
(*cx).0 = 0;
}

/// Asserts that cx is properly initialized
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
}
Expand All @@ -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 => {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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; }
Expand All @@ -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; }
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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 {
Expand All @@ -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; }

Expand Down
8 changes: 5 additions & 3 deletions secp256k1-sys/src/recovery.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -108,6 +109,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; }
Expand Down
2 changes: 1 addition & 1 deletion src/ecdh.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ impl SharedSecret {
}
}

#[cfg(test)]
#[cfg(all(test, not(feature = "fuzztarget")))]
mod tests {
use rand::thread_rng;
use super::SharedSecret;
Expand Down
2 changes: 1 addition & 1 deletion src/key.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
81 changes: 80 additions & 1 deletion src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,7 @@ fn from_hex(hex: &str, target: &mut [u8]) -> Result<usize, ()> {
}


#[cfg(test)]
#[cfg(all(test, not(feature = "fuzztarget")))]
mod tests {
use rand::{RngCore, thread_rng};
use std::str::FromStr;
Expand Down Expand Up @@ -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<F: FnOnce() -> 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<All> = 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();
}
}
2 changes: 1 addition & 1 deletion src/recovery.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ impl<C: Verification> Secp256k1<C> {
}


#[cfg(test)]
#[cfg(all(test, not(feature = "fuzztarget")))]
mod tests {
use rand::{RngCore, thread_rng};

Expand Down