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