Merge rust-bitcoin#4713: Update deserialize_witness fuzz target to use the Witness arbitrary impl

ca344e6 Update deserialize_witness fuzz target to use arbitrary impl (Shing Him Ng)
66f7a0a Rename `deserialize_amount` to `parse_amount` (Shing Him Ng)
180dcdc Update fuzz script to include `arbitrary` and `p2p` as a dependency (Shing Him Ng)

Pull request description:

  The result should be that the roundtrip logic is tested more often due to `arbitrary` yielding more valid Witnesses versus the previous case, which attempted to deserialize random bytes into a witness. IIUC this should also allow the fuzzer to find edge cases more easily as well


ACKs for top commit:
  apoelstra:
    ACK ca344e6; successfully ran local tests


Tree-SHA512: dec81a2caaadcd11989da9153228a8ad027b365ad6d6a8811b3555df272edc50209d3769fd19df95e0ee5c8776307166cbf841f4c56f0046a2942e213d76f8f5

Author: apoelstra

.github/workflows/cron-daily-fuzz.yml

@@ -18,24 +18,24 @@ jobs:
         # We only get 20 jobs at a time, we probably don't want to go
         # over that limit with fuzzing because of the hour run time.
         fuzz_target: [
+          bitcoin_deser_net_msg,
           bitcoin_deserialize_address,
           bitcoin_deserialize_block,
           bitcoin_deserialize_prefilled_transaction,
           bitcoin_deserialize_psbt,
           bitcoin_deserialize_script,
           bitcoin_deserialize_transaction,
           bitcoin_deserialize_witness,
-          bitcoin_deser_net_msg,
           bitcoin_outpoint_string,
           bitcoin_p2p_address_roundtrip,
           bitcoin_script_bytes_to_asm_fmt,
           hashes_json,
           hashes_ripemd160,
           hashes_sha1,
           hashes_sha256,
-          hashes_sha512_256,
           hashes_sha512,
-          units_deserialize_amount,
+          hashes_sha512_256,
+          units_parse_amount,
         ]
     steps:
       - name: Install test dependencies

Cargo-minimal.lock

