Regardless of threshold, all polynomials are lines due to small syntactic error

[psivesely]

In the the SSS::secret_share function, the author clearly intended to make col_in an array of threshold bytes, but put a comma where the semi-colon should go in the vec! macro. Thus the code always generates just a single coefficient instead of threshold - 1 coefficients for our secret polynomial. So regardless of how high the threshold is set two shares are enough to uncover the secret.

This did not cause an error in the secret recovery code because of the fundamental uniqueness of the Lagrange polynomial: regardless of the number of nodes (shares) presented in excess of k + 1 for a k degree polynomial, Langrange interpolation finds the unique polynomial of degree k.

Here is an illustration of the problem:

extern crate rand;
use rand::{OsRng, Rng};

fn main() {
    let threshold: u8 = 6;
    let mut col_in = vec![0u8, threshold];
    let mut osrng = OsRng::new().unwrap();
    osrng.fill_bytes(&mut col_in[1..]);
    println!("{:?}", col_in);

    let threshold: u8 = 6;
    let mut col_in = vec![0u8; threshold as usize];
    osrng.fill_bytes(&mut col_in[1..]);
    println!("{:?}", col_in);
}

[0, 234]
[0, 196, 181, 63, 217, 112]

[romac]

Hi Noah,

Thank you so much for reporting this, that's a big one.

Fortunately, this bug seem to follow from a refactoring I did while working on the deterministic secret sharing scheme, and is thus not present in RustySecrets v0.0.2. As such, Sunder is thankfully not affected, given that the npm package it depends on actually uses that very same version of the library.

I will merge this as soon as I have time to write a good test to ensure we never make the same mistake in the future.

Thank you so much again for taking the time to go through the code and report this security issue. The DSS code is going to go under audit soon and we'll work towards improving the code coverage in tests. In the meantime, I want to stress that the code published under v0.0.2 has been audited already and does not suffer from this very issue.

Thanks again!
Romain

[romac]

@nvesely Your patch has been merged. I added a test in #44 to make sure we never encounter this very same issue in the future.

Thank you again for noticing this bug, reporting it, and fixing it :)