Add fuzzers (#14)

Improved lib stability using fuzzers for testing

.github/workflows/rust.yml

@@ -18,7 +18,7 @@ jobs:
           command: check
 
   test-std:
-    name: Test
+    name: Test std
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
@@ -32,7 +32,7 @@ jobs:
           command: test
 
   test-nostd:
-    name: Test
+    name: Test no-std
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2

CHANGELOG.md

@@ -4,6 +4,14 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.4.1] - 2020-04-23
+### Added
+- Fuzz tests
+
+### Fixed
+- Unexpected panic when trying to recover secret from different length shares
+- Unexpected panic when trying to convert less than 2 bytes to `Share`
+
 ## [0.4.0] - 2020-04-02
 ### Added
 - It is now possible to compile without `std` with `--no-default-features`

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "sharks"
-version = "0.4.0"
+version = "0.4.1"
 authors = ["Aitor Ruano <codearm@pm.me>"]
 description = "Fast, small and secure Shamir's Secret Sharing library crate"
 homepage = "https://github.com/c0dearm/sharks"
@@ -19,10 +19,12 @@ codecov = { repository = "c0dearm/sharks" }
 [features]
 default = ["std"]
 std = ["rand/std"]
+fuzzing = ["std", "arbitrary"]
 
 [dependencies]
 rand = { version = "0.7", default-features = false }
 hashbrown = "0.7"
+arbitrary = {version = "0.4.2", features = ["derive"], optional = true}
 
 [dev-dependencies]
 criterion = "0.3"

fuzz/.gitignore

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

fuzz/Cargo.toml

@@ -0,0 +1,38 @@
+
+[package]
+name = "sharks-fuzz"
+version = "0.0.0"
+authors = ["Automatically generated"]
+publish = false
+edition = "2018"
+
+[package.metadata]
+cargo-fuzz = true
+
+[dependencies]
+libfuzzer-sys = "0.3"
+arbitrary = { version = "0.4.2", features = ["derive"] }
+
+[dependencies.sharks]
+path = ".."
+features = ["fuzzing"]
+
+# Prevent this from interfering with workspaces
+[workspace]
+members = ["."]
+
+[[bin]]
+name = "deserialize_share"
+path = "fuzz_targets/deserialize_share.rs"
+
+[[bin]]
+name = "serialize_share"
+path = "fuzz_targets/serialize_share.rs"
+
+[[bin]]
+name = "generate_shares"
+path = "fuzz_targets/generate_shares.rs"
+
+[[bin]]
+name = "recover"
+path = "fuzz_targets/recover.rs"

fuzz/fuzz_targets/deserialize_share.rs

@@ -0,0 +1,8 @@
+#![no_main]
+use core::convert::TryFrom;
+use libfuzzer_sys::fuzz_target;
+use sharks::Share;
+
+fuzz_target!(|data: &[u8]| {
+    let _share = Share::try_from(data);
+});

fuzz/fuzz_targets/generate_shares.rs

@@ -0,0 +1,19 @@
+#![no_main]
+use libfuzzer_sys::fuzz_target;
+
+use arbitrary::Arbitrary;
+use sharks::{Share, Sharks};
+
+#[derive(Debug, Arbitrary)]
+struct Parameters {
+    pub threshold: u8,
+    pub secret: Vec<u8>,
+    pub n_shares: usize,
+}
+
+fuzz_target!(|params: Parameters| {
+    let sharks = Sharks(params.threshold);
+    let dealer = sharks.dealer(&params.secret);
+
+    let _shares: Vec<Share> = dealer.take(params.n_shares).collect();
+});

fuzz/fuzz_targets/recover.rs

@@ -0,0 +1,16 @@
+#![no_main]
+use libfuzzer_sys::fuzz_target;
+
+use arbitrary::Arbitrary;
+use sharks::{Share, Sharks};
+
+#[derive(Debug, Arbitrary)]
+struct Parameters {
+    pub threshold: u8,
+    pub shares: Vec<Share>,
+}
+
+fuzz_target!(|params: Parameters| {
+    let sharks = Sharks(params.threshold);
+    let _secret = sharks.recover(&params.shares);
+});

fuzz/fuzz_targets/serialize_share.rs

@@ -0,0 +1,8 @@
+#![no_main]
+use libfuzzer_sys::fuzz_target;
+
+use sharks::Share;
+
+fuzz_target!(|share: Share| {
+    let _data: Vec<u8> = (&share).into();
+});

src/field.rs

@@ -4,6 +4,9 @@
 use core::iter::{Product, Sum};
 use core::ops::{Add, Div, Mul, Sub};
 
