From 830a6e1b0c57cc8b3a7a60fb0e1a61c1a4073654 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Fri, 24 May 2024 14:31:09 +0000 Subject: [PATCH] fuzz: delete CBOR test We were using an outdated CBOR crate for MSRV reasons. But this old crate is causing suprious test failures. So delete it. (Sadly, updating the crate doesn't fix the issue, replacing it with ciborium breaks our MSRV tests because it needs a more recent `half` dependency, and replacing it with `minicbor` doesn't work because minicbor is not based on serde. So we don't really have any options.) In general, I am suspicious of this decode-then-reencode test. CBOR has some ambiguity in integer encoding. Empirically it has seemed to work for a long time, but this seems more like an indictment of our test than a positive result. Also, round-trip testing serde encoding of a byte vector is probably not a great use of our fuzz resources. I don't believe we have ever had a problem with this. Fixes #2801 --- .github/workflows/cron-daily-fuzz.yml | 1 - Cargo-minimal.lock | 18 --------- Cargo-recent.lock | 24 ------------ fuzz/Cargo.toml | 5 --- fuzz/fuzz_targets/hashes/cbor.rs | 56 --------------------------- fuzz/generate-files.sh | 1 - 6 files changed, 105 deletions(-) delete mode 100644 fuzz/fuzz_targets/hashes/cbor.rs diff --git a/.github/workflows/cron-daily-fuzz.yml b/.github/workflows/cron-daily-fuzz.yml index 639869077..2686f70ff 100644 --- a/.github/workflows/cron-daily-fuzz.yml +++ b/.github/workflows/cron-daily-fuzz.yml @@ -28,7 +28,6 @@ jobs: bitcoin_deser_net_msg, bitcoin_outpoint_string, bitcoin_script_bytes_to_asm_fmt, - hashes_cbor, hashes_json, hashes_ripemd160, hashes_sha1, diff --git a/Cargo-minimal.lock b/Cargo-minimal.lock index cbc17e227..1784c15c2 100644 --- a/Cargo-minimal.lock +++ b/Cargo-minimal.lock @@ -75,7 +75,6 @@ dependencies = [ "bitcoin", "honggfuzz", "serde", - "serde_cbor", "serde_json", ] @@ -156,12 +155,6 @@ dependencies = [ "wasi", ] -[[package]] -name = "half" -version = "1.7.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "62aca2aba2d62b4a7f5b33f3712cb1b0692779a56fb510499d5c0aa594daeaf3" - [[package]] name = "hex-conservative" version = "0.2.0" @@ -386,17 +379,6 @@ dependencies = [ "serde_derive", ] -[[package]] -name = "serde_cbor" -version = "0.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "45cd6d95391b16cd57e88b68be41d504183b7faae22030c0cc3b3f73dd57b2fd" -dependencies = [ - "byteorder", - "half", - "serde", -] - [[package]] name = "serde_derive" version = "1.0.156" diff --git a/Cargo-recent.lock b/Cargo-recent.lock index 2bd141b03..b8082677c 100644 --- a/Cargo-recent.lock +++ b/Cargo-recent.lock @@ -74,7 +74,6 @@ dependencies = [ "bitcoin", "honggfuzz", "serde", - "serde_cbor", "serde_json", ] @@ -120,12 +119,6 @@ dependencies = [ "cc", ] -[[package]] -name = "byteorder" -version = "1.4.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610" - [[package]] name = "cc" version = "1.0.79" @@ -155,12 +148,6 @@ dependencies = [ "wasi", ] -[[package]] -name = "half" -version = "1.7.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "62aca2aba2d62b4a7f5b33f3712cb1b0692779a56fb510499d5c0aa594daeaf3" - [[package]] name = "hex-conservative" version = "0.2.0" @@ -375,17 +362,6 @@ dependencies = [ "serde_derive", ] -[[package]] -name = "serde_cbor" -version = "0.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "45cd6d95391b16cd57e88b68be41d504183b7faae22030c0cc3b3f73dd57b2fd" -dependencies = [ - "byteorder", - "half", - "serde", -] - [[package]] name = "serde_derive" version = "1.0.156" diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 51d639861..cd48ead1c 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -15,7 +15,6 @@ bitcoin = { path = "../bitcoin", features = [ "serde" ] } serde = { version = "1.0.103", features = [ "derive" ] } serde_json = "1.0" -serde_cbor = "0.9" [lints.rust] unexpected_cfgs = { level = "deny", check-cfg = ['cfg(fuzzing)'] } @@ -60,10 +59,6 @@ path = "fuzz_targets/bitcoin/outpoint_string.rs" name = "bitcoin_script_bytes_to_asm_fmt" path = "fuzz_targets/bitcoin/script_bytes_to_asm_fmt.rs" -[[bin]] -name = "hashes_cbor" -path = "fuzz_targets/hashes/cbor.rs" - [[bin]] name = "hashes_json" path = "fuzz_targets/hashes/json.rs" diff --git a/fuzz/fuzz_targets/hashes/cbor.rs b/fuzz/fuzz_targets/hashes/cbor.rs deleted file mode 100644 index 29287577d..000000000 --- a/fuzz/fuzz_targets/hashes/cbor.rs +++ /dev/null @@ -1,56 +0,0 @@ -use bitcoin::hashes::{ripemd160, sha1, sha256d, sha512, Hmac}; -use honggfuzz::fuzz; -use serde::{Deserialize, Serialize}; - -#[derive(Deserialize, Serialize)] -struct Hmacs { - sha1: Hmac, - sha512: Hmac, -} - -#[derive(Deserialize, Serialize)] -struct Main { - hmacs: Hmacs, - ripemd: ripemd160::Hash, - sha2d: sha256d::Hash, -} - -fn do_test(data: &[u8]) { - if let Ok(m) = serde_cbor::from_slice::
(data) { - let vec = serde_cbor::to_vec(&m).unwrap(); - assert_eq!(data, &vec[..]); - } -} - -fn main() { - loop { - fuzz!(|d| { do_test(d) }); - } -} - -#[cfg(all(test, fuzzing))] -mod tests { - fn extend_vec_from_hex(hex: &str, out: &mut Vec) { - let mut b = 0; - for (idx, c) in hex.as_bytes().iter().enumerate() { - b <<= 4; - match *c { - b'A'..=b'F' => b |= c - b'A' + 10, - b'a'..=b'f' => b |= c - b'a' + 10, - b'0'..=b'9' => b |= c - b'0', - _ => panic!("Bad hex"), - } - if (idx & 1) == 1 { - out.push(b); - b = 0; - } - } - } - - #[test] - fn duplicate_crash() { - let mut a = Vec::new(); - extend_vec_from_hex("00000", &mut a); - super::do_test(&a); - } -} diff --git a/fuzz/generate-files.sh b/fuzz/generate-files.sh index bead190b4..74aeacc2b 100755 --- a/fuzz/generate-files.sh +++ b/fuzz/generate-files.sh @@ -27,7 +27,6 @@ bitcoin = { path = "../bitcoin", features = [ "serde" ] } serde = { version = "1.0.103", features = [ "derive" ] } serde_json = "1.0" -serde_cbor = "0.9" [lints.rust] unexpected_cfgs = { level = "deny", check-cfg = ['cfg(fuzzing)'] }