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

try to fix panic at CompressedRistretto from_slice #285

Closed
wants to merge 1 commit into from
Closed

try to fix panic at CompressedRistretto from_slice #285

wants to merge 1 commit into from

Conversation

HaoXuan40404
Copy link

@HaoXuan40404 HaoXuan40404 commented Sep 19, 2019

Hello, I found that the function "from_silce" in src/ristretto.rs will happen a panic when receiving a slice of incorrect length. Maybe this unreasonable. And then I try to fix it.

Test main.rs like this

extern crate curve25519_dalek;
use curve25519_dalek::ristretto;
use curve25519_dalek::scalar::Scalar;

fn main() {
    let test_u8 = [1, 2, 3, 4];
    let test_point = ristretto::CompressedRistretto::from_slice(&test_u8);
    println!("test_point = {:?}", test_point);
}

Then I got this:

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `32`,
 right: `0`: destination and source slices have different lengths', src/libcore/slice/mod.rs:2119:9
stack backtrace:

@tarcieri
Copy link
Contributor

tarcieri commented Sep 19, 2019

I think the correct solution to avoiding a panic here is to have a fallible API, not zero-padding. Your “solution” is still panicking because copy_from_slice requires that the two slice lengths are equal, and you’re not slicing the temporary buffer to match the input length. But even if you did, this approach can’t handle an overlength input, nor does zero padding actually help in this context.

@HaoXuan40404
Copy link
Author

So maybe return ”result” would be better?

@HaoXuan40404
Copy link
Author

Ok, so this fallible API is in expected, right?

@tarcieri
Copy link
Contributor

Yes, although it’d be a breaking API change.

If @hdevalence is ok with an MSRV bump, you could add a new fallible API by adding a TryFrom impl perhaps.

@HaoXuan40404
Copy link
Author

Ok, thank you

pinkforest pushed a commit to pinkforest/curve25519-dalek that referenced this pull request Jun 27, 2023
The `ed25519` v2.2.0 crate bumps the `pkcs8` dependency to v0.10.

This updates `ed25519` to the latest version and updates the PKCS#8
support to use the new API.
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

Successfully merging this pull request may close these issues.

2 participants