Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FeldmanVerifier::verify() panics if given a malformed Share #6

Open
jgallagher opened this issue Jun 6, 2022 · 2 comments
Open

FeldmanVerifier::verify() panics if given a malformed Share #6

jgallagher opened this issue Jun 6, 2022 · 2 comments

Comments

@jgallagher
Copy link

Hi! I was testing vsss-rs with arbitrary Shares, and expected this to pass (i.e., passing an empty share should not verify):

    let sk = SecretKey::random(&mut osrng);
    let nzs = sk.to_secret_scalar();
    let (_shares, verifier): (_, FeldmanVerifier<Scalar, ProjectivePoint>) = Feldman { t: 3, n: 5 }
        .split_secret(*nzs, None, &mut osrng)
        .unwrap();

    assert!(!verifier.verify(&Share(vec![])));

but it panics with an out of bounds slice access:

thread 'main' panicked at 'range start index 1 out of range for slice of length 0', /home/john/.cargo/registry/src/github.com-1ecc6299db9ec823/vsss-rs-2.0.0-pre2/src/standard/share.rs:60:10
stack backtrace:
   0: rust_begin_unwind
             at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/std/src/panicking.rs:498:5
   1: core::panicking::panic_fmt
             at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/panicking.rs:116:14
   2: core::slice::index::slice_start_index_len_fail
             at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/slice/index.rs:35:5
   3: <core::ops::range::RangeFrom<usize> as core::slice::index::SliceIndex<[T]>>::index
             at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/slice/index.rs:329:13
   4: core::slice::index::<impl core::ops::index::Index<I> for [T]>::index
             at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/slice/index.rs:15:9
   5: <alloc::vec::Vec<T,A> as core::ops::index::Index<I>>::index
             at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/alloc/src/vec/mod.rs:2520:9
   6: vsss_rs::standard::share::Share::value
             at /home/john/.cargo/registry/src/github.com-1ecc6299db9ec823/vsss-rs-2.0.0-pre2/src/standard/share.rs:60:10
   7: vsss_rs::standard::verifier::feldman::FeldmanVerifier<F,G>::verify
             at /home/john/.cargo/registry/src/github.com-1ecc6299db9ec823/vsss-rs-2.0.0-pre2/src/standard/verifier/feldman.rs:86:37
   8: vsss_rs_panic::main
             at ./src/main.rs:15:14
   9: core::ops::function::FnOnce::call_once
             at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

It looks like Share's implementation assumes the vec it wraps is at least 1 byte long in all three of these methods. I could add a check that the vec isn't empty before creating the share, but it doesn't seem ideal that one can create a Share (using either the pub vec like I did above, or via the TryFrom<&[u8]> that always succeeds) that will then panic if used.

@jgallagher
Copy link
Author

I don't think this is directly relevant to the issue at hand so I omitted it from the main writeup, but I had a hard time finding a combination of crate versions that would compile. The above example was with

p256 = "0.9"
rand = "0.8"
vsss-rs = { version = "2.0.0-pre2", default-features = false, features = ["std"] }

@mikelodder7
Copy link
Owner

@jgallagher Can you check and see if this is still an issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants