From 6347db6a8e5a292befe9da9c565bd3ad70a165ad Mon Sep 17 00:00:00 2001
From: Michael Sproul <michael@sigmaprime.io>
Date: Fri, 7 Jun 2024 14:36:46 +1000
Subject: [PATCH] Type check fuzzers on CI and fix another bug (#41)

---
 .github/workflows/test-suite.yml |  6 ++++++
 fuzz/fuzz_targets/builder.rs     | 23 ++++++++++++++++++-----
 fuzz/fuzz_targets/list.rs        | 20 ++++++++++++++------
 src/list.rs                      |  6 ++++++
 4 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/.github/workflows/test-suite.yml b/.github/workflows/test-suite.yml
index 6edb232..3730049 100644
--- a/.github/workflows/test-suite.yml
+++ b/.github/workflows/test-suite.yml
@@ -19,6 +19,8 @@ jobs:
       run: rustup update stable
     - name: Check formatting with cargo fmt
       run: cargo fmt --all -- --check
+    - name: Check fuzzer formatting with cargo fmt
+      run: cargo fmt --manifest-path fuzz/Cargo.toml --all --check
   clippy:
     name: clippy
     runs-on: ubuntu-latest
@@ -38,7 +40,11 @@ jobs:
     - uses: actions/checkout@v3
     - name: Get latest version of stable Rust
       run: rustup update stable
+    - name: Get latest version of nightly Rust
+      run: rustup update nightly
     - name: Run tests
       run: cargo test --release
     - name: Check all examples, binaries, etc
       run: cargo check --all-targets
+    - name: Check fuzzer targets
+      run: cargo +nightly check --manifest-path fuzz/Cargo.toml --all-targets
diff --git a/fuzz/fuzz_targets/builder.rs b/fuzz/fuzz_targets/builder.rs
index a849746..a4ffb21 100644
--- a/fuzz/fuzz_targets/builder.rs
+++ b/fuzz/fuzz_targets/builder.rs
@@ -5,17 +5,30 @@ use milhouse::builder::Builder;
 
 fuzz_target!(|data: &[u8]| {
     // We use the first byte as 'depth'
-    if data.len() < 1 {return}
+    if data.len() < 1 {
+        return;
+    }
     let depth = data[0];
 
     let data = &data[1..];
     let length = data.len();
-    if length > 256 {return}
+    if length > 256 {
+        return;
+    }
 
-    let mut builder = Builder::<u8>::new(depth as usize, 0);
+    let Ok(mut builder) = Builder::<u8>::new(depth as usize, 0) else {
+        return;
+    };
     for i in 0..length {
-        builder.push(data[i]);
+        if builder.push(data[i]).is_err() {
+            assert!(i > (1 << depth));
+        }
     }
 
-    let Ok((arc_tree, depth, len)) = builder.finish() else {return};
+    let Ok((arc_tree, tree_depth, len)) = builder.finish() else {
+        return;
+    };
+    assert_eq!(tree_depth, depth as usize);
+    assert!(len.as_usize() <= length);
+    assert_eq!(arc_tree.compute_len(), len.as_usize());
 });
diff --git a/fuzz/fuzz_targets/list.rs b/fuzz/fuzz_targets/list.rs
index 19975e8..534704a 100644
--- a/fuzz/fuzz_targets/list.rs
+++ b/fuzz/fuzz_targets/list.rs
@@ -2,20 +2,28 @@
 
 use libfuzzer_sys::fuzz_target;
 use milhouse::List;
-use typenum::{Unsigned, U16, U32};
+use typenum::U16;
 
 fuzz_target!(|data: &[u8]| {
     let length = data.len();
-    if length > 256 {return}
+    if length > 256 {
+        return;
+    }
 
     // Create list using .push()
     let mut list1 = List::<u8, U16>::empty();
     for i in 0..length {
-        list1.push(data[i]);
+        if list1.push(data[i]).is_err() {
+            assert!(i >= 16);
+        }
     }
-    
-    list1.apply_updates();
+
+    list1.apply_updates().unwrap();
 
     // Create list using iterator
-    let list2 = List::<u8, U16>::try_from_iter(data.iter().copied());
+    let Ok(list2) = List::<u8, U16>::try_from_iter(data.iter().copied()) else {
+        return;
+    };
+
+    assert_eq!(list1, list2);
 });
diff --git a/src/list.rs b/src/list.rs
index bb20dc6..f3d8676 100644
--- a/src/list.rs
+++ b/src/list.rs
@@ -86,6 +86,12 @@ impl<T: Value, N: Unsigned, U: UpdateMap<T>> List<T, N, U> {
 
         let (tree, depth, length) = builder.finish()?;
 
+        // Check the length to cover the case where the capacity implied by packing_depth is
+        // greater than N. E.g. the builder might pack up to 32 u8s, even if N is < 32.
+        if length.as_usize() > N::to_usize() {
+            return Err(Error::BuilderFull);
+        }
+
         Ok(Self::from_parts(tree, depth, length))
     }