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 100% rename from rust/ql/lib/codeql/rust/frameworks/RustCrypto.qll rename to rust/ql/lib/codeql/rust/frameworks/rustcrypto/RustCrypto.qll 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..fe3fd67a8fd4 --- /dev/null +++ b/rust/ql/lib/codeql/rust/frameworks/rustcrypto/rustcrypto.model.yml @@ -0,0 +1,10 @@ +extensions: + - addsTo: + pack: codeql/rust-all + extensible: sinkModel + data: + - ["repo:https://github.com/RustCrypto/traits:digest", "<_ as crate::digest::Digest>::new_with_prefix", "Argument[0]", "hasher-input", "manual"] + - ["repo:https://github.com/RustCrypto/traits:digest", "<_ as crate::digest::Digest>::update", "Argument[0]", "hasher-input", "manual"] + - ["repo:https://github.com/RustCrypto/traits:digest", "<_ as crate::digest::Digest>::chain_update", "Argument[0]", "hasher-input", "manual"] + - ["repo:https://github.com/RustCrypto/traits:digest", "<_ as crate::digest::Digest>::digest", "Argument[0]", "hasher-input", "manual"] + - ["repo:https://github.com/stainless-steel/md5:md5", "crate::compute", "Argument[0]", "hasher-input", "manual"] 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 new file mode 100644 index 000000000000..8407ee4467d1 --- /dev/null +++ b/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll @@ -0,0 +1,200 @@ +/** + * Provides default sources, sinks and sanitizers for detecting "use of a + * broken or weak cryptographic hashing algorithm on sensitive data" + * vulnerabilities, as well as extension points for adding your own. This is + * divided into two general cases: + * - hashing sensitive data + * - hashing passwords (which requires the hashing algorithm to be + * sufficiently computationally expensive in addition to other requirements) + */ + +import rust +private import codeql.rust.Concepts +private import codeql.rust.security.SensitiveData +private import codeql.rust.dataflow.DataFlow +private import codeql.rust.dataflow.FlowSource +private import codeql.rust.dataflow.FlowSink +private import codeql.rust.dataflow.internal.DataFlowImpl + +/** + * Provides default sources, sinks and sanitizers for detecting "use of a broken or weak + * cryptographic hashing algorithm on sensitive data" vulnerabilities on sensitive data that does + * NOT require computationally expensive hashing, as well as extension points for adding your own. + * + * Also see the `ComputationallyExpensiveHashFunction` module. + */ +module NormalHashFunction { + /** + * A data flow source for "use of a broken or weak cryptographic hashing algorithm on sensitive + * data" vulnerabilities that does not require computationally expensive hashing. That is, a + * piece of sensitive data that is not a password. + */ + abstract class Source extends DataFlow::Node { + Source() { not this instanceof ComputationallyExpensiveHashFunction::Source } + + /** + * Gets the classification of the sensitive data. + */ + abstract string getClassification(); + } + + /** + * A data flow sink for "use of a broken or weak cryptographic hashing algorithm on sensitive + * data" vulnerabilities that applies to data that does not require computationally expensive + * hashing. That is, a broken or weak hashing algorithm. + */ + abstract class Sink extends DataFlow::Node { + /** + * Gets the name of the weak hashing algorithm. + */ + abstract string getAlgorithmName(); + } + + /** + * A barrier for "use of a broken or weak cryptographic hashing algorithm on sensitive data" + * vulnerabilities that applies to data that does not require computationally expensive hashing. + */ + abstract class Barrier extends DataFlow::Node { } + + /** + * A flow source modeled by the `SensitiveData` library. + */ + class SensitiveDataAsSource extends Source instanceof SensitiveData { + SensitiveDataAsSource() { + not SensitiveData.super.getClassification() = SensitiveDataClassification::password() and // (covered in ComputationallyExpensiveHashFunction) + not SensitiveData.super.getClassification() = SensitiveDataClassification::id() // (not accurate enough) + } + + override SensitiveDataClassification getClassification() { + result = SensitiveData.super.getClassification() + } + } + + /** + * A flow sink modeled by the `Cryptography` module. + */ + class WeakHashingOperationInputAsSink extends Sink { + Cryptography::HashingAlgorithm algorithm; + + WeakHashingOperationInputAsSink() { + exists(Cryptography::CryptographicOperation operation | + algorithm.isWeak() and + algorithm = operation.getAlgorithm() and + this = operation.getAnInput() + ) + } + + override string getAlgorithmName() { result = algorithm.getName() } + } +} + +/** + * Provides default sources, sinks and sanitizers for detecting "use of a broken or weak + * cryptographic hashing algorithm on sensitive data" vulnerabilities on sensitive data that DOES + * require computationally expensive hashing, as well as extension points for adding your own. + * + * Also see the `NormalHashFunction` module. + */ +module ComputationallyExpensiveHashFunction { + /** + * A data flow source for "use of a broken or weak cryptographic hashing algorithm on sensitive + * data" vulnerabilities that does require computationally expensive hashing. That is, a + * password. + */ + abstract class Source extends DataFlow::Node { + /** + * Gets the classification of the sensitive data. + */ + abstract string getClassification(); + } + + /** + * A data flow sink for "use of a broken or weak cryptographic hashing algorithm on sensitive + * data" vulnerabilities that applies to data that does require computationally expensive + * hashing. That is, a broken or weak hashing algorithm or one that is not computationally + * expensive enough for password hashing. + */ + abstract class Sink extends DataFlow::Node { + /** + * Gets the name of the weak hashing algorithm. + */ + abstract string getAlgorithmName(); + + /** + * Holds if this sink is for a computationally expensive hash function (meaning that hash + * function is just weak in some other regard. + */ + abstract predicate isComputationallyExpensive(); + } + + /** + * A barrier for "use of a broken or weak cryptographic hashing algorithm on sensitive data" + * vulnerabilities that applies to data that does require computationally expensive hashing. + */ + abstract class Barrier extends DataFlow::Node { } + + /** + * A flow source modeled by the `SensitiveData` library. + */ + class PasswordAsSource extends Source instanceof SensitiveData { + PasswordAsSource() { + SensitiveData.super.getClassification() = SensitiveDataClassification::password() + } + + override SensitiveDataClassification getClassification() { + result = SensitiveData.super.getClassification() + } + } + + /** + * A flow sink modeled by the `Cryptography` module. + */ + class WeakPasswordHashingOperationInputSink extends Sink { + Cryptography::CryptographicAlgorithm algorithm; + + WeakPasswordHashingOperationInputSink() { + exists(Cryptography::CryptographicOperation operation | + ( + algorithm instanceof Cryptography::PasswordHashingAlgorithm and + algorithm.isWeak() + or + algorithm instanceof Cryptography::HashingAlgorithm // Note that HashingAlgorithm and PasswordHashingAlgorithm are disjoint + ) and + algorithm = operation.getAlgorithm() and + this = operation.getAnInput() + ) + } + + override string getAlgorithmName() { result = algorithm.getName() } + + override predicate isComputationallyExpensive() { + algorithm instanceof Cryptography::PasswordHashingAlgorithm + } + } +} + +/** + * An externally modeled operation that hashes data, for example a call to `md5::Md5::digest(data)`. + */ +class ModeledHashOperation extends Cryptography::CryptographicOperation::Range { + DataFlow::Node input; + string algorithmName; + + ModeledHashOperation() { + exists(CallExpr call | + sinkNode(input, "hasher-input") and + call = input.(Node::FlowSummaryNode).getSinkElement().getCall() and + call = this.asExpr().getExpr() and + algorithmName = + call.getFunction().(PathExpr).getPath().getQualifier().getPart().getNameRef().getText() + ) + } + + override DataFlow::Node getInitialization() { result = this } + + override Cryptography::HashingAlgorithm getAlgorithm() { result.matchesName(algorithmName) } + + override DataFlow::Node getAnInput() { result = input } + + override Cryptography::BlockMode getBlockMode() { none() } // (does not apply for hashing) +} 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..9fbeb22d39db --- /dev/null +++ b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.qhelp @@ -0,0 +1,118 @@ + + + +

+ A broken or weak cryptographic hash function can leave data + vulnerable, and should not be used in security-related code. +

+ +

+ A strong cryptographic hash function should be resistant to: +

+ + +

+ As an example, both MD5 and SHA-1 are known to be vulnerable to collision attacks. +

+ +

+ 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. +

+ +

+ 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). +

+ + + + +

+ Ensure that you use a strong, modern cryptographic hash function, such as: +

+ + + +

+ 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. +

+ + + + +

+ 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: +

+ + +

+ To make these secure, we can use the SHA-3 algorithm for sensitive data and Argon2 for passwords: +

+ + + + +
  • + OWASP: + + Password Storage Cheat Sheet + + and + + Transport Layer Security Cheat Sheet + . +
    • +
  • + GitHub: + + RustCrypto: Hashes + + and + + RustCrypto: Password Hashes + . +
    • +
  • + The RustCrypto Book: + + Password Hashing + . +
    • +     + +     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..b22fe5762128 --- /dev/null +++ b/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql @@ -0,0 +1,116 @@ +/** + * @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) + } + } + + import TaintTracking::Global +} + +/** + * 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) + } + } + + import TaintTracking::Global +} + +/** + * 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; + +import WeakSensitiveDataHashingFlow::PathGraph + +from + WeakSensitiveDataHashingFlow::PathNode source, WeakSensitiveDataHashingFlow::PathNode sink, + string ending, string algorithmName, string classification +where + NormalHashFunctionFlow::flowPath(source.asPathNode1(), sink.asPathNode1()) and + algorithmName = sink.getNode().(NormalHashFunction::Sink).getAlgorithmName() and + classification = source.getNode().(NormalHashFunction::Source).getClassification() and + ending = "." + or + ComputationallyExpensiveHashFunctionFlow::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/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 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::_::::as_str | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:alloc::_::::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::_::::as_str | file://:0:0:0:0 | [summary] to write: ReturnValue in lang:alloc::_::::as_str | MaD:12 | | file://:0:0:0:0 | [summary param] self in repo:https://github.com/seanmonstar/reqwest:reqwest::_::::text | file://:0:0:0:0 | [summary] to write: ReturnValue.Variant[crate::result::Result::Ok(0)] in repo:https://github.com/seanmonstar/reqwest:reqwest::_::::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 | ... + ... | | 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..69e03bcca1ca --- /dev/null +++ b/rust/ql/test/query-tests/security/CWE-328/WeakSensitiveDataHashing.expected @@ -0,0 +1,60 @@ +#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) | +| 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 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] | +| 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 | +| 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 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..fad3080280be --- /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 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..768ab8a3f431 --- /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); // $ 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) + _ = md5_alt::compute(harmless); + _ = 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 + _ = sha1::Sha1::digest(harmless); + _ = 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); // $ 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); // $ Source 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, password_vec: Vec +) { + // test hashing with different code patterns + + // hash different types of data + _ = md5::Md5::digest(harmless_str); + _ = md5::Md5::digest(password_str); // $ Source Alert[rust/weak-sensitive-data-hashing] + _ = md5::Md5::digest(harmless_arr); + _ = md5::Md5::digest(password_arr); // $ Source Alert[rust/weak-sensitive-data-hashing] + _ = md5::Md5::digest(harmless_vec); + _ = md5::Md5::digest(password_vec); // $ Source 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(); // $ Source 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(); +}