diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 79bf87c..59d329b 100644 --- a/.github/workflows/rust.yml +++ b/.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 diff --git a/CHANGELOG.md b/CHANGELOG.md index e6f51a5..0840ffc 100644 --- a/CHANGELOG.md +++ b/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` diff --git a/Cargo.toml b/Cargo.toml index 32df553..91af290 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "sharks" -version = "0.4.0" +version = "0.4.1" authors = ["Aitor Ruano "] 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" diff --git a/fuzz/.gitignore b/fuzz/.gitignore new file mode 100644 index 0000000..572e03b --- /dev/null +++ b/fuzz/.gitignore @@ -0,0 +1,4 @@ + +target +corpus +artifacts diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml new file mode 100644 index 0000000..f7e4f39 --- /dev/null +++ b/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" diff --git a/fuzz/fuzz_targets/deserialize_share.rs b/fuzz/fuzz_targets/deserialize_share.rs new file mode 100644 index 0000000..74be5d1 --- /dev/null +++ b/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); +}); diff --git a/fuzz/fuzz_targets/generate_shares.rs b/fuzz/fuzz_targets/generate_shares.rs new file mode 100644 index 0000000..98a6f9e --- /dev/null +++ b/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, + pub n_shares: usize, +} + +fuzz_target!(|params: Parameters| { + let sharks = Sharks(params.threshold); + let dealer = sharks.dealer(¶ms.secret); + + let _shares: Vec = dealer.take(params.n_shares).collect(); +}); diff --git a/fuzz/fuzz_targets/recover.rs b/fuzz/fuzz_targets/recover.rs new file mode 100644 index 0000000..1bdc33f --- /dev/null +++ b/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, +} + +fuzz_target!(|params: Parameters| { + let sharks = Sharks(params.threshold); + let _secret = sharks.recover(¶ms.shares); +}); diff --git a/fuzz/fuzz_targets/serialize_share.rs b/fuzz/fuzz_targets/serialize_share.rs new file mode 100644 index 0000000..e45fe95 --- /dev/null +++ b/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 = (&share).into(); +}); diff --git a/src/field.rs b/src/field.rs index d707947..f2d2d8c 100644 --- a/src/field.rs +++ b/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)] diff --git a/src/lib.rs b/src/lib.rs index 45bfa73..e32f44e 100644 --- a/src/lib.rs +++ b/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, T::IntoIter: Iterator, { - let (keys, shares) = shares - .into_iter() - .map(|s| { - ( - s.x.0, - Share { - x: s.x, - y: s.y.clone(), - }, - ) - }) - .unzip::, Vec>(); + let mut share_length: Option = None; + let mut keys: HashSet = HashSet::new(); + let mut values: Vec = 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())) } } } diff --git a/src/share.rs b/src/share.rs index 31e8b61..7ca298d 100644 --- a/src/share.rs +++ b/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) {} /// # fn ask_shares() -> Vec> {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> = ask_shares(); -/// let shares: Vec = shares_bytes.iter().map(|s| Share::from(s.as_slice())).collect(); +/// let shares: Vec = 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, @@ -42,11 +47,17 @@ impl From<&Share> for Vec { } /// 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 { + 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)]); }