Skip to content

Commit

Permalink
Merge pull request #18471 from geoffw0/weakhash
Browse files Browse the repository at this point in the history
Rust: Weak hashing query
  • Loading branch information
geoffw0 authored Jan 14, 2025
2 parents b2bb143 + e61d6ae commit 6402aa5
Show file tree
Hide file tree
Showing 14 changed files with 705 additions and 4 deletions.
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 SensitiveData.super.getClassification() = SensitiveDataClassification::password() and // (covered in ComputationallyExpensiveHashFunction)
not SensitiveData.super.getClassification() = SensitiveDataClassification::id() // (not accurate enough)
}

override SensitiveDataClassification getClassification() {
result = SensitiveData.super.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() {
SensitiveData.super.getClassification() = SensitiveDataClassification::password()
}

override SensitiveDataClassification getClassification() {
result = SensitiveData.super.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)
}
118 changes: 118 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,118 @@
<!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

0 comments on commit 6402aa5

Please sign in to comment.