From 283b7d6e5142a3be8fedf85879e8ae827ee04235 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Fri, 28 Apr 2023 18:44:54 +0000 Subject: [PATCH 1/3] hashes: rename fuzzing cfg parameter to bitcoin_hashes_fuzz --- hashes/src/lib.rs | 2 +- hashes/src/ripemd160.rs | 8 ++++---- hashes/src/sha1.rs | 4 ++-- hashes/src/sha256.rs | 8 ++++---- hashes/src/sha512.rs | 8 ++++---- hashes/src/siphash24.rs | 4 ++-- hashes/src/util.rs | 4 ++-- 7 files changed, 19 insertions(+), 19 deletions(-) diff --git a/hashes/src/lib.rs b/hashes/src/lib.rs index 7c5dfdc2..840c3aa3 100644 --- a/hashes/src/lib.rs +++ b/hashes/src/lib.rs @@ -77,7 +77,7 @@ #![allow(ellipsis_inclusive_range_patterns)] #![cfg_attr(all(not(test), not(feature = "std")), no_std)] // Instead of littering the codebase for non-fuzzing code just globally allow. -#![cfg_attr(fuzzing, allow(dead_code, unused_imports))] +#![cfg_attr(hashes_fuzz, allow(dead_code, unused_imports))] #[cfg(all(feature = "alloc", not(feature = "std")))] extern crate alloc; diff --git a/hashes/src/ripemd160.rs b/hashes/src/ripemd160.rs index 52994e44..bc25b8dd 100644 --- a/hashes/src/ripemd160.rs +++ b/hashes/src/ripemd160.rs @@ -17,7 +17,7 @@ crate::internal_macros::hash_type! { "crate::util::json_hex_string::len_20" } -#[cfg(not(fuzzing))] +#[cfg(not(hashes_fuzz))] fn from_engine(mut e: HashEngine) -> Hash { // pad buffer with a single 1-bit then all 0s, until there are exactly 8 bytes remaining let data_len = e.length as u64; @@ -37,7 +37,7 @@ fn from_engine(mut e: HashEngine) -> Hash { Hash(e.midstate()) } -#[cfg(fuzzing)] +#[cfg(hashes_fuzz)] fn from_engine(e: HashEngine) -> Hash { let mut res = e.midstate(); res[0] ^= (e.length & 0xff) as u8; @@ -67,7 +67,7 @@ impl Default for HashEngine { impl crate::HashEngine for HashEngine { type MidState = [u8; 20]; - #[cfg(not(fuzzing))] + #[cfg(not(hashes_fuzz))] fn midstate(&self) -> [u8; 20] { let mut ret = [0; 20]; for (val, ret_bytes) in self.h.iter().zip(ret.chunks_exact_mut(4)) { @@ -76,7 +76,7 @@ impl crate::HashEngine for HashEngine { ret } - #[cfg(fuzzing)] + #[cfg(hashes_fuzz)] fn midstate(&self) -> [u8; 20] { let mut ret = [0; 20]; ret.copy_from_slice(&self.buffer[..20]); diff --git a/hashes/src/sha1.rs b/hashes/src/sha1.rs index d7237a2c..e033ef60 100644 --- a/hashes/src/sha1.rs +++ b/hashes/src/sha1.rs @@ -59,7 +59,7 @@ impl Default for HashEngine { impl crate::HashEngine for HashEngine { type MidState = [u8; 20]; - #[cfg(not(fuzzing))] + #[cfg(not(hashes_fuzz))] fn midstate(&self) -> [u8; 20] { let mut ret = [0; 20]; for (val, ret_bytes) in self.h.iter().zip(ret.chunks_exact_mut(4)) { @@ -68,7 +68,7 @@ impl crate::HashEngine for HashEngine { ret } - #[cfg(fuzzing)] + #[cfg(hashes_fuzz)] fn midstate(&self) -> [u8; 20] { let mut ret = [0; 20]; ret.copy_from_slice(&self.buffer[..20]); diff --git a/hashes/src/sha256.rs b/hashes/src/sha256.rs index 64279409..6f800207 100644 --- a/hashes/src/sha256.rs +++ b/hashes/src/sha256.rs @@ -17,7 +17,7 @@ crate::internal_macros::hash_type! { "crate::util::json_hex_string::len_32" } -#[cfg(not(fuzzing))] +#[cfg(not(hashes_fuzz))] fn from_engine(mut e: HashEngine) -> Hash { // pad buffer with a single 1-bit then all 0s, until there are exactly 8 bytes remaining let data_len = e.length as u64; @@ -37,7 +37,7 @@ fn from_engine(mut e: HashEngine) -> Hash { Hash(e.midstate().to_byte_array()) } -#[cfg(fuzzing)] +#[cfg(hashes_fuzz)] fn from_engine(e: HashEngine) -> Hash { let mut hash = e.midstate().to_byte_array(); if hash == [0; 32] { @@ -74,7 +74,7 @@ impl Default for HashEngine { impl crate::HashEngine for HashEngine { type MidState = Midstate; - #[cfg(not(fuzzing))] + #[cfg(not(hashes_fuzz))] fn midstate(&self) -> Midstate { let mut ret = [0; 32]; for (val, ret_bytes) in self.h.iter().zip(ret.chunks_exact_mut(4)) { @@ -83,7 +83,7 @@ impl crate::HashEngine for HashEngine { Midstate(ret) } - #[cfg(fuzzing)] + #[cfg(hashes_fuzz)] fn midstate(&self) -> Midstate { let mut ret = [0; 32]; ret.copy_from_slice(&self.buffer[..32]); diff --git a/hashes/src/sha512.rs b/hashes/src/sha512.rs index 1f3ad713..39b75e89 100644 --- a/hashes/src/sha512.rs +++ b/hashes/src/sha512.rs @@ -39,7 +39,7 @@ impl Default for HashEngine { impl crate::HashEngine for HashEngine { type MidState = [u8; 64]; - #[cfg(not(fuzzing))] + #[cfg(not(hashes_fuzz))] fn midstate(&self) -> [u8; 64] { let mut ret = [0; 64]; for (val, ret_bytes) in self.h.iter().zip(ret.chunks_exact_mut(8)) { @@ -48,7 +48,7 @@ impl crate::HashEngine for HashEngine { ret } - #[cfg(fuzzing)] + #[cfg(hashes_fuzz)] fn midstate(&self) -> [u8; 64] { let mut ret = [0; 64]; ret.copy_from_slice(&self.buffer[..64]); @@ -80,7 +80,7 @@ impl Hash { fn internal_engine() -> HashEngine { Default::default() } } -#[cfg(not(fuzzing))] +#[cfg(not(hashes_fuzz))] pub(crate) fn from_engine(mut e: HashEngine) -> Hash { // pad buffer with a single 1-bit then all 0s, until there are exactly 16 bytes remaining let data_len = e.length as u64; @@ -101,7 +101,7 @@ pub(crate) fn from_engine(mut e: HashEngine) -> Hash { Hash(e.midstate()) } -#[cfg(fuzzing)] +#[cfg(hashes_fuzz)] pub(crate) fn from_engine(e: HashEngine) -> Hash { let mut hash = e.midstate(); hash[0] ^= 0xff; // Make this distinct from SHA-256 diff --git a/hashes/src/siphash24.rs b/hashes/src/siphash24.rs index a2dd13bf..6eead542 100644 --- a/hashes/src/siphash24.rs +++ b/hashes/src/siphash24.rs @@ -16,10 +16,10 @@ crate::internal_macros::hash_type! { "crate::util::json_hex_string::len_8" } -#[cfg(not(fuzzing))] +#[cfg(not(hashes_fuzz))] fn from_engine(e: HashEngine) -> Hash { Hash::from_u64(Hash::from_engine_to_u64(e)) } -#[cfg(fuzzing)] +#[cfg(hashes_fuzz)] fn from_engine(e: HashEngine) -> Hash { let state = e.midstate(); Hash::from_u64(state.v0 ^ state.v1 ^ state.v2 ^ state.v3) diff --git a/hashes/src/util.rs b/hashes/src/util.rs index cf43e8b8..cf6729a6 100644 --- a/hashes/src/util.rs +++ b/hashes/src/util.rs @@ -68,7 +68,7 @@ macro_rules! borrow_slice_impl( macro_rules! engine_input_impl( () => ( - #[cfg(not(fuzzing))] + #[cfg(not(hashes_fuzz))] fn input(&mut self, mut inp: &[u8]) { while !inp.is_empty() { let buf_idx = self.length % ::BLOCK_SIZE; @@ -85,7 +85,7 @@ macro_rules! engine_input_impl( } } - #[cfg(fuzzing)] + #[cfg(hashes_fuzz)] fn input(&mut self, inp: &[u8]) { for c in inp { self.buffer[0] ^= *c; From 6649e1519305973a96d6d16e839711352c6044d8 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Fri, 28 Apr 2023 19:20:16 +0000 Subject: [PATCH 2/3] add README note explaining how to disable crypto for fuzzing --- fuzz/README.md | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/fuzz/README.md b/fuzz/README.md index 073457dc..f53a22e4 100644 --- a/fuzz/README.md +++ b/fuzz/README.md @@ -20,6 +20,29 @@ On Nix, you can obtain these libraries by running and then run fuzz.sh as above. +# Fuzzing with weak cryptography + +You may wish to replace the hashing and signing code with broken crypto, +which will be faster and enable the fuzzer to do otherwise impossible +things such as forging signatures or finding preimages to hashes. + +Doing so may result in spurious bug reports since the broken crypto does +not respect the encoding or algebraic invariants upheld by the real crypto. We +would like to improve this but it's a nontrivial problem -- though not +beyond the abilities of a motivated student with a few months of time. +Please let us know if you are interested in taking this on! + +Meanwhile, to use the broken crypto, simply compile (and run the fuzzing +scripts) with + + RUSTFLAGS="--cfg=hashes_fuzz --cfg=secp256k1_fuzz" + +which will replace the hashing library with broken hashes, and the +secp256k1 library with broken cryptography. + +Needless to say, NEVER COMPILE REAL CODE WITH THESE FLAGS because if a +fuzzer can break your crypto, so can anybody. + # Long-term fuzzing To see the full list of targets, the most straightforward way is to run @@ -85,9 +108,8 @@ The final line is a hex-encoded version of the input that caused the crash. You can test this directly by editing the `duplicate_crash` test to copy/paste the hex output into the call to `extend_vec_from_hex`. Then run the test with - RUSTFLAGS=--cfg=fuzzing cargo test + cargo test -It is important to add the `cfg=fuzzing` flag, which tells rustc to compile the -library as though it were running a fuzztest. In particular, this will disable -or weaken all the cryptography. +Note that if you set your `RUSTFLAGS` while fuzzing (see above) you must make +sure they are set the same way when running `cargo test`. From ab4a48c8bae04589d13c09f267704005a9156262 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 1 May 2023 21:29:44 +0000 Subject: [PATCH 3/3] ci: use new fuzzing cfg flags when fuzzing bitcoin (but not hashes) --- .github/workflows/fuzz.yml | 1 + fuzz/Cargo.toml | 2 +- fuzz/generate-files.sh | 3 ++- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml index 72465588..f67f25c9 100644 --- a/.github/workflows/fuzz.yml +++ b/.github/workflows/fuzz.yml @@ -53,6 +53,7 @@ hashes_sha1, override: true profile: minimal - name: fuzz + run: if [[ "${{ matrix.fuzz_target }}" =~ ^bitcoin ]]; then export RUSTFLAGS='--cfg=hashes_fuzz --cfg=secp256k1_fuzz'; fi run: cd fuzz && ./fuzz.sh "${{ matrix.fuzz_target }}" - run: echo "${{ matrix.fuzz_target }}" >executed_${{ matrix.fuzz_target }} - uses: actions/upload-artifact@v2 diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 9ca4340d..8728202c 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -9,7 +9,7 @@ publish = false cargo-fuzz = true [dependencies] -honggfuzz = { version = "0.5", default-features = false } +honggfuzz = { version = "0.5.55", default-features = false } bitcoin = { version = "0.30.0", features = [ "serde" ] } serde = { version = "1.0.103", features = [ "derive" ] } diff --git a/fuzz/generate-files.sh b/fuzz/generate-files.sh index 46b3a6a1..acb598c8 100755 --- a/fuzz/generate-files.sh +++ b/fuzz/generate-files.sh @@ -20,7 +20,7 @@ publish = false cargo-fuzz = true [dependencies] -honggfuzz = { version = "0.5", default-features = false } +honggfuzz = { version = "0.5.55", default-features = false } bitcoin = { version = "0.30.0", features = [ "serde" ] } serde = { version = "1.0.103", features = [ "derive" ] } @@ -78,6 +78,7 @@ $(for name in $(listTargetNames); do echo "$name,"; done) override: true profile: minimal - name: fuzz + run: if [[ "\${{ matrix.fuzz_target }}" =~ ^bitcoin ]]; then export RUSTFLAGS='--cfg=hashes_fuzz --cfg=secp256k1_fuzz'; fi run: cd fuzz && ./fuzz.sh "\${{ matrix.fuzz_target }}" - run: echo "\${{ matrix.fuzz_target }}" >executed_\${{ matrix.fuzz_target }} - uses: actions/upload-artifact@v2