From 509c6ffb7a67e00387f36c5d9da86793a86515fb Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 3 Jan 2025 11:20:08 +0000
Subject: [PATCH 01/18] Rust: Add tests for weak hashing.
---
.../query-tests/security/CWE-328/options.yml | 12 ++
.../test/query-tests/security/CWE-328/test.rs | 160 ++++++++++++++++++
2 files changed, 172 insertions(+)
create mode 100644 rust/ql/test/query-tests/security/CWE-328/options.yml
create mode 100644 rust/ql/test/query-tests/security/CWE-328/test.rs
diff --git a/rust/ql/test/query-tests/security/CWE-328/options.yml b/rust/ql/test/query-tests/security/CWE-328/options.yml
new file mode 100644
index 000000000000..3c69a7519e76
--- /dev/null
+++ b/rust/ql/test/query-tests/security/CWE-328/options.yml
@@ -0,0 +1,12 @@
+qltest_cargo_check: true
+qltest_dependencies:
+ - digest = { version = "0.10.7" }
+ - md-5 = { version = "0.10.6" }
+ - md5_alt = { package = "md5", version = "0.7.0" }
+ - sha1 = { version = "0.10.6" }
+ - sha1-checked = { version = "0.10.0" }
+ - sha3 = { version = "0.10.8" }
+ - argon2 = { version = "0.5.3" }
+ - serde = { version = "1.0.217", features = ["derive"] }
+ - serde_json = { version = "1.0.134" }
+ - serde_urlencoded = { version = "0.7.1" }
diff --git a/rust/ql/test/query-tests/security/CWE-328/test.rs b/rust/ql/test/query-tests/security/CWE-328/test.rs
new file mode 100644
index 000000000000..a837a360bda4
--- /dev/null
+++ b/rust/ql/test/query-tests/security/CWE-328/test.rs
@@ -0,0 +1,160 @@
+use md5::{Digest};
+use serde::{Serialize};
+use argon2::{PasswordHasher};
+
+// --- tests ---
+
+fn test_hash_algorithms(
+ harmless: &str, credit_card_no: &str, password: &str, encrypted_password: &str, salt: &str
+) {
+ // test hashing with different algorithms and data
+
+ // MD5
+ _ = md5::Md5::digest(harmless);
+ _ = md5::Md5::digest(credit_card_no); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = md5::Md5::digest(password); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = md5::Md5::digest(encrypted_password);
+
+ // MD5 (alternative / older library)
+ _ = md5_alt::compute(harmless);
+ _ = md5_alt::compute(credit_card_no); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = md5_alt::compute(password); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = md5_alt::compute(encrypted_password);
+
+ // SHA-1
+ _ = sha1::Sha1::digest(harmless);
+ _ = sha1::Sha1::digest(credit_card_no); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = sha1::Sha1::digest(password); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = sha1::Sha1::digest(encrypted_password);
+
+ // SHA-1 checked
+ _ = sha1_checked::Sha1::digest(harmless);
+ _ = sha1_checked::Sha1::digest(credit_card_no); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = sha1_checked::Sha1::digest(password); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = sha1_checked::Sha1::digest(encrypted_password);
+
+ // SHA-256 (appropriate for sensitive data hashing)
+ _ = sha3::Sha3_256::digest(harmless);
+ _ = sha3::Sha3_256::digest(credit_card_no);
+ _ = sha3::Sha3_256::digest(password); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = sha3::Sha3_256::digest(encrypted_password);
+
+ // Argon2 (appropriate for password hashing)
+ let argon2_salt = argon2::password_hash::Salt::from_b64(salt).unwrap();
+ _ = argon2::Argon2::default().hash_password(harmless.as_bytes(), argon2_salt).unwrap().to_string();
+ _ = argon2::Argon2::default().hash_password(credit_card_no.as_bytes(), argon2_salt).unwrap().to_string();
+ _ = argon2::Argon2::default().hash_password(password.as_bytes(), argon2_salt).unwrap().to_string();
+ _ = argon2::Argon2::default().hash_password(encrypted_password.as_bytes(), argon2_salt).unwrap().to_string();
+}
+
+fn test_hash_code_patterns(
+ harmless: &str, password: &str,
+ harmless_str: String, password_str: String,
+ harmless_arr: &[u8], password_arr: &[u8],
+ harmless_vec: Vec<u8>, password_vec: Vec<u8>
+) {
+ // test hashing with different code patterns
+
+ // hash different types of data
+ _ = md5::Md5::digest(harmless_str);
+ _ = md5::Md5::digest(password_str); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = md5::Md5::digest(harmless_arr);
+ _ = md5::Md5::digest(password_arr); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = md5::Md5::digest(harmless_vec);
+ _ = md5::Md5::digest(password_vec); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+
+ // hash through a hasher object
+ let mut md5_hasher = md5::Md5::new();
+ md5_hasher.update(b"abc");
+ md5_hasher.update(harmless);
+ md5_hasher.update(password); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = md5_hasher.finalize();
+
+ _ = md5::Md5::new().chain_update(harmless).chain_update(harmless).chain_update(harmless).finalize();
+ _ = md5::Md5::new().chain_update(harmless).chain_update(password).chain_update(harmless).finalize(); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+
+ _ = md5::Md5::new_with_prefix(harmless).finalize();
+ _ = md5::Md5::new_with_prefix(password).finalize(); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+
+ // hash transformed data
+ _ = md5::Md5::digest(harmless.trim());
+ _ = md5::Md5::digest(password.trim()); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = md5::Md5::digest(harmless.as_bytes());
+ _ = md5::Md5::digest(password.as_bytes()); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = md5::Md5::digest(std::str::from_utf8(harmless_arr).unwrap());
+ _ = md5::Md5::digest(std::str::from_utf8(password_arr).unwrap()); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+}
+
+#[derive(Serialize)]
+struct MyStruct1 {
+ id: u64,
+ data: String,
+}
+
+#[derive(Serialize)]
+struct MyStruct2 {
+ id: u64,
+ credit_card_no: String,
+}
+
+#[derive(Serialize)]
+struct MyStruct3 {
+ id: u64,
+ password: String,
+}
+
+fn test_hash_structs() {
+ // test hashing with data in a struct
+ let s1 = MyStruct1 {
+ id: 1,
+ data: "0123456789".to_string(),
+ };
+ let s2 = MyStruct2 {
+ id: 2,
+ credit_card_no: "0123456789".to_string(),
+ };
+ let s3 = MyStruct3 {
+ id: 3,
+ password: "0123456789".to_string(),
+ };
+
+ // serialize with serde
+ let str1a = serde_json::to_string(&s1).unwrap();
+ let str2a = serde_json::to_string(&s2).unwrap();
+ let str3a = serde_json::to_string(&s3).unwrap();
+ let str1b = serde_json::to_vec(&s1).unwrap();
+ let str2b = serde_json::to_vec(&s2).unwrap();
+ let str3b = serde_json::to_vec(&s3).unwrap();
+ let str1c = serde_urlencoded::to_string(&s1).unwrap();
+ let str2c = serde_urlencoded::to_string(&s2).unwrap();
+ let str3c = serde_urlencoded::to_string(&s3).unwrap();
+
+ // hash with MD5
+ let mut md5_hasher = md5::Md5::new();
+ md5_hasher.update(s1.data);
+ md5_hasher.update(s2.credit_card_no); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ md5_hasher.update(s3.password); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ md5_hasher.update(str1a);
+ md5_hasher.update(str2a); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ md5_hasher.update(str3a); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ md5_hasher.update(str1b);
+ md5_hasher.update(str2b); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ md5_hasher.update(str3b); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ md5_hasher.update(str1c);
+ md5_hasher.update(str2c); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ md5_hasher.update(str3c); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = md5_hasher.finalize();
+}
+
+fn test_hash_file(
+ harmless_filename: &str, password_filename: &str
+) {
+ // test hashing files
+ let mut harmless_file = std::fs::File::open(harmless_filename).unwrap();
+ let mut password_file = std::fs::File::open(password_filename).unwrap();
+
+ let mut md5_hasher = md5::Md5::new();
+ _ = std::io::copy(&mut harmless_file, &mut md5_hasher);
+ _ = std::io::copy(&mut password_file, &mut md5_hasher); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = md5_hasher.finalize();
+}
From 8f4a52001f5ddd254187e5277cdfccdc5dd902b4 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 3 Jan 2025 13:21:09 +0000
Subject: [PATCH 02/18] Rust: Add query framework.
---
.../WeakSensitiveDataHashingExtensions.qll | 147 ++++++++++++++++++
.../CWE-328/WeakSensitiveDataHashing.ql | 117 ++++++++++++++
.../CWE-328/WeakSensitiveDataHashing.expected | 4 +
.../CWE-328/WeakSensitiveDataHashing.qlref | 4 +
4 files changed, 272 insertions(+)
create mode 100644 rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll
create mode 100755 rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql
create mode 100644 rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.expected
create mode 100644 rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.qlref
diff --git a/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll b/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll
new file mode 100644
index 000000000000..24eede034491
--- /dev/null
+++ b/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll
@@ -0,0 +1,147 @@
+/**
+ * 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.dataflow.DataFlow
+
+/**
+ * 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.
+ */
+ 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 { }
+
+ // TODO: SensitiveDataSourceAsSource
+
+ /**
+ * A flow sink modelled 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 { }
+
+ // TODO: PasswordSourceAsSource
+
+ /**
+ * A flow sink modelled 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
+ }
+ }
+}
diff --git a/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql
new file mode 100755
index 000000000000..8779ce56d273
--- /dev/null
+++ b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql
@@ -0,0 +1,117 @@
+/**
+ * @name Use of a broken or weak cryptographic hashing algorithm on sensitive data
+ * @description Using broken or weak cryptographic hashing algorithms can compromise security.
+ * @kind path-problem
+ * @problem.severity warning
+ * @security-severity 7.5
+ * @precision high
+ * @id rust/weak-sensitive-data-hashing
+ * @tags security
+ * external/cwe/cwe-327
+ * external/cwe/cwe-328
+ * external/cwe/cwe-916
+ */
+
+import rust
+import codeql.rust.security.WeakSensitiveDataHashingExtensions
+import codeql.rust.dataflow.DataFlow
+import codeql.rust.dataflow.TaintTracking
+
+/**
+ * Provides a taint-tracking configuration for detecting use of a broken or weak
+ * cryptographic hash function on sensitive data, that does NOT require a
+ * computationally expensive hash function.
+ */
+module NormalHashFunctionFlow {
+ import NormalHashFunction
+
+ private module Config implements DataFlow::ConfigSig {
+ predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+ predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+ predicate isBarrier(DataFlow::Node node) { node instanceof Barrier }
+
+ predicate isBarrierIn(DataFlow::Node node) {
+ // make sources barriers so that we only report the closest instance
+ isSource(node)
+ }
+
+ predicate isBarrierOut(DataFlow::Node node) {
+ // make sinks barriers so that we only report the closest instance
+ isSink(node)
+ }
+ }
+
+ module Flow = TaintTracking::Global<Config>;
+}
+
+/**
+ * Provides a taint-tracking configuration for detecting use of a broken or weak
+ * cryptographic hashing algorithm on passwords.
+ *
+ * Passwords has stricter requirements on the hashing algorithm used (must be
+ * computationally expensive to prevent brute-force attacks).
+ */
+module ComputationallyExpensiveHashFunctionFlow {
+ import ComputationallyExpensiveHashFunction
+
+ private module Config implements DataFlow::ConfigSig {
+ predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+ predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+ predicate isBarrier(DataFlow::Node node) { node instanceof Barrier }
+
+ predicate isBarrierIn(DataFlow::Node node) {
+ // make sources barriers so that we only report the closest instance
+ isSource(node)
+ }
+
+ predicate isBarrierOut(DataFlow::Node node) {
+ // make sinks barriers so that we only report the closest instance
+ isSink(node)
+ }
+ }
+
+ module Flow = TaintTracking::Global<Config>;
+}
+
+/**
+ * Global taint-tracking for detecting both variants of "use of a broken or weak
+ * cryptographic hashing algorithm on sensitive data" vulnerabilities. The two configurations are
+ * merged to generate a combined path graph.
+ */
+module WeakSensitiveDataHashingFlow =
+ DataFlow::MergePathGraph<NormalHashFunctionFlow::Flow::PathNode,
+ ComputationallyExpensiveHashFunctionFlow::Flow::PathNode,
+ NormalHashFunctionFlow::Flow::PathGraph,
+ ComputationallyExpensiveHashFunctionFlow::Flow::PathGraph>;
+
+import WeakSensitiveDataHashingFlow::PathGraph
+
+from
+ WeakSensitiveDataHashingFlow::PathNode source, WeakSensitiveDataHashingFlow::PathNode sink,
+ string ending, string algorithmName, string classification
+where
+ NormalHashFunctionFlow::Flow::flowPath(source.asPathNode1(), sink.asPathNode1()) and
+ algorithmName = sink.getNode().(NormalHashFunction::Sink).getAlgorithmName() and
+ classification = source.getNode().(NormalHashFunction::Source).getClassification() and
+ ending = "."
+ or
+ ComputationallyExpensiveHashFunctionFlow::Flow::flowPath(source.asPathNode2(), sink.asPathNode2()) and
+ algorithmName = sink.getNode().(ComputationallyExpensiveHashFunction::Sink).getAlgorithmName() and
+ classification =
+ source.getNode().(ComputationallyExpensiveHashFunction::Source).getClassification() and
+ (
+ sink.getNode().(ComputationallyExpensiveHashFunction::Sink).isComputationallyExpensive() and
+ ending = "."
+ or
+ not sink.getNode().(ComputationallyExpensiveHashFunction::Sink).isComputationallyExpensive() and
+ ending =
+ " for " + classification +
+ " hashing, since it is not a computationally expensive hash function."
+ )
+select sink.getNode(), source, sink,
+ "$@ is used in a hashing algorithm (" + algorithmName + ") that is insecure" + ending,
+ source.getNode(), "Sensitive data (" + classification + ")"
diff --git a/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.expected b/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.expected
new file mode 100644
index 000000000000..e217064d1dfc
--- /dev/null
+++ b/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.expected
@@ -0,0 +1,4 @@
+edges
+nodes
+subpaths
+#select
diff --git a/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.qlref b/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.qlref
new file mode 100644
index 000000000000..718e500931d3
--- /dev/null
+++ b/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.qlref
@@ -0,0 +1,4 @@
+query: queries/Security/CWE-328/WeakSensitiveDataHashing.ql
+postprocess:
+ - utils/test/PrettyPrintModels.ql
+ - utils/test/InlineExpectationsTestQuery.ql
From d72b978bc7b1eac5d32c52eb782dff94e78e0400 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Mon, 6 Jan 2025 14:26:20 +0000
Subject: [PATCH 03/18] Rust: Add sensitive data sources.
---
.../codeql/rust/security/SensitiveData.qll | 2 +-
.../WeakSensitiveDataHashingExtensions.qll | 30 +++++++++++++++++--
2 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/rust/ql/lib/codeql/rust/security/SensitiveData.qll b/rust/ql/lib/codeql/rust/security/SensitiveData.qll
index ac17a3ee0752..49db524cf235 100644
--- a/rust/ql/lib/codeql/rust/security/SensitiveData.qll
+++ b/rust/ql/lib/codeql/rust/security/SensitiveData.qll
@@ -6,7 +6,7 @@
*/
import rust
-private import internal.SensitiveDataHeuristics
+import internal.SensitiveDataHeuristics
private import codeql.rust.dataflow.DataFlow
/**
diff --git a/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll b/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll
index 24eede034491..bed193de32a0 100644
--- a/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll
+++ b/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll
@@ -10,6 +10,7 @@
import rust
private import codeql.rust.Concepts
+private import codeql.rust.security.SensitiveData
private import codeql.rust.dataflow.DataFlow
/**
@@ -23,7 +24,7 @@ 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.
+ * piece of sensitive data that is not a password.
*/
abstract class Source extends DataFlow::Node {
Source() { not this instanceof ComputationallyExpensiveHashFunction::Source }
@@ -52,7 +53,19 @@ module NormalHashFunction {
*/
abstract class Barrier extends DataFlow::Node { }
- // TODO: SensitiveDataSourceAsSource
+ /**
+ * A flow source modelled 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 modelled by the `Cryptography` module.
@@ -117,7 +130,18 @@ module ComputationallyExpensiveHashFunction {
*/
abstract class Barrier extends DataFlow::Node { }
- // TODO: PasswordSourceAsSource
+ /**
+ * A flow source modelled 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 modelled by the `Cryptography` module.
From ae0f4f10def24ad664c0462de0e8fe0b795a33f0 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Mon, 6 Jan 2025 17:26:08 +0000
Subject: [PATCH 04/18] Rust: Add hash function sinks.
---
rust/ql/lib/codeql/rust/Frameworks.qll | 2 +-
.../{ => rustcrypto}/RustCrypto.qll | 28 +++++++++++
.../rustcrypto/rustcrypto.model.yml | 9 ++++
.../CWE-328/WeakSensitiveDataHashing.expected | 49 ++++++++++++++++++-
.../test/query-tests/security/CWE-328/test.rs | 22 ++++-----
5 files changed, 97 insertions(+), 13 deletions(-)
rename rust/ql/lib/codeql/rust/frameworks/{ => rustcrypto}/RustCrypto.qll (66%)
create mode 100644 rust/ql/lib/codeql/rust/frameworks/rustcrypto/rustcrypto.model.yml
diff --git a/rust/ql/lib/codeql/rust/Frameworks.qll b/rust/ql/lib/codeql/rust/Frameworks.qll
index 483056888ec6..6a5e95c82e56 100644
--- a/rust/ql/lib/codeql/rust/Frameworks.qll
+++ b/rust/ql/lib/codeql/rust/Frameworks.qll
@@ -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
diff --git a/rust/ql/lib/codeql/rust/frameworks/RustCrypto.qll b/rust/ql/lib/codeql/rust/frameworks/rustcrypto/RustCrypto.qll
similarity index 66%
rename from rust/ql/lib/codeql/rust/frameworks/RustCrypto.qll
rename to rust/ql/lib/codeql/rust/frameworks/rustcrypto/RustCrypto.qll
index 9dd40004766a..8a91044891f6 100644
--- a/rust/ql/lib/codeql/rust/frameworks/RustCrypto.qll
+++ b/rust/ql/lib/codeql/rust/frameworks/rustcrypto/RustCrypto.qll
@@ -5,6 +5,9 @@
private import rust
private import codeql.rust.Concepts
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
bindingset[algorithmName]
private string simplifyAlgorithmName(string algorithmName) {
@@ -55,3 +58,28 @@ class StreamCipherInit extends Cryptography::CryptographicOperation::Range {
override Cryptography::BlockMode getBlockMode() { result = "" }
}
+
+/**
+ * An externally modelled operation that hashes data, for example a call to `md5::Md5::digest(data)`.
+ */
+class ModelledHashOperation extends Cryptography::CryptographicOperation::Range {
+ DataFlow::Node input;
+ CallExpr call;
+ string algorithmName;
+
+ ModelledHashOperation() {
+ 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::CryptographicAlgorithm getAlgorithm() { result.matchesName(algorithmName) }
+
+ override DataFlow::Node getAnInput() { result = input }
+
+ override Cryptography::BlockMode getBlockMode() { none() } // (does not apply for hashing)
+}
diff --git a/rust/ql/lib/codeql/rust/frameworks/rustcrypto/rustcrypto.model.yml b/rust/ql/lib/codeql/rust/frameworks/rustcrypto/rustcrypto.model.yml
new file mode 100644
index 000000000000..3473ba51683d
--- /dev/null
+++ b/rust/ql/lib/codeql/rust/frameworks/rustcrypto/rustcrypto.model.yml
@@ -0,0 +1,9 @@
+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"]
diff --git a/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.expected b/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.expected
index e217064d1dfc..aa91cb94090d 100644
--- a/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.expected
+++ b/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.expected
@@ -1,4 +1,51 @@
+#select
+| test.rs:14:9:14:24 | ...::digest | test.rs:14:26:14:39 | credit_card_no | test.rs:14:9:14:24 | ...::digest | $@ is used in a hashing algorithm (MD5) that is insecure. | test.rs:14:26:14:39 | credit_card_no | Sensitive data (private) |
+| test.rs:15:9:15:24 | ...::digest | test.rs:15:26:15:33 | password | test.rs:15:9:15:24 | ...::digest | $@ is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. | test.rs:15:26:15:33 | password | Sensitive data (password) |
+| test.rs:26:9:26:26 | ...::digest | test.rs:26:28:26:41 | credit_card_no | test.rs:26:9:26:26 | ...::digest | $@ is used in a hashing algorithm (SHA1) that is insecure. | test.rs:26:28:26:41 | credit_card_no | Sensitive data (private) |
+| test.rs:27:9:27:26 | ...::digest | test.rs:27:28:27:35 | password | test.rs:27:9:27:26 | ...::digest | $@ is used in a hashing algorithm (SHA1) that is insecure for password hashing, since it is not a computationally expensive hash function. | test.rs:27:28:27:35 | password | Sensitive data (password) |
+| test.rs:32:9:32:34 | ...::digest | test.rs:32:36:32:49 | credit_card_no | test.rs:32:9:32:34 | ...::digest | $@ is used in a hashing algorithm (SHA1) that is insecure. | test.rs:32:36:32:49 | credit_card_no | Sensitive data (private) |
+| test.rs:33:9:33:34 | ...::digest | test.rs:33:36:33:43 | password | test.rs:33:9:33:34 | ...::digest | $@ is used in a hashing algorithm (SHA1) that is insecure for password hashing, since it is not a computationally expensive hash function. | test.rs:33:36:33:43 | password | Sensitive data (password) |
+| test.rs:39:9:39:30 | ...::digest | test.rs:39:32:39:39 | password | test.rs:39:9:39:30 | ...::digest | $@ is used in a hashing algorithm (SHA3256) that is insecure for password hashing, since it is not a computationally expensive hash function. | test.rs:39:32:39:39 | password | Sensitive data (password) |
+| test.rs:60:9:60:24 | ...::digest | test.rs:60:26:60:37 | password_str | test.rs:60:9:60:24 | ...::digest | $@ is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. | test.rs:60:26:60:37 | password_str | Sensitive data (password) |
+| test.rs:62:9:62:24 | ...::digest | test.rs:62:26:62:37 | password_arr | test.rs:62:9:62:24 | ...::digest | $@ is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. | test.rs:62:26:62:37 | password_arr | Sensitive data (password) |
+| test.rs:64:9:64:24 | ...::digest | test.rs:64:26:64:37 | password_vec | test.rs:64:9:64:24 | ...::digest | $@ is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. | test.rs:64:26:64:37 | password_vec | Sensitive data (password) |
+| test.rs:77:9:77:33 | ...::new_with_prefix | test.rs:77:35:77:42 | password | test.rs:77:9:77:33 | ...::new_with_prefix | $@ is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. | test.rs:77:35:77:42 | password | Sensitive data (password) |
edges
+| test.rs:14:26:14:39 | credit_card_no | test.rs:14:9:14:24 | ...::digest | provenance | MaD:1 |
+| test.rs:15:26:15:33 | password | test.rs:15:9:15:24 | ...::digest | provenance | MaD:1 |
+| test.rs:26:28:26:41 | credit_card_no | test.rs:26:9:26:26 | ...::digest | provenance | MaD:1 |
+| test.rs:27:28:27:35 | password | test.rs:27:9:27:26 | ...::digest | provenance | MaD:1 |
+| test.rs:32:36:32:49 | credit_card_no | test.rs:32:9:32:34 | ...::digest | provenance | MaD:1 |
+| test.rs:33:36:33:43 | password | test.rs:33:9:33:34 | ...::digest | provenance | MaD:1 |
+| test.rs:39:32:39:39 | password | test.rs:39:9:39:30 | ...::digest | provenance | MaD:1 |
+| test.rs:60:26:60:37 | password_str | test.rs:60:9:60:24 | ...::digest | provenance | MaD:1 |
+| test.rs:62:26:62:37 | password_arr | test.rs:62:9:62:24 | ...::digest | provenance | MaD:1 |
+| test.rs:64:26:64:37 | password_vec | test.rs:64:9:64:24 | ...::digest | provenance | MaD:1 |
+| test.rs:77:35:77:42 | password | test.rs:77:9:77:33 | ...::new_with_prefix | provenance | MaD:2 |
+models
+| 1 | Sink: repo:https://github.com/RustCrypto/traits:digest; <_ as crate::digest::Digest>::digest; hasher-input; Argument[0] |
+| 2 | Sink: repo:https://github.com/RustCrypto/traits:digest; <_ as crate::digest::Digest>::new_with_prefix; hasher-input; Argument[0] |
nodes
+| test.rs:14:9:14:24 | ...::digest | semmle.label | ...::digest |
+| test.rs:14:26:14:39 | credit_card_no | semmle.label | credit_card_no |
+| test.rs:15:9:15:24 | ...::digest | semmle.label | ...::digest |
+| test.rs:15:26:15:33 | password | semmle.label | password |
+| test.rs:26:9:26:26 | ...::digest | semmle.label | ...::digest |
+| test.rs:26:28:26:41 | credit_card_no | semmle.label | credit_card_no |
+| test.rs:27:9:27:26 | ...::digest | semmle.label | ...::digest |
+| test.rs:27:28:27:35 | password | semmle.label | password |
+| test.rs:32:9:32:34 | ...::digest | semmle.label | ...::digest |
+| test.rs:32:36:32:49 | credit_card_no | semmle.label | credit_card_no |
+| test.rs:33:9:33:34 | ...::digest | semmle.label | ...::digest |
+| test.rs:33:36:33:43 | password | semmle.label | password |
+| test.rs:39:9:39:30 | ...::digest | semmle.label | ...::digest |
+| test.rs:39:32:39:39 | password | semmle.label | password |
+| test.rs:60:9:60:24 | ...::digest | semmle.label | ...::digest |
+| test.rs:60:26:60:37 | password_str | semmle.label | password_str |
+| test.rs:62:9:62:24 | ...::digest | semmle.label | ...::digest |
+| test.rs:62:26:62:37 | password_arr | semmle.label | password_arr |
+| test.rs:64:9:64:24 | ...::digest | semmle.label | ...::digest |
+| test.rs:64:26:64:37 | password_vec | semmle.label | password_vec |
+| test.rs:77:9:77:33 | ...::new_with_prefix | semmle.label | ...::new_with_prefix |
+| test.rs:77:35:77:42 | password | semmle.label | password |
subpaths
-#select
diff --git a/rust/ql/test/query-tests/security/CWE-328/test.rs b/rust/ql/test/query-tests/security/CWE-328/test.rs
index a837a360bda4..1c97bf5773ac 100644
--- a/rust/ql/test/query-tests/security/CWE-328/test.rs
+++ b/rust/ql/test/query-tests/security/CWE-328/test.rs
@@ -11,8 +11,8 @@ fn test_hash_algorithms(
// MD5
_ = md5::Md5::digest(harmless);
- _ = md5::Md5::digest(credit_card_no); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
- _ = md5::Md5::digest(password); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = md5::Md5::digest(credit_card_no); // $ Source Alert[rust/weak-sensitive-data-hashing]
+ _ = md5::Md5::digest(password); // $ Source Alert[rust/weak-sensitive-data-hashing]
_ = md5::Md5::digest(encrypted_password);
// MD5 (alternative / older library)
@@ -23,20 +23,20 @@ fn test_hash_algorithms(
// SHA-1
_ = sha1::Sha1::digest(harmless);
- _ = sha1::Sha1::digest(credit_card_no); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
- _ = sha1::Sha1::digest(password); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = sha1::Sha1::digest(credit_card_no); // $ Source Alert[rust/weak-sensitive-data-hashing]
+ _ = sha1::Sha1::digest(password); // $ Source Alert[rust/weak-sensitive-data-hashing]
_ = sha1::Sha1::digest(encrypted_password);
// SHA-1 checked
_ = sha1_checked::Sha1::digest(harmless);
- _ = sha1_checked::Sha1::digest(credit_card_no); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
- _ = sha1_checked::Sha1::digest(password); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = sha1_checked::Sha1::digest(credit_card_no); // $ Source Alert[rust/weak-sensitive-data-hashing]
+ _ = sha1_checked::Sha1::digest(password); // $ Source Alert[rust/weak-sensitive-data-hashing]
_ = sha1_checked::Sha1::digest(encrypted_password);
// SHA-256 (appropriate for sensitive data hashing)
_ = sha3::Sha3_256::digest(harmless);
_ = sha3::Sha3_256::digest(credit_card_no);
- _ = sha3::Sha3_256::digest(password); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = sha3::Sha3_256::digest(password); // $ Source Alert[rust/weak-sensitive-data-hashing]
_ = sha3::Sha3_256::digest(encrypted_password);
// Argon2 (appropriate for password hashing)
@@ -57,11 +57,11 @@ fn test_hash_code_patterns(
// hash different types of data
_ = md5::Md5::digest(harmless_str);
- _ = md5::Md5::digest(password_str); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = md5::Md5::digest(password_str); // $ Source Alert[rust/weak-sensitive-data-hashing]
_ = md5::Md5::digest(harmless_arr);
- _ = md5::Md5::digest(password_arr); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = md5::Md5::digest(password_arr); // $ Source Alert[rust/weak-sensitive-data-hashing]
_ = md5::Md5::digest(harmless_vec);
- _ = md5::Md5::digest(password_vec); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = md5::Md5::digest(password_vec); // $ Source Alert[rust/weak-sensitive-data-hashing]
// hash through a hasher object
let mut md5_hasher = md5::Md5::new();
@@ -74,7 +74,7 @@ fn test_hash_code_patterns(
_ = md5::Md5::new().chain_update(harmless).chain_update(password).chain_update(harmless).finalize(); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
_ = md5::Md5::new_with_prefix(harmless).finalize();
- _ = md5::Md5::new_with_prefix(password).finalize(); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = md5::Md5::new_with_prefix(password).finalize(); // $ Source Alert[rust/weak-sensitive-data-hashing]
// hash transformed data
_ = md5::Md5::digest(harmless.trim());
From babfa758a3a3d9f9fca5b0357a33fbbf6c2182cd Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Thu, 9 Jan 2025 17:38:05 +0000
Subject: [PATCH 05/18] Rust: Add models for an alternative md5 library.
---
.../rust/frameworks/rustcrypto/rustcrypto.model.yml | 1 +
.../security/CWE-328/WeakSensitiveDataHashing.expected | 9 +++++++++
rust/ql/test/query-tests/security/CWE-328/test.rs | 4 ++--
3 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/rust/ql/lib/codeql/rust/frameworks/rustcrypto/rustcrypto.model.yml b/rust/ql/lib/codeql/rust/frameworks/rustcrypto/rustcrypto.model.yml
index 3473ba51683d..fe3fd67a8fd4 100644
--- a/rust/ql/lib/codeql/rust/frameworks/rustcrypto/rustcrypto.model.yml
+++ b/rust/ql/lib/codeql/rust/frameworks/rustcrypto/rustcrypto.model.yml
@@ -7,3 +7,4 @@ extensions:
- ["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"]
diff --git a/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.expected b/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.expected
index aa91cb94090d..888902d913d1 100644
--- a/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.expected
+++ b/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.expected
@@ -1,6 +1,8 @@
#select
| test.rs:14:9:14:24 | ...::digest | test.rs:14:26:14:39 | credit_card_no | test.rs:14:9:14:24 | ...::digest | $@ is used in a hashing algorithm (MD5) that is insecure. | test.rs:14:26:14:39 | credit_card_no | Sensitive data (private) |
| test.rs:15:9:15:24 | ...::digest | test.rs:15:26:15:33 | password | test.rs:15:9:15:24 | ...::digest | $@ is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. | test.rs:15:26:15:33 | password | Sensitive data (password) |
+| test.rs:20:9:20:24 | ...::compute | test.rs:20:26:20:39 | credit_card_no | test.rs:20:9:20:24 | ...::compute | $@ is used in a hashing algorithm (MD5) that is insecure. | test.rs:20:26:20:39 | credit_card_no | Sensitive data (private) |
+| test.rs:21:9:21:24 | ...::compute | test.rs:21:26:21:33 | password | test.rs:21:9:21:24 | ...::compute | $@ is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. | test.rs:21:26:21:33 | password | Sensitive data (password) |
| test.rs:26:9:26:26 | ...::digest | test.rs:26:28:26:41 | credit_card_no | test.rs:26:9:26:26 | ...::digest | $@ is used in a hashing algorithm (SHA1) that is insecure. | test.rs:26:28:26:41 | credit_card_no | Sensitive data (private) |
| test.rs:27:9:27:26 | ...::digest | test.rs:27:28:27:35 | password | test.rs:27:9:27:26 | ...::digest | $@ is used in a hashing algorithm (SHA1) that is insecure for password hashing, since it is not a computationally expensive hash function. | test.rs:27:28:27:35 | password | Sensitive data (password) |
| test.rs:32:9:32:34 | ...::digest | test.rs:32:36:32:49 | credit_card_no | test.rs:32:9:32:34 | ...::digest | $@ is used in a hashing algorithm (SHA1) that is insecure. | test.rs:32:36:32:49 | credit_card_no | Sensitive data (private) |
@@ -13,6 +15,8 @@
edges
| test.rs:14:26:14:39 | credit_card_no | test.rs:14:9:14:24 | ...::digest | provenance | MaD:1 |
| test.rs:15:26:15:33 | password | test.rs:15:9:15:24 | ...::digest | provenance | MaD:1 |
+| test.rs:20:26:20:39 | credit_card_no | test.rs:20:9:20:24 | ...::compute | provenance | MaD:3 |
+| test.rs:21:26:21:33 | password | test.rs:21:9:21:24 | ...::compute | provenance | MaD:3 |
| test.rs:26:28:26:41 | credit_card_no | test.rs:26:9:26:26 | ...::digest | provenance | MaD:1 |
| test.rs:27:28:27:35 | password | test.rs:27:9:27:26 | ...::digest | provenance | MaD:1 |
| test.rs:32:36:32:49 | credit_card_no | test.rs:32:9:32:34 | ...::digest | provenance | MaD:1 |
@@ -25,11 +29,16 @@ edges
models
| 1 | Sink: repo:https://github.com/RustCrypto/traits:digest; <_ as crate::digest::Digest>::digest; hasher-input; Argument[0] |
| 2 | Sink: repo:https://github.com/RustCrypto/traits:digest; <_ as crate::digest::Digest>::new_with_prefix; hasher-input; Argument[0] |
+| 3 | Sink: repo:https://github.com/stainless-steel/md5:md5; crate::compute; hasher-input; Argument[0] |
nodes
| test.rs:14:9:14:24 | ...::digest | semmle.label | ...::digest |
| test.rs:14:26:14:39 | credit_card_no | semmle.label | credit_card_no |
| test.rs:15:9:15:24 | ...::digest | semmle.label | ...::digest |
| test.rs:15:26:15:33 | password | semmle.label | password |
+| test.rs:20:9:20:24 | ...::compute | semmle.label | ...::compute |
+| test.rs:20:26:20:39 | credit_card_no | semmle.label | credit_card_no |
+| test.rs:21:9:21:24 | ...::compute | semmle.label | ...::compute |
+| test.rs:21:26:21:33 | password | semmle.label | password |
| test.rs:26:9:26:26 | ...::digest | semmle.label | ...::digest |
| test.rs:26:28:26:41 | credit_card_no | semmle.label | credit_card_no |
| test.rs:27:9:27:26 | ...::digest | semmle.label | ...::digest |
diff --git a/rust/ql/test/query-tests/security/CWE-328/test.rs b/rust/ql/test/query-tests/security/CWE-328/test.rs
index 1c97bf5773ac..768ab8a3f431 100644
--- a/rust/ql/test/query-tests/security/CWE-328/test.rs
+++ b/rust/ql/test/query-tests/security/CWE-328/test.rs
@@ -17,8 +17,8 @@ fn test_hash_algorithms(
// MD5 (alternative / older library)
_ = md5_alt::compute(harmless);
- _ = md5_alt::compute(credit_card_no); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
- _ = md5_alt::compute(password); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+ _ = md5_alt::compute(credit_card_no); // $ Source Alert[rust/weak-sensitive-data-hashing]
+ _ = md5_alt::compute(password); // $ Source Alert[rust/weak-sensitive-data-hashing]
_ = md5_alt::compute(encrypted_password);
// SHA-1
From 59386597c382e74c04eb824eff33bbcc587bf3c8 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Thu, 9 Jan 2025 18:02:00 +0000
Subject: [PATCH 06/18] Rust: Add .qhelp.
---
.../CWE-328/WeakSensitiveDataHashing.qhelp | 109 ++++++++++++++++++
.../CWE-328/WeakSensitiveDataHashingBad.rs | 10 ++
.../CWE-328/WeakSensitiveDataHashingGood.rs | 11 ++
3 files changed, 130 insertions(+)
create mode 100755 rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp
create mode 100755 rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashingBad.rs
create mode 100755 rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashingGood.rs
diff --git a/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp
new file mode 100755
index 000000000000..6da303a42ce7
--- /dev/null
+++ b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp
@@ -0,0 +1,109 @@
+<!DOCTYPE qhelp PUBLIC
+ "-//Semmle//qhelp//EN"
+ "qhelp.dtd">
+<qhelp>
+ <overview>
+ <p>
+ Using 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>
+ 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>
+ </references>
+
+</qhelp>
diff --git a/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashingBad.rs b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashingBad.rs
new file mode 100755
index 000000000000..078fbe982e27
--- /dev/null
+++ b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashingBad.rs
@@ -0,0 +1,10 @@
+// MD5 is not appropriate for hashing sensitive data.
+let mut md5_hasher = md5::Md5::new();
+...
+md5_hasher.update(emergency_contact); // BAD
+md5_hasher.update(credit_card_no); // BAD
+...
+my_hash = md5_hasher.finalize();
+
+// SHA3-256 is not appropriate for hashing passwords.
+my_hash = sha3::Sha3_256::digest(password); // BAD
diff --git a/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashingGood.rs b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashingGood.rs
new file mode 100755
index 000000000000..e3417a488361
--- /dev/null
+++ b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashingGood.rs
@@ -0,0 +1,11 @@
+// SHA3-256 *is* appropriate for hashing sensitive data.
+let mut sha3_256_hasher = sha3::Sha3_256::new();
+...
+sha3_256_hasher.update(emergency_contact); // GOOD
+sha3_256_hasher.update(credit_card_no); // GOOD
+...
+my_hash = sha3_256_hasher.finalize();
+
+// Argon2 is appropriate for hashing passwords.
+let argon2_salt = argon2::password_hash::Salt::from_b64(salt)?;
+my_hash = argon2::Argon2::default().hash_password(password.as_bytes(), argon2_salt)?.to_string(); // GOOD
From 9b8f561614e963141af912371ad551e6695755da Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 10 Jan 2025 09:23:54 +0000
Subject: [PATCH 07/18] Rust: Add another reference.
---
.../queries/security/CWE-328/WeakSensitiveDataHashing.qhelp | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp
index 6da303a42ce7..0c25aa127417 100755
--- a/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp
+++ b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp
@@ -103,6 +103,11 @@
<a href="https://github.com/RustCrypto/password-hashes?tab=readme-ov-file#rustcrypto-password-hashes">
RustCrypto: Password Hashes
</a>
+ The RustCrypto Book:
+ <a href="https://rustcrypto.org/key-derivation/hashing-password.html">
+ Password Hashing
+ </a>
+
</li>
</references>
From ae26cd6c32366d7491d1cd82a2fd7cc22053e6d1 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 10 Jan 2025 12:13:21 +0000
Subject: [PATCH 08/18] Rust: Update test for changes on main.
---
.../CWE-328/WeakSensitiveDataHashing.expected | 26 +++++++++----------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.expected b/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.expected
index 888902d913d1..69e03bcca1ca 100644
--- a/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.expected
+++ b/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.expected
@@ -13,19 +13,19 @@
| test.rs:64:9:64:24 | ...::digest | test.rs:64:26:64:37 | password_vec | test.rs:64:9:64:24 | ...::digest | $@ is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. | test.rs:64:26:64:37 | password_vec | Sensitive data (password) |
| test.rs:77:9:77:33 | ...::new_with_prefix | test.rs:77:35:77:42 | password | test.rs:77:9:77:33 | ...::new_with_prefix | $@ is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. | test.rs:77:35:77:42 | password | Sensitive data (password) |
edges
-| test.rs:14:26:14:39 | credit_card_no | test.rs:14:9:14:24 | ...::digest | provenance | MaD:1 |
-| test.rs:15:26:15:33 | password | test.rs:15:9:15:24 | ...::digest | provenance | MaD:1 |
-| test.rs:20:26:20:39 | credit_card_no | test.rs:20:9:20:24 | ...::compute | provenance | MaD:3 |
-| test.rs:21:26:21:33 | password | test.rs:21:9:21:24 | ...::compute | provenance | MaD:3 |
-| test.rs:26:28:26:41 | credit_card_no | test.rs:26:9:26:26 | ...::digest | provenance | MaD:1 |
-| test.rs:27:28:27:35 | password | test.rs:27:9:27:26 | ...::digest | provenance | MaD:1 |
-| test.rs:32:36:32:49 | credit_card_no | test.rs:32:9:32:34 | ...::digest | provenance | MaD:1 |
-| test.rs:33:36:33:43 | password | test.rs:33:9:33:34 | ...::digest | provenance | MaD:1 |
-| test.rs:39:32:39:39 | password | test.rs:39:9:39:30 | ...::digest | provenance | MaD:1 |
-| test.rs:60:26:60:37 | password_str | test.rs:60:9:60:24 | ...::digest | provenance | MaD:1 |
-| test.rs:62:26:62:37 | password_arr | test.rs:62:9:62:24 | ...::digest | provenance | MaD:1 |
-| test.rs:64:26:64:37 | password_vec | test.rs:64:9:64:24 | ...::digest | provenance | MaD:1 |
-| test.rs:77:35:77:42 | password | test.rs:77:9:77:33 | ...::new_with_prefix | provenance | MaD:2 |
+| test.rs:14:26:14:39 | credit_card_no | test.rs:14:9:14:24 | ...::digest | provenance | MaD:1 Sink:MaD:1 |
+| test.rs:15:26:15:33 | password | test.rs:15:9:15:24 | ...::digest | provenance | MaD:1 Sink:MaD:1 |
+| test.rs:20:26:20:39 | credit_card_no | test.rs:20:9:20:24 | ...::compute | provenance | MaD:3 Sink:MaD:3 |
+| test.rs:21:26:21:33 | password | test.rs:21:9:21:24 | ...::compute | provenance | MaD:3 Sink:MaD:3 |
+| test.rs:26:28:26:41 | credit_card_no | test.rs:26:9:26:26 | ...::digest | provenance | MaD:1 Sink:MaD:1 |
+| test.rs:27:28:27:35 | password | test.rs:27:9:27:26 | ...::digest | provenance | MaD:1 Sink:MaD:1 |
+| test.rs:32:36:32:49 | credit_card_no | test.rs:32:9:32:34 | ...::digest | provenance | MaD:1 Sink:MaD:1 |
+| test.rs:33:36:33:43 | password | test.rs:33:9:33:34 | ...::digest | provenance | MaD:1 Sink:MaD:1 |
+| test.rs:39:32:39:39 | password | test.rs:39:9:39:30 | ...::digest | provenance | MaD:1 Sink:MaD:1 |
+| test.rs:60:26:60:37 | password_str | test.rs:60:9:60:24 | ...::digest | provenance | MaD:1 Sink:MaD:1 |
+| test.rs:62:26:62:37 | password_arr | test.rs:62:9:62:24 | ...::digest | provenance | MaD:1 Sink:MaD:1 |
+| test.rs:64:26:64:37 | password_vec | test.rs:64:9:64:24 | ...::digest | provenance | MaD:1 Sink:MaD:1 |
+| test.rs:77:35:77:42 | password | test.rs:77:9:77:33 | ...::new_with_prefix | provenance | MaD:2 Sink:MaD:2 |
models
| 1 | Sink: repo:https://github.com/RustCrypto/traits:digest; <_ as crate::digest::Digest>::digest; hasher-input; Argument[0] |
| 2 | Sink: repo:https://github.com/RustCrypto/traits:digest; <_ as crate::digest::Digest>::new_with_prefix; hasher-input; Argument[0] |
From c115169dbeaaae3c1396a25bb6a21eee326aaecc Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 10 Jan 2025 12:35:45 +0000
Subject: [PATCH 09/18] Rust: Move ModelledHashOperation to a more logical
location.
---
.../rust/frameworks/rustcrypto/RustCrypto.qll | 28 -------------------
.../WeakSensitiveDataHashingExtensions.qll | 28 +++++++++++++++++++
2 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/rust/ql/lib/codeql/rust/frameworks/rustcrypto/RustCrypto.qll b/rust/ql/lib/codeql/rust/frameworks/rustcrypto/RustCrypto.qll
index 8a91044891f6..9dd40004766a 100644
--- a/rust/ql/lib/codeql/rust/frameworks/rustcrypto/RustCrypto.qll
+++ b/rust/ql/lib/codeql/rust/frameworks/rustcrypto/RustCrypto.qll
@@ -5,9 +5,6 @@
private import rust
private import codeql.rust.Concepts
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
bindingset[algorithmName]
private string simplifyAlgorithmName(string algorithmName) {
@@ -58,28 +55,3 @@ class StreamCipherInit extends Cryptography::CryptographicOperation::Range {
override Cryptography::BlockMode getBlockMode() { result = "" }
}
-
-/**
- * An externally modelled operation that hashes data, for example a call to `md5::Md5::digest(data)`.
- */
-class ModelledHashOperation extends Cryptography::CryptographicOperation::Range {
- DataFlow::Node input;
- CallExpr call;
- string algorithmName;
-
- ModelledHashOperation() {
- 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::CryptographicAlgorithm getAlgorithm() { result.matchesName(algorithmName) }
-
- override DataFlow::Node getAnInput() { result = input }
-
- override Cryptography::BlockMode getBlockMode() { none() } // (does not apply for hashing)
-}
diff --git a/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll b/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll
index bed193de32a0..90c57db3b669 100644
--- a/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll
+++ b/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll
@@ -12,6 +12,9 @@ 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
@@ -169,3 +172,28 @@ module ComputationallyExpensiveHashFunction {
}
}
}
+
+/**
+ * An externally modelled operation that hashes data, for example a call to `md5::Md5::digest(data)`.
+ */
+class ModelledHashOperation extends Cryptography::CryptographicOperation::Range {
+ DataFlow::Node input;
+ CallExpr call;
+ string algorithmName;
+
+ ModelledHashOperation() {
+ 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::CryptographicAlgorithm getAlgorithm() { result.matchesName(algorithmName) }
+
+ override DataFlow::Node getAnInput() { result = input }
+
+ override Cryptography::BlockMode getBlockMode() { none() } // (does not apply for hashing)
+}
From bb4322cf7c436ff29ed485d423ba7fb9b9542fbb Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 10 Jan 2025 12:39:10 +0000
Subject: [PATCH 10/18] Rust: Make a type more accurate.
---
.../codeql/rust/security/WeakSensitiveDataHashingExtensions.qll | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll b/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll
index 90c57db3b669..37caec9e64c8 100644
--- a/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll
+++ b/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll
@@ -191,7 +191,7 @@ class ModelledHashOperation extends Cryptography::CryptographicOperation::Range
override DataFlow::Node getInitialization() { result = this }
- override Cryptography::CryptographicAlgorithm getAlgorithm() { result.matchesName(algorithmName) }
+ override Cryptography::HashingAlgorithm getAlgorithm() { result.matchesName(algorithmName) }
override DataFlow::Node getAnInput() { result = input }
From 39a38c4c53325c4f8eaf75b887cd43f0a504cb99 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 10 Jan 2025 12:48:53 +0000
Subject: [PATCH 11/18] Rust: Tweak .qhelp layout.
---
.../CWE-328/WeakSensitiveDataHashing.qhelp | 62 ++++++++++---------
1 file changed, 32 insertions(+), 30 deletions(-)
diff --git a/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp
index 0c25aa127417..ffe905ea4231 100755
--- a/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp
+++ b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp
@@ -4,31 +4,31 @@
<qhelp>
<overview>
<p>
- Using a broken or weak cryptographic hash function can leave data
+ 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:
+ <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>
- <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.
@@ -51,18 +51,17 @@
<p>
Ensure that you use a strong, modern cryptographic hash function, such as:
+ <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>
- <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
@@ -95,6 +94,8 @@
<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
@@ -103,11 +104,12 @@
<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>
From ad268220bfed2ba23c3a214642bc0266c42fd84b Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 10 Jan 2025 12:53:12 +0000
Subject: [PATCH 12/18] Rust: Address QL-for-QL comments.
---
.../WeakSensitiveDataHashingExtensions.qll | 27 ++++++++++---------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll b/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll
index 37caec9e64c8..80612d615764 100644
--- a/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll
+++ b/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll
@@ -57,7 +57,7 @@ module NormalHashFunction {
abstract class Barrier extends DataFlow::Node { }
/**
- * A flow source modelled by the `SensitiveData` library.
+ * A flow source modeled by the `SensitiveData` library.
*/
class SensitiveDataAsSource extends Source instanceof SensitiveData {
SensitiveDataAsSource() {
@@ -71,7 +71,7 @@ module NormalHashFunction {
}
/**
- * A flow sink modelled by the `Cryptography` module.
+ * A flow sink modeled by the `Cryptography` module.
*/
class WeakHashingOperationInputAsSink extends Sink {
Cryptography::HashingAlgorithm algorithm;
@@ -134,7 +134,7 @@ module ComputationallyExpensiveHashFunction {
abstract class Barrier extends DataFlow::Node { }
/**
- * A flow source modelled by the `SensitiveData` library.
+ * A flow source modeled by the `SensitiveData` library.
*/
class PasswordAsSource extends Source instanceof SensitiveData {
PasswordAsSource() {
@@ -147,7 +147,7 @@ module ComputationallyExpensiveHashFunction {
}
/**
- * A flow sink modelled by the `Cryptography` module.
+ * A flow sink modeled by the `Cryptography` module.
*/
class WeakPasswordHashingOperationInputSink extends Sink {
Cryptography::CryptographicAlgorithm algorithm;
@@ -174,19 +174,20 @@ module ComputationallyExpensiveHashFunction {
}
/**
- * An externally modelled operation that hashes data, for example a call to `md5::Md5::digest(data)`.
+ * An externally modeled operation that hashes data, for example a call to `md5::Md5::digest(data)`.
*/
-class ModelledHashOperation extends Cryptography::CryptographicOperation::Range {
+class ModeledHashOperation extends Cryptography::CryptographicOperation::Range {
DataFlow::Node input;
- CallExpr call;
string algorithmName;
- ModelledHashOperation() {
- 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()
+ 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 }
From 19d3e9dbcadbc45a4f2edc5e59b9f9cb8c59d657 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 10 Jan 2025 14:19:12 +0000
Subject: [PATCH 13/18] Rust: Correct the qhelp.
---
.../CWE-328/WeakSensitiveDataHashing.qhelp | 55 ++++++++++---------
1 file changed, 28 insertions(+), 27 deletions(-)
diff --git a/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp
index ffe905ea4231..44f824d89fdc 100755
--- a/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp
+++ b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp
@@ -10,25 +10,25 @@
<p>
A strong cryptographic hash function should be resistant to:
- <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>
+ <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.
@@ -51,17 +51,18 @@
<p>
Ensure that you use a strong, modern cryptographic hash function, such as:
- <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>
+ <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
From 1b6c289cb4c222f6e65a6a705b62d134e9fd4857 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 10 Jan 2025 14:38:05 +0000
Subject: [PATCH 14/18] Rust: Unrelated MaD test impact. :(
---
.../test/library-tests/dataflow/taint/TaintFlowStep.expected | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/rust/ql/test/library-tests/dataflow/taint/TaintFlowStep.expected b/rust/ql/test/library-tests/dataflow/taint/TaintFlowStep.expected
index 5233ed1d139f..b2b6872a6d25 100644
--- a/rust/ql/test/library-tests/dataflow/taint/TaintFlowStep.expected
+++ b/rust/ql/test/library-tests/dataflow/taint/TaintFlowStep.expected
@@ -1,5 +1,5 @@
-| file://:0:0:0:0 | [summary param] 0 in lang:alloc::_::crate::fmt::format | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:alloc::_::crate::fmt::format | MaD:9 |
-| file://:0:0:0:0 | [summary param] self in lang:alloc::_::<crate::string::String>::as_str | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:alloc::_::<crate::string::String>::as_str | MaD:7 |
+| file://:0:0:0:0 | [summary param] 0 in lang:alloc::_::crate::fmt::format | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:alloc::_::crate::fmt::format | MaD:14 |
+| file://:0:0:0:0 | [summary param] self in lang:alloc::_::<crate::string::String>::as_str | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:alloc::_::<crate::string::String>::as_str | MaD:12 |
| file://:0:0:0:0 | [summary param] self in repo:https://github.com/seanmonstar/reqwest:reqwest::_::<crate::blocking::response::Response>::text | file://:0:0:0:0 | [summary] to write: ReturnValue.Variant[crate::result::Result::Ok(0)] in repo:https://github.com/seanmonstar/reqwest:reqwest::_::<crate::blocking::response::Response>::text | MaD:0 |
| main.rs:4:5:4:8 | 1000 | main.rs:4:5:4:12 | ... + ... | |
| main.rs:4:12:4:12 | i | main.rs:4:5:4:12 | ... + ... | |
From edd1f257ad6a6b8d508b5c1e2fc860b1c7eb76a9 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 10 Jan 2025 14:51:15 +0000
Subject: [PATCH 15/18] Rust: Attempt to fix the test on CI.
---
.../query-tests/security/CWE-328/WeakSensitiveDataHashing.qlref | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.qlref b/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.qlref
index 718e500931d3..fad3080280be 100644
--- a/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.qlref
+++ b/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.qlref
@@ -1,4 +1,4 @@
-query: queries/Security/CWE-328/WeakSensitiveDataHashing.ql
+query: queries/security/CWE-328/WeakSensitiveDataHashing.ql
postprocess:
- utils/test/PrettyPrintModels.ql
- utils/test/InlineExpectationsTestQuery.ql
From 722b7bb55b218b5449614e19de3a7334bb27b713 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Mon, 13 Jan 2025 10:28:08 +0000
Subject: [PATCH 16/18] Apply suggestions from code review
Co-authored-by: mc <42146119+mchammer01@users.noreply.github.com>
---
.../CWE-328/WeakSensitiveDataHashing.qhelp | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp
index 44f824d89fdc..8ee0fc81a807 100755
--- a/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp
+++ b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp
@@ -5,7 +5,7 @@
<overview>
<p>
A broken or weak cryptographic hash function can leave data
- vulnerable, and should not be used in security related code.
+ vulnerable, and should not be used in security-related code.
</p>
<p>
@@ -24,7 +24,7 @@
</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>
+ 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>
@@ -37,7 +37,8 @@
<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.
+
+ SHA-384, and SHA-512, which are in the SHA-2 family.
</p>
<p>
@@ -94,7 +95,8 @@
and
<a href="https://cheatsheetseries.owasp.org/cheatsheets/Transport_Layer_Security_Cheat_Sheet.html">
Transport Layer Security Cheat Sheet
- </a>
+
+ </a>.
</li>
<li>
GitHub:
@@ -104,13 +106,13 @@
and
<a href="https://github.com/RustCrypto/password-hashes?tab=readme-ov-file#rustcrypto-password-hashes">
RustCrypto: Password Hashes
- </a>
+ </a>.
</li>
<li>
The RustCrypto Book:
<a href="https://rustcrypto.org/key-derivation/hashing-password.html">
Password Hashing
- </a>
+ </a>.
</li>
</references>
From 676141bbb9d50ad4dbf33a758eeba59c5ac6eb0a Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Mon, 13 Jan 2025 10:36:46 +0000
Subject: [PATCH 17/18] Rust: More suggestions from review.
---
.../WeakSensitiveDataHashingExtensions.qll | 10 +++++-----
.../CWE-328/WeakSensitiveDataHashing.qhelp | 1 -
.../security/CWE-328/WeakSensitiveDataHashing.ql | 16 ++++++++--------
3 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll b/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll
index 80612d615764..8407ee4467d1 100644
--- a/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll
+++ b/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll
@@ -61,12 +61,12 @@ module NormalHashFunction {
*/
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)
+ not SensitiveData.super.getClassification() = SensitiveDataClassification::password() and // (covered in ComputationallyExpensiveHashFunction)
+ not SensitiveData.super.getClassification() = SensitiveDataClassification::id() // (not accurate enough)
}
override SensitiveDataClassification getClassification() {
- result = this.(SensitiveData).getClassification()
+ result = SensitiveData.super.getClassification()
}
}
@@ -138,11 +138,11 @@ module ComputationallyExpensiveHashFunction {
*/
class PasswordAsSource extends Source instanceof SensitiveData {
PasswordAsSource() {
- this.(SensitiveData).getClassification() = SensitiveDataClassification::password()
+ SensitiveData.super.getClassification() = SensitiveDataClassification::password()
}
override SensitiveDataClassification getClassification() {
- result = this.(SensitiveData).getClassification()
+ result = SensitiveData.super.getClassification()
}
}
diff --git a/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp
index 8ee0fc81a807..9fbeb22d39db 100755
--- a/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp
+++ b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp
@@ -95,7 +95,6 @@
and
<a href="https://cheatsheetseries.owasp.org/cheatsheets/Transport_Layer_Security_Cheat_Sheet.html">
Transport Layer Security Cheat Sheet
-
</a>.
</li>
<li>
diff --git a/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql
index 8779ce56d273..b7906d9af127 100755
--- a/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql
+++ b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql
@@ -43,7 +43,7 @@ module NormalHashFunctionFlow {
}
}
- module Flow = TaintTracking::Global<Config>;
+ import TaintTracking::Global<Config>
}
/**
@@ -74,7 +74,7 @@ module ComputationallyExpensiveHashFunctionFlow {
}
}
- module Flow = TaintTracking::Global<Config>;
+ import TaintTracking::Global<Config>
}
/**
@@ -83,10 +83,10 @@ module ComputationallyExpensiveHashFunctionFlow {
* merged to generate a combined path graph.
*/
module WeakSensitiveDataHashingFlow =
- DataFlow::MergePathGraph<NormalHashFunctionFlow::Flow::PathNode,
- ComputationallyExpensiveHashFunctionFlow::Flow::PathNode,
- NormalHashFunctionFlow::Flow::PathGraph,
- ComputationallyExpensiveHashFunctionFlow::Flow::PathGraph>;
+ DataFlow::MergePathGraph<NormalHashFunctionFlow::PathNode,
+ ComputationallyExpensiveHashFunctionFlow::PathNode,
+ NormalHashFunctionFlow::PathGraph,
+ ComputationallyExpensiveHashFunctionFlow::PathGraph>;
import WeakSensitiveDataHashingFlow::PathGraph
@@ -94,12 +94,12 @@ from
WeakSensitiveDataHashingFlow::PathNode source, WeakSensitiveDataHashingFlow::PathNode sink,
string ending, string algorithmName, string classification
where
- NormalHashFunctionFlow::Flow::flowPath(source.asPathNode1(), sink.asPathNode1()) and
+ NormalHashFunctionFlow::flowPath(source.asPathNode1(), sink.asPathNode1()) and
algorithmName = sink.getNode().(NormalHashFunction::Sink).getAlgorithmName() and
classification = source.getNode().(NormalHashFunction::Source).getClassification() and
ending = "."
or
- ComputationallyExpensiveHashFunctionFlow::Flow::flowPath(source.asPathNode2(), sink.asPathNode2()) and
+ ComputationallyExpensiveHashFunctionFlow::flowPath(source.asPathNode2(), sink.asPathNode2()) and
algorithmName = sink.getNode().(ComputationallyExpensiveHashFunction::Sink).getAlgorithmName() and
classification =
source.getNode().(ComputationallyExpensiveHashFunction::Source).getClassification() and
From e61d6aec224c02c2bb0d403163638aade2abb0cd Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Mon, 13 Jan 2025 10:51:49 +0000
Subject: [PATCH 18/18] Rust: Autoformat.
---
.../src/queries/security/CWE-328/WeakSensitiveDataHashing.ql | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql
index b7906d9af127..b22fe5762128 100755
--- a/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql
+++ b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql
@@ -84,8 +84,7 @@ module ComputationallyExpensiveHashFunctionFlow {
*/
module WeakSensitiveDataHashingFlow =
DataFlow::MergePathGraph<NormalHashFunctionFlow::PathNode,
- ComputationallyExpensiveHashFunctionFlow::PathNode,
- NormalHashFunctionFlow::PathGraph,
+ ComputationallyExpensiveHashFunctionFlow::PathNode, NormalHashFunctionFlow::PathGraph,
ComputationallyExpensiveHashFunctionFlow::PathGraph>;
import WeakSensitiveDataHashingFlow::PathGraph