Add fuzzing for payload and fix all the issues found. (#113)

This PR adds fuzzing via cargo fuzz to the payload module and adds fixes for
the issues that the fuzzer uncovered.

Cargo.lock

@@ -80,6 +80,15 @@ dependencies = [
  "windows-sys 0.52.0",
 ]
 
+[[package]]
+name = "arbitrary"
+version = "1.3.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7d5a26814d8dcb93b0e5a0ff3c6d80a8843bafb21b39e8e18a6f05471870e110"
+dependencies = [
+ "derive_arbitrary",
+]
+
 [[package]]
 name = "arc-swap"
 version = "1.7.1"
@@ -166,6 +175,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "a21f936df1771bf62b77f047b726c4625ff2e8aa607c01ec06e5a05bd8463401"
 dependencies = [
  "android-tzdata",
+ "arbitrary",
  "iana-time-zone",
  "js-sys",
  "num-traits",
@@ -266,6 +276,17 @@ dependencies = [
  "powerfmt",
 ]
 
+[[package]]
+name = "derive_arbitrary"
+version = "1.3.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "67e77553c4162a157adbf834ebae5b415acbecbeafc7a74b0e886657506a7611"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
 [[package]]
 name = "either"
 version = "1.11.0"
@@ -887,6 +908,7 @@ version = "0.17.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "98a05b958a41ba8c923cf14bd2ad5f1aca3f3509c8ffd147c36e094346a0290b"
 dependencies = [
+ "arbitrary",
  "base64",
  "bcder",
  "bytes",
@@ -906,6 +928,7 @@ dependencies = [
 name = "rtrtr"
 version = "0.3.0-dev"
 dependencies = [
+ "arbitrary",
  "arc-swap",
  "bytes",
  "chrono",

Cargo.toml

@@ -11,6 +11,7 @@ license = "BSD-3-Clause"
 readme = "README.md"
 
 [dependencies]
+arbitrary       = { version = "1", optional = true, features = ["derive"] }
 arc-swap        = "1.0"
 bytes           = "1"
 chrono          = "0.4.31"
@@ -38,6 +39,7 @@ webpki-roots    = "0.25.2"
 
 [features]
 default = [ "socks" ]
+arbitrary = [ "dep:arbitrary", "chrono/arbitrary", "rpki/arbitrary" ]
 socks = [ "reqwest/socks" ]
 
 

fuzz/.gitignore

@@ -0,0 +1,4 @@
+target
+corpus
+artifacts
+coverage

fuzz/Cargo.toml

@@ -0,0 +1,40 @@
+[package]
+name = "rtrtr-fuzz"
+version = "0.0.0"
+publish = false
+edition = "2021"
+
+[package.metadata]
+cargo-fuzz = true
+
+[dependencies]
+libfuzzer-sys = "0.4"
+rpki = { version = "*", features = [ "arbitrary" ] }
+
+[dependencies.rtrtr]
+path = ".."
+
+# Prevent this from interfering with workspaces
+[workspace]
+members = ["."]
+
+[profile.release]
+debug = 1
+
+[[bin]]
+name = "payload_set_builder"
+path = "fuzz_targets/payload_set_builder.rs"
+test = false
+doc = false
+
+[[bin]]
+name = "payload_set_merge"
+path = "fuzz_targets/payload_set_merge.rs"
+test = false
+doc = false
+
+[[bin]]
+name = "payload_set_filter"
+path = "fuzz_targets/payload_set_filter.rs"
+test = false
+doc = false

fuzz/fuzz_targets/payload_set_builder.rs

@@ -0,0 +1,25 @@
+#![no_main]
+
+use std::collections::HashSet;
+use libfuzzer_sys::fuzz_target;
+use rpki::rtr::Payload;
+use rtrtr::payload::{SetBuilder, PackBuilder};
+
+fuzz_target!(|data: Vec<Vec<Payload>> | {
+    let mut builder = SetBuilder::empty();
+    let mut payload = HashSet::<Payload>::default();
+    for item in data {
+        let mut pack = PackBuilder::empty();
+        for item in item {
+            if pack.insert(item.clone()).is_ok() {
+                payload.insert(item);
+            }
+        }
+        builder.insert_pack(pack.finalize())
+    }
+    let set = builder.finalize();
+    for item in set.iter() {
+        assert!(payload.remove(item));
+    }
+    assert!(payload.is_empty());
+});

fuzz/fuzz_targets/payload_set_filter.rs

@@ -0,0 +1,35 @@
+#![no_main]
+
+use std::collections::HashSet;
+use libfuzzer_sys::fuzz_target;
+use rpki::rtr::Payload;
+use rtrtr::payload::{SetBuilder, PackBuilder};
+
+fuzz_target!(|data: (Vec<Vec<Payload>>, HashSet<Payload>)| {
+    // Make a set from the first item in data.
+    let mut builder = SetBuilder::empty();
+    let mut payload = HashSet::<Payload>::default();
+    for item in data.0 {
+        let mut pack = PackBuilder::empty();
+        for item in item {
+            if pack.insert(item.clone()).is_ok() {
+                payload.insert(item.clone());
+            }
+        }
+        builder.insert_pack(pack.finalize())
+    }
+    let set = builder.finalize();
+ 
+    // Now filter everything in data.1 out of both set and payload.
+    let filtered_set = set.filter(|item| data.1.contains(item));
+    let mut filtered_payload =
+        payload.intersection(&data.1).cloned().collect::<Vec<_>>();
+    filtered_payload.sort();
+    
+    let eq = filtered_set.iter().eq(filtered_payload.iter());
+    if !eq {
+        eprintln!("filtered_set: {:#?}", filtered_set);
+        eprintln!("filtered_payload: {:#?}", filtered_payload);
+    }
+    assert!(eq);
+});

fuzz/fuzz_targets/payload_set_merge.rs

@@ -0,0 +1,50 @@
+#![no_main]
+#![feature(is_sorted)]
+
+use std::collections::HashSet;
+use libfuzzer_sys::fuzz_target;
+use rpki::rtr::Payload;
+use rtrtr::payload::{Set, SetBuilder, PackBuilder};
+
+fn make_set(data: &Vec<Vec<Payload>>) -> (Set, HashSet<Payload>) {
+    let mut builder = SetBuilder::empty();
+    let mut payload = HashSet::<Payload>::default();
+    for item in data {
+        let mut pack = PackBuilder::empty();
+        for item in item {
+            if pack.insert(item.clone()).is_ok() {
+                payload.insert(item.clone());
+            }
+        }
+        builder.insert_pack(pack.finalize())
+    }
+    let set = builder.finalize();
+    (set, payload)
+}
+
+fuzz_target!(|data: (Vec<Vec<Payload>>, Vec<Vec<Payload>>)| {
+    let (left_set, left_hash) = make_set(&data.0);
+    let (right_set, right_hash) = make_set(&data.1);
+
+    // Check that the sets are in correct order.
+    // Iterator::is_sorted is unstable, but we have to use nightly anyway.
+    assert!(left_set.iter().is_sorted());
+    assert!(right_set.iter().is_sorted());
+
+    let merged_set = left_set.merge(&right_set);
+    let mut merged_vec = left_hash.union(
+        &right_hash
+    ).cloned().collect::<Vec<_>>();
+    merged_vec.sort();
+    let eq = merged_set.iter().eq(merged_vec.iter());
+    if !eq {
+        eprintln!("Left set: {:#?}", data.0);
+        eprintln!("Right set: {:#?}", data.1);
+        eprintln!("Result: {:#?}", merged_set);
+        eprintln!("Result iterated: {:#?}",
+            merged_set.iter().collect::<Vec<_>>()
+        );
+        eprintln!("Result set: {:#?}", merged_vec);
+    }
+    assert!(eq);
+});

src/payload.rs

@@ -47,7 +47,7 @@
 //! base type yet returns references to the items. For now, these need to
 //! separate because the `Iterator` trait requires the returned items to have
 //! the same lifetime as the iterator type itself. 
-use std::{mem, slice};
+use std::slice;
 use std::borrow::Borrow;
 use std::cmp::Ordering;
 use std::collections::HashSet;
@@ -67,6 +67,7 @@ use rpki::rtr::server::{PayloadDiff, PayloadSet};
 /// A pack always keeps the payload in sorted order. Once created, it cannot
 /// be changed anymore.
 #[derive(Clone, Debug, Eq, PartialEq)]
+#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
 pub struct Pack {
     /// The payload items.
     items: Arc<[Payload]>,
@@ -140,6 +141,7 @@ impl Borrow<[Payload]> for Pack {
 
 /// A builder for a payload pack.
 #[derive(Clone, Debug, Default)]
+#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
 pub struct PackBuilder {
     items: HashSet<Payload>,
 }
@@ -210,6 +212,7 @@ impl PackBuilder {
 ///
 /// A block references a slice of a [`Pack`]’s items.
 #[derive(Clone, Debug)]
+#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
 pub struct Block {
     pack: Pack,
     range: Range<usize>,
@@ -396,6 +399,7 @@ impl OwnedBlockIter {
 
 /// An ordered set of payload items.
 #[derive(Clone, Debug)]
+#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
 pub struct Set {
     /// The blocks of the set.
     blocks: Arc<[Block]>,
@@ -430,7 +434,7 @@ impl Set {
         OwnedSetIter::new(self)
     }
 
-    /// Returns a set with the indicated elements removed.
+    /// Returns a set with only the indicated elements included.
     ///
     /// Each element in the current set is presented to the closure and only
     /// those for which the closure returns `true` are added to the returned
@@ -511,16 +515,18 @@ impl Set {
                 }
                 right_head = right_tail.next();
             };
-            let (left, right) = match (left, right) {
-                (Some(left), Some(right)) => (left, right),
-                _ => break,
-            };
 
             // Make left the block that starts first. Since neither block is
             // empty, we can unwrap.
-            if right.first().unwrap() < left.first().unwrap() {
-                mem::swap(left, right);
-            }
+            let (left, right) = match (left, right) {
+                (Some(left), Some(right))
+                    if right.first().unwrap() < left.first().unwrap() =>
+                {
+                    (right, left)
+                }
+                (Some(left), Some(right)) => (left, right),
+                _ => break,
+            };
 
             // Find out how much of left we can add.
             //
@@ -806,6 +812,7 @@ impl PayloadSet for OwnedSetIter {
 
 /// A builder for a set.
 #[derive(Clone, Debug, Default)]
+#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
 pub struct SetBuilder {
     blocks: Vec<Block>,
 }
@@ -950,6 +957,7 @@ impl SetBuilder {
 /// as a single list of pairs of [`Payload`] and [`Action`]s in order of the
 /// payload. This makes it relatively safe to apply non-atomically.
 #[derive(Clone, Debug, Default, Eq, PartialEq)]
+#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
 pub struct Diff {
     /// A set of announced elements.
     announced: Pack,
@@ -1153,6 +1161,7 @@ impl PayloadDiff for OwnedDiffIter {
 
 /// A builder for a diff.
 #[derive(Clone, Debug, Default)]
+#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
 pub struct DiffBuilder {
     announced: PackBuilder,
     withdrawn: PackBuilder,
@@ -1237,6 +1246,7 @@ impl DiffBuilder {
 
 /// An update of a unit’s payload data.
 #[derive(Clone, Debug, Eq, PartialEq)]
+#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
 pub struct Update {
     /// The new payload set.
     set: Set,