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

Fix Hash to scalar incomplete #3

Closed
moCello opened this issue Feb 8, 2024 · 0 comments · Fixed by #4
Closed

Fix Hash to scalar incomplete #3

moCello opened this issue Feb 8, 2024 · 0 comments · Fixed by #4

Comments

@moCello
Copy link
Member

moCello commented Feb 8, 2024

Summary

From the audit report point BLS01:

The h() function in hash.rs hashes to a subgroup element by truncating
a 64-byte BLAKE2B hash to BlsScalar::SIZE=32 bytes, then clearing the
top 2 bytes to obtain a value less than 2254:

/// Hash an arbitrary slice of bytes into a [`BlsScalar`]
fn h(msg: &[u8]) -> BlsScalar {
    let mut digest: [u8; BlsScalar::SIZE] = Blake2b::digest(msg).into();

    // Truncate the contract id to fit bls
    digest[31] &= 0x3f;

    let hash: Option<BlsScalar> = BlsScalar::from_bytes(&digest).into();
    hash.unwrap_or_default()
}

However with such an implementation, certain subgroup elements cannot be
reached: as the subgroup size
0x73eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000001 is
approx. 2254.857.

It means that certain points will never be reached by h0(), and
certain scalars will never be reached by h1() (approx.
2253.7.

This appears to us not to be exploitable, but it would be safer to
implement the standard hash_to_field() function, as defined in the
Hashing to Ellpitic
Curve

RFC and implemented in
zkcrypto/bls12_381.

Possible Solution

We can use our BlsScalar::hash_to_scalar(msg) here.

moCello added a commit that referenced this issue Feb 8, 2024
@moCello moCello closed this as completed in #4 Feb 9, 2024
@moCello moCello mentioned this issue Feb 28, 2024
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 a pull request may close this issue.

1 participant