@@ -76,6 +76,7 @@ version = "0.0.0"
 name = "bitcoin-fuzz"
 version = "0.0.1"
 dependencies = [
+ "arbitrary",
  "bitcoin",
  "bitcoin-p2p-messages",
  "honggfuzz",

Cargo-recent.lock

@@ -75,6 +75,7 @@ version = "0.0.0"
 name = "bitcoin-fuzz"
 version = "0.0.1"
 dependencies = [
+ "arbitrary",
  "bitcoin",
  "bitcoin-p2p-messages",
  "honggfuzz",

fuzz/Cargo.toml

@@ -1,6 +1,7 @@
 [package]
 name = "bitcoin-fuzz"
 edition = "2021"
+rust-version = "1.63.0"
 version = "0.0.1"
 authors = ["Generated by fuzz/generate-files.sh"]
 publish = false
@@ -10,15 +11,20 @@ cargo-fuzz = true
 
 [dependencies]
 honggfuzz = { version = "0.5.56", default-features = false }
-bitcoin = { path = "../bitcoin", features = [ "serde" ] }
+bitcoin = { path = "../bitcoin", features = [ "serde", "arbitrary" ] }
 p2p = { path = "../p2p", package = "bitcoin-p2p-messages" }
+arbitrary = { version = "1.4" }
 
 serde = { version = "1.0.103", features = [ "derive" ] }
 serde_json = "1.0"
 
 [lints.rust]
 unexpected_cfgs = { level = "deny", check-cfg = ['cfg(fuzzing)'] }
 
+[[bin]]
+name = "bitcoin_deser_net_msg"
+path = "fuzz_targets/bitcoin/deser_net_msg.rs"
+
 [[bin]]
 name = "bitcoin_deserialize_address"
 path = "fuzz_targets/bitcoin/deserialize_address.rs"
@@ -47,14 +53,14 @@ path = "fuzz_targets/bitcoin/deserialize_transaction.rs"
 name = "bitcoin_deserialize_witness"
 path = "fuzz_targets/bitcoin/deserialize_witness.rs"
 
-[[bin]]
-name = "bitcoin_deser_net_msg"
-path = "fuzz_targets/bitcoin/deser_net_msg.rs"
-
 [[bin]]
 name = "bitcoin_outpoint_string"
 path = "fuzz_targets/bitcoin/outpoint_string.rs"
 
+[[bin]]
+name = "bitcoin_p2p_address_roundtrip"
+path = "fuzz_targets/bitcoin/p2p_address_roundtrip.rs"
+
 [[bin]]
 name = "bitcoin_script_bytes_to_asm_fmt"
 path = "fuzz_targets/bitcoin/script_bytes_to_asm_fmt.rs"
@@ -75,18 +81,14 @@ path = "fuzz_targets/hashes/sha1.rs"
 name = "hashes_sha256"
 path = "fuzz_targets/hashes/sha256.rs"
 
-[[bin]]
-name = "hashes_sha512_256"
-path = "fuzz_targets/hashes/sha512_256.rs"
-
 [[bin]]
 name = "hashes_sha512"
 path = "fuzz_targets/hashes/sha512.rs"
 
 [[bin]]
-name = "units_deserialize_amount"
-path = "fuzz_targets/units/deserialize_amount.rs"
+name = "hashes_sha512_256"
+path = "fuzz_targets/hashes/sha512_256.rs"
 
 [[bin]]
-name = "bitcoin_p2p_address_roundtrip"
-path = "fuzz_targets/bitcoin/p2p_address_roundtrip.rs"
+name = "units_parse_amount"
+path = "fuzz_targets/units/parse_amount.rs"

fuzz/fuzz-util.sh

@@ -1,5 +1,9 @@
 #!/usr/bin/env bash
 
+# Sort order is affected by locale. See `man sort`.
+# > Set LC_ALL=C to get the traditional sort order that uses native byte values.
+export LC_ALL=C
+
 REPO_DIR=$(git rev-parse --show-toplevel)
 
 listTargetFiles() {

fuzz/fuzz_targets/bitcoin/deserialize_witness.rs

@@ -1,12 +1,18 @@
 use bitcoin::consensus::{deserialize, serialize};
 use bitcoin::witness::Witness;
 use honggfuzz::fuzz;
+use arbitrary::{Arbitrary, Unstructured};
 
 fn do_test(data: &[u8]) {
-    let w: Result<Witness, _> = deserialize(data);
+    let mut u = Unstructured::new(data);
+
+    let w = Witness::arbitrary(&mut u);
     if let Ok(witness) = w {
         let serialized = serialize(&witness);
-        assert_eq!(data, &serialized[..]);
+        let deserialized: Result<Witness, _> = deserialize(serialized.as_slice());
+
+        assert!(deserialized.is_ok());
+        assert_eq!(deserialized.unwrap(), witness);
     }
 }
 

…fuzz_targets/units/deserialize_amount.rs fuzz/fuzz_targets/units/parse_amount.rsfuzz/fuzz_targets/units/deserialize_amount.rs renamed to fuzz/fuzz_targets/units/parse_amount.rs fuzz/fuzz_targets/units/deserialize_amount.rs renamed to fuzz/fuzz_targets/units/parse_amount.rs

@@ -22,8 +22,10 @@ publish = false
 cargo-fuzz = true
 
 [dependencies]
-honggfuzz = { version = "0.5.55", default-features = false }
-bitcoin = { path = "../bitcoin", features = [ "serde" ] }
+honggfuzz = { version = "0.5.56", default-features = false }
+bitcoin = { path = "../bitcoin", features = [ "serde", "arbitrary" ] }
+p2p = { path = "../p2p", package = "bitcoin-p2p-messages" }
+arbitrary = { version = "1.4" }
 
 serde = { version = "1.0.103", features = [ "derive" ] }
 serde_json = "1.0"