+#[cfg(feature = "fuzzing")]
+use arbitrary::Arbitrary;
+
 const LOG_TABLE: [u8; 256] = [
     0x00, 0x00, 0x01, 0x19, 0x02, 0x32, 0x1a, 0xc6, 0x03, 0xdf, 0x33, 0xee, 0x1b, 0x68, 0xc7, 0x4b,
     0x04, 0x64, 0xe0, 0x0e, 0x34, 0x8d, 0xef, 0x81, 0x1c, 0xc1, 0x69, 0xf8, 0xc8, 0x08, 0x4c, 0x71,
@@ -59,6 +62,7 @@ const EXP_TABLE: [u8; 512] = [
 ];
 
 #[derive(Debug, PartialEq, Copy, Clone)]
+#[cfg_attr(feature = "fuzzing", derive(Arbitrary))]
 pub struct GF256(pub u8);
 
 #[allow(clippy::suspicious_arithmetic_impl)]

src/lib.rs

@@ -34,7 +34,7 @@
 //! let secret = sharks.recover(shares.as_slice()).unwrap();
 //! assert_eq!(secret, vec![1, 2, 3, 4]);
 //! ```
-#![no_std]
+#![cfg_attr(not(feature = "std"), no_std)]
 
 mod field;
 mod math;
@@ -143,23 +143,27 @@ impl Sharks {
         T: IntoIterator<Item = &'a Share>,
         T::IntoIter: Iterator<Item = &'a Share>,
     {
-        let (keys, shares) = shares
-            .into_iter()
-            .map(|s| {
-                (
-                    s.x.0,
-                    Share {
-                        x: s.x,
-                        y: s.y.clone(),
-                    },
-                )
-            })
-            .unzip::<u8, Share, HashSet<u8>, Vec<Share>>();
+        let mut share_length: Option<usize> = None;
+        let mut keys: HashSet<u8> = HashSet::new();
+        let mut values: Vec<Share> = Vec::new();
 
-        if keys.len() < self.0 as usize {
+        for share in shares.into_iter() {
+            if share_length.is_none() {
+                share_length = Some(share.y.len());
+            }
+
+            if Some(share.y.len()) != share_length {
+                return Err("All shares must have the same length");
+            } else {
+                keys.insert(share.x.0);
+                values.push(share.clone());
+            }
+        }
+
+        if keys.is_empty() || (keys.len() < self.0 as usize) {
             Err("Not enough shares to recover original secret")
         } else {
-            Ok(math::interpolate(shares.as_slice()))
+            Ok(math::interpolate(values.as_slice()))
         }
     }
 }

src/share.rs

@@ -2,12 +2,16 @@ use alloc::vec::Vec;
 
 use super::field::GF256;
 
+#[cfg(feature = "fuzzing")]
+use arbitrary::Arbitrary;
+
 /// A share used to reconstruct the secret. Can be serialized to and from a byte array.
 ///
 /// Usage example:
 /// ```
 /// use sharks::{Sharks, Share};
 /// # use rand_chacha::rand_core::SeedableRng;
+/// # use core::convert::TryFrom;
 /// # fn send_to_printer(_: Vec<u8>) {}
 /// # fn ask_shares() -> Vec<Vec<u8>> {vec![vec![1, 2], vec![2, 3], vec![3, 4]]}
 ///
@@ -23,9 +27,10 @@ use super::field::GF256;
 ///
 /// // Get share bytes from an external source and recover secret
 /// let shares_bytes: Vec<Vec<u8>> = ask_shares();
-/// let shares: Vec<Share> = shares_bytes.iter().map(|s| Share::from(s.as_slice())).collect();
+/// let shares: Vec<Share> = shares_bytes.iter().map(|s| Share::try_from(s.as_slice()).unwrap()).collect();
 /// let secret = sharks.recover(&shares).unwrap();
 #[derive(Clone)]
+#[cfg_attr(feature = "fuzzing", derive(Arbitrary, Debug))]
 pub struct Share {
     pub x: GF256,
     pub y: Vec<GF256>,
@@ -42,11 +47,17 @@ impl From<&Share> for Vec<u8> {
 }
 
 /// Obtains a `Share` instance from a byte slice
-impl From<&[u8]> for Share {
-    fn from(s: &[u8]) -> Share {
-        let x = GF256(s[0]);
-        let y = s[1..].iter().map(|p| GF256(*p)).collect();
-        Share { x, y }
+impl core::convert::TryFrom<&[u8]> for Share {
+    type Error = &'static str;
+
+    fn try_from(s: &[u8]) -> Result<Share, Self::Error> {
+        if s.len() < 2 {
+            Err("A Share must be at least 2 bytes long")
+        } else {
+            let x = GF256(s[0]);
+            let y = s[1..].iter().map(|p| GF256(*p)).collect();
+            Ok(Share { x, y })
+        }
     }
 }
 
@@ -54,6 +65,7 @@ impl From<&[u8]> for Share {
 mod tests {
     use super::{Share, GF256};
     use alloc::{vec, vec::Vec};
+    use core::convert::TryFrom;
 
     #[test]
     fn vec_from_share_works() {
@@ -68,7 +80,7 @@ mod tests {
     #[test]
     fn share_from_u8_slice_works() {
         let bytes = [1, 2, 3];
-        let share = Share::from(&bytes[..]);
+        let share = Share::try_from(&bytes[..]).unwrap();
         assert_eq!(share.x, GF256(1));
         assert_eq!(share.y, vec![GF256(2), GF256(3)]);
     }