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

Argon2dHasher uses a predictable salt #2455

Closed
jsdanielh opened this issue May 9, 2024 · 0 comments · Fixed by #2444
Closed

Argon2dHasher uses a predictable salt #2455

jsdanielh opened this issue May 9, 2024 · 0 comments · Fixed by #2444

Comments

@jsdanielh
Copy link
Contributor

Link: https://hackerone.com/reports/2491434
Date: 2024-05-06 06:41:10 UTC
By: ryanrb
Weakness: Use of a One-Way Hash with a Predictable Salt

Details:

Summary

The hash lib uses a hardcoded, predictable salt of nimiqrocks! which is leveraged in Argon2dHasher. Argon2dHasher is then used in web-client.

The original commit did not make any mention of why this approach was chosen, which may be able to add further context. In absence of that, it is assumed that this should be adjusted to a more secure approach.

Project: core-rs-albatross
File reference: hash/src/lib.rs
Line: 269


// Argon2d

const ARGON2D_LENGTH: usize = 32;
const NIMIQ_ARGON2_SALT: &str = "nimiqrocks!";

Recommendation

Salt should be unique per password
source: Rust argon2 docs

Ensure that the salt is a sufficiently random value. The docs for argon2 make use of let salt = SaltString::generate(&mut OsRng); to generate a secure salt.

References:

Impact

Per CWE-760:

This makes it easier for attackers to pre-compute the hash value using dictionary attack techniques such as rainbow tables, effectively disabling the protection that an unpredictable salt would provide.

As was noted above, the original commit occurred 6 years ago on Jun 24, 2018. Given the open-source nature of this project, any potential weaknesses like this are amplified by the fact that they are freely discoverable on the open internet.

This implementation is public in web-client, but doesn't appear to be used, so the direct impact is difficult to assess. However, current or future usages of it are susceptible to the risks laid out in CWE-760.

hrxi added a commit that referenced this issue May 15, 2024
They were only exposed to the web-client, but I couldn't find any
reference to this function.

The Argon2d/Argon2id implementation in `nimiq_hash::argon2kdf` remain.

Fixes #2455 by removing the unused code.
@styppo styppo added this to the Nimiq PoS Mainnet milestone May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants