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

Rust: Weak hashing query #18471

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion rust/ql/lib/codeql/rust/Frameworks.qll
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
*/

private import codeql.rust.frameworks.Reqwest
private import codeql.rust.frameworks.RustCrypto
private import codeql.rust.frameworks.rustcrypto.RustCrypto
private import codeql.rust.frameworks.stdlib.Env
private import codeql.rust.frameworks.Sqlx
10 changes: 10 additions & 0 deletions rust/ql/lib/codeql/rust/frameworks/rustcrypto/rustcrypto.model.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
extensions:
- addsTo:
pack: codeql/rust-all
extensible: sinkModel
data:
- ["repo:https://github.com/RustCrypto/traits:digest", "<_ as crate::digest::Digest>::new_with_prefix", "Argument[0]", "hasher-input", "manual"]
- ["repo:https://github.com/RustCrypto/traits:digest", "<_ as crate::digest::Digest>::update", "Argument[0]", "hasher-input", "manual"]
- ["repo:https://github.com/RustCrypto/traits:digest", "<_ as crate::digest::Digest>::chain_update", "Argument[0]", "hasher-input", "manual"]
- ["repo:https://github.com/RustCrypto/traits:digest", "<_ as crate::digest::Digest>::digest", "Argument[0]", "hasher-input", "manual"]
- ["repo:https://github.com/stainless-steel/md5:md5", "crate::compute", "Argument[0]", "hasher-input", "manual"]
2 changes: 1 addition & 1 deletion rust/ql/lib/codeql/rust/security/SensitiveData.qll
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import rust
private import internal.SensitiveDataHeuristics
import internal.SensitiveDataHeuristics
private import codeql.rust.dataflow.DataFlow

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
/**
* Provides default sources, sinks and sanitizers for detecting "use of a
* broken or weak cryptographic hashing algorithm on sensitive data"
* vulnerabilities, as well as extension points for adding your own. This is
* divided into two general cases:
* - hashing sensitive data
* - hashing passwords (which requires the hashing algorithm to be
* sufficiently computationally expensive in addition to other requirements)
*/

import rust
private import codeql.rust.Concepts
private import codeql.rust.security.SensitiveData
private import codeql.rust.dataflow.DataFlow
private import codeql.rust.dataflow.FlowSource
private import codeql.rust.dataflow.FlowSink
private import codeql.rust.dataflow.internal.DataFlowImpl

/**
* Provides default sources, sinks and sanitizers for detecting "use of a broken or weak
* cryptographic hashing algorithm on sensitive data" vulnerabilities on sensitive data that does
* NOT require computationally expensive hashing, as well as extension points for adding your own.
*
* Also see the `ComputationallyExpensiveHashFunction` module.
*/
module NormalHashFunction {
/**
* A data flow source for "use of a broken or weak cryptographic hashing algorithm on sensitive
* data" vulnerabilities that does not require computationally expensive hashing. That is, a
* piece of sensitive data that is not a password.
*/
abstract class Source extends DataFlow::Node {
Source() { not this instanceof ComputationallyExpensiveHashFunction::Source }

/**
* Gets the classification of the sensitive data.
*/
abstract string getClassification();
}

/**
* A data flow sink for "use of a broken or weak cryptographic hashing algorithm on sensitive
* data" vulnerabilities that applies to data that does not require computationally expensive
* hashing. That is, a broken or weak hashing algorithm.
*/
abstract class Sink extends DataFlow::Node {
/**
* Gets the name of the weak hashing algorithm.
*/
abstract string getAlgorithmName();
}

/**
* A barrier for "use of a broken or weak cryptographic hashing algorithm on sensitive data"
* vulnerabilities that applies to data that does not require computationally expensive hashing.
*/
abstract class Barrier extends DataFlow::Node { }

/**
* A flow source modeled by the `SensitiveData` library.
*/
class SensitiveDataAsSource extends Source instanceof SensitiveData {
SensitiveDataAsSource() {
not this.(SensitiveData).getClassification() = SensitiveDataClassification::password() and // (covered in ComputationallyExpensiveHashFunction)
not this.(SensitiveData).getClassification() = SensitiveDataClassification::id() // (not accurate enough)
}

override SensitiveDataClassification getClassification() {
result = this.(SensitiveData).getClassification()
}
}

/**
* A flow sink modeled by the `Cryptography` module.
*/
class WeakHashingOperationInputAsSink extends Sink {
Cryptography::HashingAlgorithm algorithm;

WeakHashingOperationInputAsSink() {
exists(Cryptography::CryptographicOperation operation |
algorithm.isWeak() and
algorithm = operation.getAlgorithm() and
this = operation.getAnInput()
)
}

override string getAlgorithmName() { result = algorithm.getName() }
}
}

/**
* Provides default sources, sinks and sanitizers for detecting "use of a broken or weak
* cryptographic hashing algorithm on sensitive data" vulnerabilities on sensitive data that DOES
* require computationally expensive hashing, as well as extension points for adding your own.
*
* Also see the `NormalHashFunction` module.
*/
module ComputationallyExpensiveHashFunction {
/**
* A data flow source for "use of a broken or weak cryptographic hashing algorithm on sensitive
* data" vulnerabilities that does require computationally expensive hashing. That is, a
* password.
*/
abstract class Source extends DataFlow::Node {
/**
* Gets the classification of the sensitive data.
*/
abstract string getClassification();
}

/**
* A data flow sink for "use of a broken or weak cryptographic hashing algorithm on sensitive
* data" vulnerabilities that applies to data that does require computationally expensive
* hashing. That is, a broken or weak hashing algorithm or one that is not computationally
* expensive enough for password hashing.
*/
abstract class Sink extends DataFlow::Node {
/**
* Gets the name of the weak hashing algorithm.
*/
abstract string getAlgorithmName();

/**
* Holds if this sink is for a computationally expensive hash function (meaning that hash
* function is just weak in some other regard.
*/
abstract predicate isComputationallyExpensive();
}

/**
* A barrier for "use of a broken or weak cryptographic hashing algorithm on sensitive data"
* vulnerabilities that applies to data that does require computationally expensive hashing.
*/
abstract class Barrier extends DataFlow::Node { }

/**
* A flow source modeled by the `SensitiveData` library.
*/
class PasswordAsSource extends Source instanceof SensitiveData {
PasswordAsSource() {
this.(SensitiveData).getClassification() = SensitiveDataClassification::password()
}

override SensitiveDataClassification getClassification() {
result = this.(SensitiveData).getClassification()
}
}

/**
* A flow sink modeled by the `Cryptography` module.
*/
class WeakPasswordHashingOperationInputSink extends Sink {
Cryptography::CryptographicAlgorithm algorithm;

WeakPasswordHashingOperationInputSink() {
exists(Cryptography::CryptographicOperation operation |
(
algorithm instanceof Cryptography::PasswordHashingAlgorithm and
algorithm.isWeak()
or
algorithm instanceof Cryptography::HashingAlgorithm // Note that HashingAlgorithm and PasswordHashingAlgorithm are disjoint
) and
algorithm = operation.getAlgorithm() and
this = operation.getAnInput()
)
}

override string getAlgorithmName() { result = algorithm.getName() }

override predicate isComputationallyExpensive() {
algorithm instanceof Cryptography::PasswordHashingAlgorithm
}
}
}

/**
* An externally modeled operation that hashes data, for example a call to `md5::Md5::digest(data)`.
*/
class ModeledHashOperation extends Cryptography::CryptographicOperation::Range {
DataFlow::Node input;
string algorithmName;

ModeledHashOperation() {
exists(CallExpr call |
sinkNode(input, "hasher-input") and
call = input.(Node::FlowSummaryNode).getSinkElement().getCall() and
call = this.asExpr().getExpr() and
algorithmName =
call.getFunction().(PathExpr).getPath().getQualifier().getPart().getNameRef().getText()
)
}

override DataFlow::Node getInitialization() { result = this }

override Cryptography::HashingAlgorithm getAlgorithm() { result.matchesName(algorithmName) }

override DataFlow::Node getAnInput() { result = input }

override Cryptography::BlockMode getBlockMode() { none() } // (does not apply for hashing)
}
117 changes: 117 additions & 0 deletions rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
A broken or weak cryptographic hash function can leave data
vulnerable, and should not be used in security related code.
</p>

<p>
A strong cryptographic hash function should be resistant to:
</p>
<ul>
<li>
<b>Pre-image attacks</b>. If you know a hash value <code>h(x)</code>,
you should not be able to easily find the input <code>x</code>.
</li>
<li>
<b>Collision attacks</b>. If you know a hash value <code>h(x)</code>,
you should not be able to easily find a different input
<code>y</code>
with the same hash value <code>h(x) = h(y)</code>.
</li>
<li>
<b>Brute force</b>. For passwords and other data with limited
input space, if you know a hash value <code>h(x)</code>
you should not be able to find the input <code>x</code> even using
a brute force attack (without significant computational effort).
</li>
</ul>

<p>
As an example, both MD5 and SHA-1 are known to be vulnerable to collision attacks.
</p>

<p>
All of MD5, SHA-1, SHA-2 and SHA-3 are weak against offline brute forcing, so
they are not suitable for hashing passwords. This includes SHA-224, SHA-256,
SHA-384 and SHA-512, which are in the SHA-2 family.
</p>

<p>
Since it's OK to use a weak cryptographic hash function in a non-security
context, this query only alerts when these are used to hash sensitive
data (such as passwords, certificates, usernames).
</p>

</overview>
<recommendation>

<p>
Ensure that you use a strong, modern cryptographic hash function, such as:
</p>

<ul>
<li>
Argon2, scrypt, bcrypt, or PBKDF2 for passwords and other data with limited input space where
a dictionary-like attack is feasible.
</li>
<li>
SHA-2, or SHA-3 in other cases.
</li>
</ul>

<p>
Note that special purpose algorithms, which are used to ensure that a message comes from a
particular sender, exist for message authentication. These algorithms should be used when
appropriate, as they address common vulnerabilities of simple hashing schemes in this context.
</p>

</recommendation>
<example>

<p>
The following examples show hashing sensitive data using the MD5 hashing algorithm that is known to be
vulnerable to collision attacks, and hashing passwords using the SHA-3 algorithm that is weak to brute
force attacks:
</p>
<sample src="WeakSensitiveDataHashingBad.rs"/>

<p>
To make these secure, we can use the SHA-3 algorithm for sensitive data and Argon2 for passwords:
</p>
<sample src="WeakSensitiveDataHashingGood.rs"/>

</example>
<references>
<li>
OWASP:
<a href="https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html">
Password Storage Cheat Sheet
</a>
and
<a href="https://cheatsheetseries.owasp.org/cheatsheets/Transport_Layer_Security_Cheat_Sheet.html">
Transport Layer Security Cheat Sheet
</a>
</li>
<li>
GitHub:
<a href="https://github.com/RustCrypto/hashes?tab=readme-ov-file#rustcrypto-hashes">
RustCrypto: Hashes
</a>
and
<a href="https://github.com/RustCrypto/password-hashes?tab=readme-ov-file#rustcrypto-password-hashes">
RustCrypto: Password Hashes
</a>
</li>
<li>
The RustCrypto Book:
<a href="https://rustcrypto.org/key-derivation/hashing-password.html">
Password Hashing
</a>
</li>
</references>

</qhelp>
Loading
Loading