From 766eaabdec151158dd7c70366dd9f1bc5cc1af63 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Wed, 9 Jul 2025 14:55:27 +0100 Subject: [PATCH 01/18] Move weak crypto algorithm query out of experimental --- go/ql/src/{experimental => Security}/CWE-327/CryptoLibraries.qll | 0 .../{experimental => Security}/CWE-327/WeakCryptoAlgorithm.qhelp | 0 .../src/{experimental => Security}/CWE-327/WeakCryptoAlgorithm.ql | 0 .../CWE-327/WeakCryptoAlgorithmCustomizations.qll | 0 go/ql/src/{experimental => Security}/CWE-327/examples/Crypto.go | 0 .../CWE-327/examples/InsecureRandomness.go | 0 6 files changed, 0 insertions(+), 0 deletions(-) rename go/ql/src/{experimental => Security}/CWE-327/CryptoLibraries.qll (100%) rename go/ql/src/{experimental => Security}/CWE-327/WeakCryptoAlgorithm.qhelp (100%) rename go/ql/src/{experimental => Security}/CWE-327/WeakCryptoAlgorithm.ql (100%) rename go/ql/src/{experimental => Security}/CWE-327/WeakCryptoAlgorithmCustomizations.qll (100%) rename go/ql/src/{experimental => Security}/CWE-327/examples/Crypto.go (100%) rename go/ql/src/{experimental => Security}/CWE-327/examples/InsecureRandomness.go (100%) diff --git a/go/ql/src/experimental/CWE-327/CryptoLibraries.qll b/go/ql/src/Security/CWE-327/CryptoLibraries.qll similarity index 100% rename from go/ql/src/experimental/CWE-327/CryptoLibraries.qll rename to go/ql/src/Security/CWE-327/CryptoLibraries.qll diff --git a/go/ql/src/experimental/CWE-327/WeakCryptoAlgorithm.qhelp b/go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.qhelp similarity index 100% rename from go/ql/src/experimental/CWE-327/WeakCryptoAlgorithm.qhelp rename to go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.qhelp diff --git a/go/ql/src/experimental/CWE-327/WeakCryptoAlgorithm.ql b/go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.ql similarity index 100% rename from go/ql/src/experimental/CWE-327/WeakCryptoAlgorithm.ql rename to go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.ql diff --git a/go/ql/src/experimental/CWE-327/WeakCryptoAlgorithmCustomizations.qll b/go/ql/src/Security/CWE-327/WeakCryptoAlgorithmCustomizations.qll similarity index 100% rename from go/ql/src/experimental/CWE-327/WeakCryptoAlgorithmCustomizations.qll rename to go/ql/src/Security/CWE-327/WeakCryptoAlgorithmCustomizations.qll diff --git a/go/ql/src/experimental/CWE-327/examples/Crypto.go b/go/ql/src/Security/CWE-327/examples/Crypto.go similarity index 100% rename from go/ql/src/experimental/CWE-327/examples/Crypto.go rename to go/ql/src/Security/CWE-327/examples/Crypto.go diff --git a/go/ql/src/experimental/CWE-327/examples/InsecureRandomness.go b/go/ql/src/Security/CWE-327/examples/InsecureRandomness.go similarity index 100% rename from go/ql/src/experimental/CWE-327/examples/InsecureRandomness.go rename to go/ql/src/Security/CWE-327/examples/InsecureRandomness.go From f1af8b8f5a770d28d2945dc8326e203723b7d164 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Fri, 18 Jul 2025 22:28:40 +0100 Subject: [PATCH 02/18] Convert test to inline expectations --- .../query-tests/Security/CWE-327/Crypto.go | 12 ++++---- .../CWE-327/WeakCryptoAlgorithm.expected | 28 +++++++++---------- .../CWE-327/WeakCryptoAlgorithm.qlref | 5 +++- 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/go/ql/test/query-tests/Security/CWE-327/Crypto.go b/go/ql/test/query-tests/Security/CWE-327/Crypto.go index 75229b020a86..a58052df38d7 100644 --- a/go/ql/test/query-tests/Security/CWE-327/Crypto.go +++ b/go/ql/test/query-tests/Security/CWE-327/Crypto.go @@ -13,19 +13,21 @@ func crypto() { public := []byte("hello") password := []byte("123456") - buf := password // testing dataflow by passing into different variable + + // testing dataflow by passing into different variable + buf := password // $ Source // BAD, des is a weak crypto algorithm and password is sensitive data - des.NewTripleDESCipher(buf) + des.NewTripleDESCipher(buf) // $ Alert // BAD, md5 is a weak crypto algorithm and password is sensitive data - md5.Sum(buf) + md5.Sum(buf) // $ Alert // BAD, rc4 is a weak crypto algorithm and password is sensitive data - rc4.NewCipher(buf) + rc4.NewCipher(buf) // $ Alert // BAD, sha1 is a weak crypto algorithm and password is sensitive data - sha1.Sum(buf) + sha1.Sum(buf) // $ Alert // GOOD, password is sensitive data but aes is a strong crypto algorithm aes.NewCipher(buf) diff --git a/go/ql/test/query-tests/Security/CWE-327/WeakCryptoAlgorithm.expected b/go/ql/test/query-tests/Security/CWE-327/WeakCryptoAlgorithm.expected index 53cfd40145d3..6f40dfcc7ad0 100644 --- a/go/ql/test/query-tests/Security/CWE-327/WeakCryptoAlgorithm.expected +++ b/go/ql/test/query-tests/Security/CWE-327/WeakCryptoAlgorithm.expected @@ -1,17 +1,17 @@ +#select +| Crypto.go:21:25:21:27 | buf | Crypto.go:18:9:18:16 | password | Crypto.go:21:25:21:27 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:18:9:18:16 | password | Sensitive data | +| Crypto.go:24:10:24:12 | buf | Crypto.go:18:9:18:16 | password | Crypto.go:24:10:24:12 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:18:9:18:16 | password | Sensitive data | +| Crypto.go:27:16:27:18 | buf | Crypto.go:18:9:18:16 | password | Crypto.go:27:16:27:18 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:18:9:18:16 | password | Sensitive data | +| Crypto.go:30:11:30:13 | buf | Crypto.go:18:9:18:16 | password | Crypto.go:30:11:30:13 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:18:9:18:16 | password | Sensitive data | edges -| Crypto.go:16:9:16:16 | password | Crypto.go:19:25:19:27 | buf | provenance | | -| Crypto.go:16:9:16:16 | password | Crypto.go:22:10:22:12 | buf | provenance | | -| Crypto.go:16:9:16:16 | password | Crypto.go:25:16:25:18 | buf | provenance | | -| Crypto.go:16:9:16:16 | password | Crypto.go:28:11:28:13 | buf | provenance | | +| Crypto.go:18:9:18:16 | password | Crypto.go:21:25:21:27 | buf | provenance | | +| Crypto.go:18:9:18:16 | password | Crypto.go:24:10:24:12 | buf | provenance | | +| Crypto.go:18:9:18:16 | password | Crypto.go:27:16:27:18 | buf | provenance | | +| Crypto.go:18:9:18:16 | password | Crypto.go:30:11:30:13 | buf | provenance | | nodes -| Crypto.go:16:9:16:16 | password | semmle.label | password | -| Crypto.go:19:25:19:27 | buf | semmle.label | buf | -| Crypto.go:22:10:22:12 | buf | semmle.label | buf | -| Crypto.go:25:16:25:18 | buf | semmle.label | buf | -| Crypto.go:28:11:28:13 | buf | semmle.label | buf | +| Crypto.go:18:9:18:16 | password | semmle.label | password | +| Crypto.go:21:25:21:27 | buf | semmle.label | buf | +| Crypto.go:24:10:24:12 | buf | semmle.label | buf | +| Crypto.go:27:16:27:18 | buf | semmle.label | buf | +| Crypto.go:30:11:30:13 | buf | semmle.label | buf | subpaths -#select -| Crypto.go:19:25:19:27 | buf | Crypto.go:16:9:16:16 | password | Crypto.go:19:25:19:27 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:16:9:16:16 | password | Sensitive data | -| Crypto.go:22:10:22:12 | buf | Crypto.go:16:9:16:16 | password | Crypto.go:22:10:22:12 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:16:9:16:16 | password | Sensitive data | -| Crypto.go:25:16:25:18 | buf | Crypto.go:16:9:16:16 | password | Crypto.go:25:16:25:18 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:16:9:16:16 | password | Sensitive data | -| Crypto.go:28:11:28:13 | buf | Crypto.go:16:9:16:16 | password | Crypto.go:28:11:28:13 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:16:9:16:16 | password | Sensitive data | diff --git a/go/ql/test/query-tests/Security/CWE-327/WeakCryptoAlgorithm.qlref b/go/ql/test/query-tests/Security/CWE-327/WeakCryptoAlgorithm.qlref index 00d68df5a7c5..cdc89fa3080b 100644 --- a/go/ql/test/query-tests/Security/CWE-327/WeakCryptoAlgorithm.qlref +++ b/go/ql/test/query-tests/Security/CWE-327/WeakCryptoAlgorithm.qlref @@ -1 +1,4 @@ -experimental/CWE-327/WeakCryptoAlgorithm.ql +query: Security/CWE-327/WeakCryptoAlgorithm.ql +postprocess: + - utils/test/PrettyPrintModels.ql + - utils/test/InlineExpectationsTestQuery.ql From a0ba752faa647da4dd2ce6f388e94909a9a0a5dc Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Wed, 9 Jul 2025 14:55:54 +0100 Subject: [PATCH 03/18] Remove `experimental` tag from query metadata --- go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.ql | 1 - 1 file changed, 1 deletion(-) diff --git a/go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.ql b/go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.ql index ffaaae8e02a5..c649bc8e374e 100644 --- a/go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.ql +++ b/go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.ql @@ -5,7 +5,6 @@ * @problem.severity error * @id go/weak-crypto-algorithm * @tags security - * experimental * external/cwe/cwe-327 * external/cwe/cwe-328 */ From 993afd26fbe0ce784cdd5abdf94629641f1687ff Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Thu, 10 Jul 2025 16:29:48 +0100 Subject: [PATCH 04/18] Align metadata with related queries --- go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.ql | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.ql b/go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.ql index c649bc8e374e..3de3a8d34ce1 100644 --- a/go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.ql +++ b/go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.ql @@ -1,8 +1,10 @@ /** - * @name Use of a weak cryptographic algorithm - * @description Using weak cryptographic algorithms can allow an attacker to compromise security. + * @name Use of a broken or weak cryptographic algorithm + * @description Using broken or weak cryptographic algorithms can compromise security. * @kind path-problem - * @problem.severity error + * @problem.severity warning + * @security-severity 7.5 + * @precision high * @id go/weak-crypto-algorithm * @tags security * external/cwe/cwe-327 From dbd1bfd4172a4bcb8c07e0221e7b84075f249038 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Thu, 17 Jul 2025 14:56:38 +0100 Subject: [PATCH 05/18] Move crypto qll files from query pack to library pack --- .../CWE-327 => lib/semmle/go/frameworks}/CryptoLibraries.qll | 0 .../semmle/go/security}/WeakCryptoAlgorithmCustomizations.qll | 2 +- go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.ql | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename go/ql/{src/Security/CWE-327 => lib/semmle/go/frameworks}/CryptoLibraries.qll (100%) rename go/ql/{src/Security/CWE-327 => lib/semmle/go/security}/WeakCryptoAlgorithmCustomizations.qll (97%) diff --git a/go/ql/src/Security/CWE-327/CryptoLibraries.qll b/go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll similarity index 100% rename from go/ql/src/Security/CWE-327/CryptoLibraries.qll rename to go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll diff --git a/go/ql/src/Security/CWE-327/WeakCryptoAlgorithmCustomizations.qll b/go/ql/lib/semmle/go/security/WeakCryptoAlgorithmCustomizations.qll similarity index 97% rename from go/ql/src/Security/CWE-327/WeakCryptoAlgorithmCustomizations.qll rename to go/ql/lib/semmle/go/security/WeakCryptoAlgorithmCustomizations.qll index b9104f1fe096..6e25789531f4 100644 --- a/go/ql/src/Security/CWE-327/WeakCryptoAlgorithmCustomizations.qll +++ b/go/ql/lib/semmle/go/security/WeakCryptoAlgorithmCustomizations.qll @@ -5,8 +5,8 @@ */ import go +private import semmle.go.frameworks.CryptoLibraries private import semmle.go.security.SensitiveActions -private import CryptoLibraries /** * Provides default sources, sinks and sanitizers for reasoning about diff --git a/go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.ql b/go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.ql index 3de3a8d34ce1..58adfc003440 100644 --- a/go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.ql +++ b/go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.ql @@ -12,7 +12,7 @@ */ import go -import WeakCryptoAlgorithmCustomizations +import semmle.go.security.WeakCryptoAlgorithmCustomizations import WeakCryptoAlgorithm::Flow::PathGraph from WeakCryptoAlgorithm::Flow::PathNode source, WeakCryptoAlgorithm::Flow::PathNode sink From ae3849203d78faf9da15c839c06eb9eaaad0b303 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Fri, 18 Jul 2025 16:14:45 +0100 Subject: [PATCH 06/18] Copy the structure of the Javascript query --- go/ql/lib/go.qll | 1 + go/ql/lib/qlpack.yml | 1 + go/ql/lib/semmle/go/Concepts.qll | 32 ++++ .../semmle/go/frameworks/CryptoLibraries.qll | 177 ++---------------- .../BrokenCryptoAlgorithmCustomizations.qll | 58 ++++++ .../security/BrokenCryptoAlgorithmQuery.qll | 41 ++++ .../WeakCryptoAlgorithmCustomizations.qll | 66 ------- ...ithm.qhelp => BrokenCryptoAlgorithm.qhelp} | 0 ...oAlgorithm.ql => BrokenCryptoAlgorithm.ql} | 8 +- ...xpected => BrokenCryptoAlgorithm.expected} | 0 ...ithm.qlref => BrokenCryptoAlgorithm.qlref} | 2 +- 11 files changed, 152 insertions(+), 234 deletions(-) create mode 100644 go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmCustomizations.qll create mode 100644 go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmQuery.qll delete mode 100644 go/ql/lib/semmle/go/security/WeakCryptoAlgorithmCustomizations.qll rename go/ql/src/Security/CWE-327/{WeakCryptoAlgorithm.qhelp => BrokenCryptoAlgorithm.qhelp} (100%) rename go/ql/src/Security/CWE-327/{WeakCryptoAlgorithm.ql => BrokenCryptoAlgorithm.ql} (66%) rename go/ql/test/query-tests/Security/CWE-327/{WeakCryptoAlgorithm.expected => BrokenCryptoAlgorithm.expected} (100%) rename go/ql/test/query-tests/Security/CWE-327/{WeakCryptoAlgorithm.qlref => BrokenCryptoAlgorithm.qlref} (65%) diff --git a/go/ql/lib/go.qll b/go/ql/lib/go.qll index 16f2f1702faa..688214aae855 100644 --- a/go/ql/lib/go.qll +++ b/go/ql/lib/go.qll @@ -33,6 +33,7 @@ import semmle.go.frameworks.AwsLambda import semmle.go.frameworks.Beego import semmle.go.frameworks.BeegoOrm import semmle.go.frameworks.Bun +import semmle.go.frameworks.CryptoLibraries import semmle.go.frameworks.RsCors import semmle.go.frameworks.Couchbase import semmle.go.frameworks.Echo diff --git a/go/ql/lib/qlpack.yml b/go/ql/lib/qlpack.yml index bf2586d9089e..145bb4747ff6 100644 --- a/go/ql/lib/qlpack.yml +++ b/go/ql/lib/qlpack.yml @@ -6,6 +6,7 @@ extractor: go library: true upgrades: upgrades dependencies: + codeql/concepts: ${workspace} codeql/dataflow: ${workspace} codeql/mad: ${workspace} codeql/threat-models: ${workspace} diff --git a/go/ql/lib/semmle/go/Concepts.qll b/go/ql/lib/semmle/go/Concepts.qll index 1931f16871ab..0f30519d0beb 100644 --- a/go/ql/lib/semmle/go/Concepts.qll +++ b/go/ql/lib/semmle/go/Concepts.qll @@ -8,6 +8,10 @@ import go import semmle.go.dataflow.FunctionInputsAndOutputs import semmle.go.concepts.HTTP import semmle.go.concepts.GeneratedFile +private import codeql.concepts.ConceptsShared +private import semmle.go.dataflow.internal.DataFlowImplSpecific + +private module ConceptsShared = ConceptsMake; /** * A data-flow node that executes an operating system command, @@ -505,3 +509,31 @@ module UnmarshalingFunction { abstract string getFormat(); } } + +/** + * Provides models for cryptographic things. + */ +module Cryptography { + private import ConceptsShared::Cryptography as SC + + /** + * A data-flow node that is an application of a cryptographic algorithm. For example, + * encryption, decryption, signature-validation. + * + * Extend this class to refine existing API models. If you want to model new APIs, + * extend `CryptographicOperation::Range` instead. + */ + class CryptographicOperation extends SC::CryptographicOperation { } + + class EncryptionAlgorithm = SC::EncryptionAlgorithm; + + class HashingAlgorithm = SC::HashingAlgorithm; + + class PasswordHashingAlgorithm = SC::PasswordHashingAlgorithm; + + module CryptographicOperation = SC::CryptographicOperation; + + class BlockMode = SC::BlockMode; + + class CryptographicAlgorithm = SC::CryptographicAlgorithm; +} diff --git a/go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll b/go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll index 5518067b668b..9cc0235cbbb3 100644 --- a/go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll +++ b/go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll @@ -3,180 +3,31 @@ */ import go +import semmle.go.Concepts::Cryptography +private import codeql.concepts.internal.CryptoAlgorithmNames /** - * Names of cryptographic algorithms, separated into strong and weak variants. - * - * The names are normalized: upper-case, no spaces, dashes or underscores. - * - * The names are inspired by the names used in real world crypto libraries. - * - * The classification into strong and weak are based on OWASP and Wikipedia (2020). - * - * Sources (more links in qhelp file): - * https://en.wikipedia.org/wiki/Strong_cryptography#Cryptographically_strong_algorithms - * https://en.wikipedia.org/wiki/Strong_cryptography#Examples - * https://cheatsheetseries.owasp.org/cheatsheets/Cryptographic_Storage_Cheat_Sheet.html - */ -private module AlgorithmNames { - predicate isStrongHashingAlgorithm(string name) { - name = - [ - "DSA", "ED25519", "SHA256", "SHA384", "SHA512", "SHA3", "ES256", "ECDSA256", "ES384", - "ECDSA384", "ES512", "ECDSA512", "SHA2", "SHA224" - ] - } - - predicate isWeakHashingAlgorithm(string name) { - name = - [ - "HAVEL128", "MD2", "SHA1", "MD4", "MD5", "PANAMA", "RIPEMD", "RIPEMD128", "RIPEMD256", - "RIPEMD320", "SHA0" - ] - } - - predicate isStrongEncryptionAlgorithm(string name) { - name = ["AES", "AES128", "AES192", "AES256", "AES512", "RSA", "RABBIT", "BLOWFISH"] - } - - predicate isWeakEncryptionAlgorithm(string name) { - name = - [ - "DES", "3DES", "ARC5", "RC5", "TRIPLEDES", "TDEA", "TRIPLEDEA", "ARC2", "RC2", "ARC4", - "RC4", "ARCFOUR" - ] - } - - predicate isStrongPasswordHashingAlgorithm(string name) { - name = ["ARGON2", "PBKDF2", "BCRYPT", "SCRYPT"] - } - - predicate isWeakPasswordHashingAlgorithm(string name) { none() } -} - -private import AlgorithmNames - -/** - * A cryptographic algorithm. - */ -private newtype TCryptographicAlgorithm = - MkHashingAlgorithm(string name, boolean isWeak) { - isStrongHashingAlgorithm(name) and isWeak = false - or - isWeakHashingAlgorithm(name) and isWeak = true - } or - MkEncryptionAlgorithm(string name, boolean isWeak) { - isStrongEncryptionAlgorithm(name) and isWeak = false - or - isWeakEncryptionAlgorithm(name) and isWeak = true - } or - MkPasswordHashingAlgorithm(string name, boolean isWeak) { - isStrongPasswordHashingAlgorithm(name) and isWeak = false - or - isWeakPasswordHashingAlgorithm(name) and isWeak = true - } - -/** - * A cryptographic algorithm. - */ -abstract class CryptographicAlgorithm extends TCryptographicAlgorithm { - /** Gets a textual representation of this element. */ - string toString() { result = this.getName() } - - /** - * Gets the name of this algorithm. - */ - abstract string getName(); - - /** - * Holds if the name of this algorithm matches `name` modulo case, - * white space, dashes and underscores. - */ - bindingset[name] - predicate matchesName(string name) { - exists(name.regexpReplaceAll("[-_]", "").regexpFind("(?i)\\Q" + this.getName() + "\\E", _, _)) - } - - /** - * Holds if this algorithm is weak. - */ - abstract predicate isWeak(); -} - -/** - * A hashing algorithm such as `MD5` or `SHA512`. - */ -class HashingAlgorithm extends MkHashingAlgorithm, CryptographicAlgorithm { - string name; - boolean isWeak; - - HashingAlgorithm() { this = MkHashingAlgorithm(name, isWeak) } - - override string getName() { result = name } - - override predicate isWeak() { isWeak = true } -} - -/** - * An encryption algorithm such as `DES` or `AES512`. - */ -class EncryptionAlgorithm extends MkEncryptionAlgorithm, CryptographicAlgorithm { - string name; - boolean isWeak; - - EncryptionAlgorithm() { this = MkEncryptionAlgorithm(name, isWeak) } - - override string getName() { result = name } - - override predicate isWeak() { isWeak = true } -} - -/** - * A password hashing algorithm such as `PBKDF2` or `SCRYPT`. - */ -class PasswordHashingAlgorithm extends MkPasswordHashingAlgorithm, CryptographicAlgorithm { - string name; - boolean isWeak; - - PasswordHashingAlgorithm() { this = MkPasswordHashingAlgorithm(name, isWeak) } - - override string getName() { result = name } - - override predicate isWeak() { isWeak = true } -} - -/** - * An application of a cryptographic algorithm. + * A cryptographic operation from the `crypto/md5` package. */ -abstract class CryptographicOperation extends DataFlow::Node { - /** - * Gets the input the algorithm is used on, e.g. the plain text input to be encrypted. - */ - abstract Expr getInput(); +private module CryptoMd5 { + private class Md5 extends CryptographicOperation::Range instanceof DataFlow::CallNode { + Md5() { this.getTarget().hasQualifiedName("crypto/md5", ["New", "Sum"]) } - /** - * Gets the applied algorithm. - */ - abstract CryptographicAlgorithm getAlgorithm(); -} + override DataFlow::Node getInitialization() { result = this } -/** - * A cryptographic operation from the `crypto/md5` package. - */ -class Md5 extends CryptographicOperation, DataFlow::CallNode { - Md5() { this.getTarget().hasQualifiedName("crypto/md5", ["New", "Sum"]) } + override CryptographicAlgorithm getAlgorithm() { result.matchesName("MD5") } - override Expr getInput() { result = this.getArgument(0).asExpr() } + override DataFlow::Node getAnInput() { result = super.getArgument(0) } - override CryptographicAlgorithm getAlgorithm() { - result.matchesName(this.getTarget().getPackage().getName()) + // not relevant for md5 + override BlockMode getBlockMode() { none() } } } /** * A cryptographic operation from the `crypto/sha1` package. */ -class Sha1 extends CryptographicOperation, DataFlow::CallNode { +class Sha1 extends CryptographicOperation::Range instanceof DataFlow::CallNode { Sha1() { this.getTarget().hasQualifiedName("crypto/sha1", ["New", "Sum"]) } override Expr getInput() { result = this.getArgument(0).asExpr() } @@ -189,7 +40,7 @@ class Sha1 extends CryptographicOperation, DataFlow::CallNode { /** * A cryptographic operation from the `crypto/des` package. */ -class Des extends CryptographicOperation, DataFlow::CallNode { +class Des extends CryptographicOperation::Range instanceof DataFlow::CallNode { Des() { this.getTarget().hasQualifiedName("crypto/des", ["NewCipher", "NewTripleDESCipher"]) } override Expr getInput() { result = this.getArgument(0).asExpr() } @@ -202,7 +53,7 @@ class Des extends CryptographicOperation, DataFlow::CallNode { /** * A cryptographic operation from the `crypto/rc4` package. */ -class Rc4 extends CryptographicOperation, DataFlow::CallNode { +class Rc4 extends CryptographicOperation::Range instanceof DataFlow::CallNode { Rc4() { this.getTarget().hasQualifiedName("crypto/rc4", "NewCipher") } override Expr getInput() { result = this.getArgument(0).asExpr() } diff --git a/go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmCustomizations.qll b/go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmCustomizations.qll new file mode 100644 index 000000000000..e5ac77fbb75f --- /dev/null +++ b/go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmCustomizations.qll @@ -0,0 +1,58 @@ +/** + * Provides default sources, sinks and sanitizers for reasoning about + * sensitive information in weak cryptographic algorithms, + * as well as extension points for adding your own. + */ + +import go +private import semmle.go.security.SensitiveActions + +/** + * Provides default sources, sinks and sanitizers for reasoning about + * sensitive information in weak cryptographic algorithms, + * as well as extension points for adding your own. + */ +module BrokenCryptoAlgorithm { + /** + * A data flow source for sensitive information in broken or weak cryptographic algorithms. + */ + abstract class Source extends DataFlow::Node { } + + /** + * A data flow sink for sensitive information in broken or weak cryptographic algorithms. + */ + abstract class Sink extends DataFlow::Node { + /** Gets the data-flow node where the cryptographic algorithm used in this operation is configured. */ + abstract DataFlow::Node getInitialization(); + } + + /** + * A sanitizer for sensitive information in broken or weak cryptographic algorithms. + */ + abstract class Sanitizer extends DataFlow::Node { } + + /** + * A sensitive source. + */ + class SensitiveSource extends Source { + SensitiveSource() { this.asExpr() instanceof SensitiveExpr } + } + + /** + * An expression used by a broken or weak cryptographic algorithm. + */ + class WeakCryptographicOperationSink extends Sink { + CryptographicOperation application; + + WeakCryptographicOperationSink() { + ( + application.getAlgorithm().isWeak() + or + application.getBlockMode().isWeak() + ) and + this = application.getAnInput() + } + + override DataFlow::Node getInitialization() { result = application.getInitialization() } + } +} diff --git a/go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmQuery.qll b/go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmQuery.qll new file mode 100644 index 000000000000..2e400c76c85a --- /dev/null +++ b/go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmQuery.qll @@ -0,0 +1,41 @@ +/** + * Provides a taint tracking configuration for reasoning about + * sensitive information in broken or weak cryptographic algorithms. + * + * Note, for performance reasons: only import this file if + * `BrokenCryptoAlgorithm::Configuration` is needed, otherwise + * `BrokenCryptoAlgorithmCustomizations` should be imported instead. + */ + +import go +import BrokenCryptoAlgorithmCustomizations::BrokenCryptoAlgorithm + +/** + * A taint tracking configuration for sensitive information in broken or weak cryptographic algorithms. + * + * This configuration identifies flows from `Source`s, which are sources of + * sensitive data, to `Sink`s, which is an abstract class representing all + * the places sensitive data may used in broken or weak cryptographic algorithms. Additional sources or sinks can be + * added either by extending the relevant class, or by subclassing this configuration itself, + * and amending the sources and sinks. + */ +private module BrokenCryptoAlgorithmConfig 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 Sanitizer } + + predicate observeDiffInformedIncrementalMode() { any() } + + Location getASelectedSinkLocation(DataFlow::Node sink) { + result = sink.(Sink).getLocation() + or + result = sink.(Sink).getInitialization().getLocation() + } +} + +/** + * Taint tracking flow for sensitive information in broken or weak cryptographic algorithms. + */ +module BrokenCryptoAlgorithmFlow = TaintTracking::Global; diff --git a/go/ql/lib/semmle/go/security/WeakCryptoAlgorithmCustomizations.qll b/go/ql/lib/semmle/go/security/WeakCryptoAlgorithmCustomizations.qll deleted file mode 100644 index 6e25789531f4..000000000000 --- a/go/ql/lib/semmle/go/security/WeakCryptoAlgorithmCustomizations.qll +++ /dev/null @@ -1,66 +0,0 @@ -/** - * Provides default sources, sinks and sanitizers for reasoning about - * sensitive information in weak cryptographic algorithms, - * as well as extension points for adding your own. - */ - -import go -private import semmle.go.frameworks.CryptoLibraries -private import semmle.go.security.SensitiveActions - -/** - * Provides default sources, sinks and sanitizers for reasoning about - * sensitive information in weak cryptographic algorithms, - * as well as extension points for adding your own. - */ -module WeakCryptoAlgorithm { - /** - * A data flow source for sensitive information in weak cryptographic algorithms. - */ - abstract class Source extends DataFlow::Node { } - - /** - * A data flow sink for sensitive information in weak cryptographic algorithms. - */ - abstract class Sink extends DataFlow::Node { } - - /** - * A sanitizer for sensitive information in weak cryptographic algorithms. - */ - abstract class Sanitizer extends DataFlow::Node { } - - /** - * A sensitive source. - */ - class SensitiveSource extends Source { - SensitiveSource() { this.asExpr() instanceof SensitiveExpr } - } - - /** - * An expression used by a weak cryptographic algorithm. - */ - class WeakCryptographicOperationSink extends Sink { - WeakCryptographicOperationSink() { - exists(CryptographicOperation application | - application.getAlgorithm().isWeak() and - this.asExpr() = application.getInput() - ) - } - } - - 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 Sanitizer } - - predicate observeDiffInformedIncrementalMode() { any() } - } - - /** - * Tracks taint flow from sensitive information to weak cryptographic - * algorithms. - */ - module Flow = TaintTracking::Global; -} diff --git a/go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.qhelp b/go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.qhelp similarity index 100% rename from go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.qhelp rename to go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.qhelp diff --git a/go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.ql b/go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql similarity index 66% rename from go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.ql rename to go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql index 58adfc003440..714a1cc7b3ec 100644 --- a/go/ql/src/Security/CWE-327/WeakCryptoAlgorithm.ql +++ b/go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql @@ -12,10 +12,10 @@ */ import go -import semmle.go.security.WeakCryptoAlgorithmCustomizations -import WeakCryptoAlgorithm::Flow::PathGraph +import semmle.go.security.BrokenCryptoAlgorithmQuery +import BrokenCryptoAlgorithmFlow::PathGraph -from WeakCryptoAlgorithm::Flow::PathNode source, WeakCryptoAlgorithm::Flow::PathNode sink -where WeakCryptoAlgorithm::Flow::flowPath(source, sink) +from BrokenCryptoAlgorithmFlow::PathNode source, BrokenCryptoAlgorithmFlow::PathNode sink +where BrokenCryptoAlgorithmFlow::flowPath(source, sink) select sink.getNode(), source, sink, "$@ is used in a weak cryptographic algorithm.", source.getNode(), "Sensitive data" diff --git a/go/ql/test/query-tests/Security/CWE-327/WeakCryptoAlgorithm.expected b/go/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.expected similarity index 100% rename from go/ql/test/query-tests/Security/CWE-327/WeakCryptoAlgorithm.expected rename to go/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.expected diff --git a/go/ql/test/query-tests/Security/CWE-327/WeakCryptoAlgorithm.qlref b/go/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.qlref similarity index 65% rename from go/ql/test/query-tests/Security/CWE-327/WeakCryptoAlgorithm.qlref rename to go/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.qlref index cdc89fa3080b..a618df1ed20a 100644 --- a/go/ql/test/query-tests/Security/CWE-327/WeakCryptoAlgorithm.qlref +++ b/go/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.qlref @@ -1,4 +1,4 @@ -query: Security/CWE-327/WeakCryptoAlgorithm.ql +query: Security/CWE-327/BrokenCryptoAlgorithm.ql postprocess: - utils/test/PrettyPrintModels.ql - utils/test/InlineExpectationsTestQuery.ql From f0e7681ce0292e2d5212143d47d30048044d5878 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 14 Oct 2025 13:52:20 +0100 Subject: [PATCH 07/18] Avoid duplicate results using in-barriers --- go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmQuery.qll | 2 ++ 1 file changed, 2 insertions(+) diff --git a/go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmQuery.qll b/go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmQuery.qll index 2e400c76c85a..ba24dcf57078 100644 --- a/go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmQuery.qll +++ b/go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmQuery.qll @@ -24,6 +24,8 @@ private module BrokenCryptoAlgorithmConfig implements DataFlow::ConfigSig { predicate isSink(DataFlow::Node sink) { sink instanceof Sink } + predicate isBarrierIn(DataFlow::Node node) { isSource(node) } + predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } predicate observeDiffInformedIncrementalMode() { any() } From 6e6c47608437eb9f0e73d30f0293de2a0922a461 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Wed, 15 Oct 2025 12:10:22 +0100 Subject: [PATCH 08/18] Model cryptographic operations --- go/ql/lib/semmle/go/Concepts.qll | 67 +++ .../semmle/go/frameworks/CryptoLibraries.qll | 480 +++++++++++++++++- .../CWE-327/BrokenCryptoAlgorithm.expected | 92 +++- .../query-tests/Security/CWE-327/Crypto.go | 255 +++++++++- .../Security/CWE-327/CryptoAlgorithm.expected | 2 + .../Security/CWE-327/CryptoAlgorithm.ql | 47 ++ .../test/query-tests/Security/CWE-327/go.mod | 3 + .../internal/CryptoAlgorithmNames.qll | 3 +- 8 files changed, 879 insertions(+), 70 deletions(-) create mode 100644 go/ql/test/query-tests/Security/CWE-327/CryptoAlgorithm.expected create mode 100644 go/ql/test/query-tests/Security/CWE-327/CryptoAlgorithm.ql create mode 100644 go/ql/test/query-tests/Security/CWE-327/go.mod diff --git a/go/ql/lib/semmle/go/Concepts.qll b/go/ql/lib/semmle/go/Concepts.qll index 0f30519d0beb..acb16b62d077 100644 --- a/go/ql/lib/semmle/go/Concepts.qll +++ b/go/ql/lib/semmle/go/Concepts.qll @@ -536,4 +536,71 @@ module Cryptography { class BlockMode = SC::BlockMode; class CryptographicAlgorithm = SC::CryptographicAlgorithm; + + /** A data flow node that initializes a hash algorithm. */ + abstract class HashAlgorithmInit extends DataFlow::Node { + /** Gets the hash algorithm being initialized. */ + abstract HashingAlgorithm getAlgorithm(); + } + + /** A data flow node that is an application of a hash algorithm. */ + abstract class HashOperation extends CryptographicOperation::Range { + override BlockMode getBlockMode() { none() } + } + + /** A data flow node that initializes an encryption algorithm. */ + abstract class EncryptionAlgorithmInit extends DataFlow::Node { + /** Gets the encryption algorithm being initialized. */ + abstract EncryptionAlgorithm getAlgorithm(); + } + + /** + * A data flow node that initializes a block cipher mode of operation, and + * may also propagate taint for encryption algorithms. + */ + abstract class BlockModeInit extends DataFlow::CallNode { + /** Gets the block cipher mode of operation being initialized. */ + abstract BlockMode getMode(); + + /** Gets a step propagating the encryption algorithm through this call. */ + abstract predicate step(DataFlow::Node node1, DataFlow::Node node2); + } + + /** + * A data flow node that is an application of an encryption algorithm, where + * the encryption algorithm and the block cipher mode of operation (if there + * is one) have been initialized separately. + */ + abstract class EncryptionOperation extends CryptographicOperation::Range { + DataFlow::Node encryptionFlowTarget; + DataFlow::Node inputNode; + + override DataFlow::Node getInitialization() { + EncryptionFlow::flow(result, encryptionFlowTarget) + } + + override EncryptionAlgorithm getAlgorithm() { + result = this.getInitialization().(EncryptionAlgorithmInit).getAlgorithm() + } + + override DataFlow::Node getAnInput() { result = inputNode } + + override BlockMode getBlockMode() { + result = this.getInitialization().(BlockModeInit).getMode() + } + } + + /** + * An `EncryptionOperation` which is a method call where the encryption + * algorithm and block cipher mode of operation (if there is one) flow to the + * receiver and the input is an argument. + */ + abstract class EncryptionMethodCall extends EncryptionOperation instanceof DataFlow::CallNode { + int inputArg; + + EncryptionMethodCall() { + encryptionFlowTarget = super.getReceiver() and + inputNode = super.getArgument(inputArg) + } + } } diff --git a/go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll b/go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll index 9cc0235cbbb3..0c56d8c7e6a5 100644 --- a/go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll +++ b/go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll @@ -7,58 +7,482 @@ import semmle.go.Concepts::Cryptography private import codeql.concepts.internal.CryptoAlgorithmNames /** - * A cryptographic operation from the `crypto/md5` package. + * A data flow call node that is an application of a hash operation where the + * hash algorithm is defined in any earlier initialization node, and the input + * is the first argument of the call. */ -private module CryptoMd5 { - private class Md5 extends CryptographicOperation::Range instanceof DataFlow::CallNode { - Md5() { this.getTarget().hasQualifiedName("crypto/md5", ["New", "Sum"]) } +abstract class DirectHashOperation extends HashOperation instanceof DataFlow::CallNode { + override DataFlow::Node getInitialization() { result = this } - override DataFlow::Node getInitialization() { result = this } + override DataFlow::Node getAnInput() { result = super.getArgument(0) } +} - override CryptographicAlgorithm getAlgorithm() { result.matchesName("MD5") } +private module HashConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof HashAlgorithmInit } - override DataFlow::Node getAnInput() { result = super.getArgument(0) } + predicate isSink(DataFlow::Node sink) { any() } - // not relevant for md5 - override BlockMode getBlockMode() { none() } - } + predicate observeDiffInformedIncrementalMode() { any() } } +/** Tracks the flow of hash algorithms. */ +module HashFlow = DataFlow::Global; + /** - * A cryptographic operation from the `crypto/sha1` package. + * A data flow node that initializes a block mode and propagates the encryption + * algorithm from the first argument to the receiver. */ -class Sha1 extends CryptographicOperation::Range instanceof DataFlow::CallNode { - Sha1() { this.getTarget().hasQualifiedName("crypto/sha1", ["New", "Sum"]) } +abstract class StdLibNewEncrypter extends BlockModeInit { + override predicate step(DataFlow::Node node1, DataFlow::Node node2) { + node1 = this.getArgument(0) and + node2 = this.getResult(0) + } +} + +private module EncryptionConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { + source instanceof EncryptionAlgorithmInit or + source instanceof BlockModeInit + } - override Expr getInput() { result = this.getArgument(0).asExpr() } + predicate isSink(DataFlow::Node sink) { any() } - override CryptographicAlgorithm getAlgorithm() { - result.matchesName(this.getTarget().getPackage().getName()) + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + any(BlockModeInit nbcm).step(node1, node2) } + + predicate observeDiffInformedIncrementalMode() { any() } } /** - * A cryptographic operation from the `crypto/des` package. + * Tracks algorithms and block cipher modes of operation used for encryption. */ -class Des extends CryptographicOperation::Range instanceof DataFlow::CallNode { - Des() { this.getTarget().hasQualifiedName("crypto/des", ["NewCipher", "NewTripleDESCipher"]) } +module EncryptionFlow = DataFlow::Global; + +private module Crypto { + private module Aes { + private class NewCipher extends EncryptionAlgorithmInit { + NewCipher() { + exists(Function f | this = f.getACall().getResult(0) | + f.hasQualifiedName("crypto/aes", "NewCipher") + ) + } + + override EncryptionAlgorithm getAlgorithm() { result.matchesName("AES") } + } + } + + private module Des { + private class NewCipher extends EncryptionAlgorithmInit { + NewCipher() { + exists(Function f | this = f.getACall().getResult(0) | + f.hasQualifiedName("crypto/des", "NewCipher") + ) + } + + override EncryptionAlgorithm getAlgorithm() { result.matchesName("DES") } + } + + private class NewTripleDESCipher extends EncryptionAlgorithmInit { + NewTripleDESCipher() { + exists(Function f | this = f.getACall().getResult(0) | + f.hasQualifiedName("crypto/des", "NewTripleDESCipher") + ) + } + + override EncryptionAlgorithm getAlgorithm() { result.matchesName("TRIPLEDES") } + } + } + + private module Md5 { + private class Sum extends DirectHashOperation instanceof DataFlow::CallNode { + Sum() { this.getTarget().hasQualifiedName("crypto/md5", "Sum") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("MD5") } + } + + private class New extends HashAlgorithmInit instanceof DataFlow::CallNode { + New() { this.getTarget().hasQualifiedName("crypto/md5", "New") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("MD5") } + } + } + + private module Rc4 { + private class CipherXORKeyStream extends CryptographicOperation::Range instanceof DataFlow::CallNode + { + CipherXORKeyStream() { + this.(DataFlow::MethodCallNode) + .getTarget() + .hasQualifiedName("crypto/rc4", "Cipher", "XORKeyStream") + } + + override DataFlow::Node getInitialization() { result = this } + + override EncryptionAlgorithm getAlgorithm() { result.matchesName("RC4") } + + override DataFlow::Node getAnInput() { result = super.getArgument(1) } + + override BlockMode getBlockMode() { none() } + } + } + + /** + * Cryptographic operations from the `crypto/sha1` package. + */ + private module Sha1 { + private class Sum extends DirectHashOperation instanceof DataFlow::CallNode { + Sum() { this.getTarget().hasQualifiedName("crypto/sha1", "Sum") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA1") } + } + + private class New extends HashAlgorithmInit instanceof DataFlow::CallNode { + New() { this.getTarget().hasQualifiedName("crypto/sha1", "New") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA1") } + } + } + + /** + * Cryptographic operations from the `crypto/sha256` package. + */ + private module Sha256 { + private class Sum256 extends DirectHashOperation instanceof DataFlow::CallNode { + Sum256() { this.getTarget().hasQualifiedName("crypto/sha256", "Sum256") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA256") } + } + + private class Sum224 extends DirectHashOperation instanceof DataFlow::CallNode { + Sum224() { this.getTarget().hasQualifiedName("crypto/sha256", "Sum224") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA224") } + } + + private class New extends HashAlgorithmInit instanceof DataFlow::CallNode { + New() { this.getTarget().hasQualifiedName("crypto/sha256", "New") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA256") } + } + + private class New224 extends HashAlgorithmInit instanceof DataFlow::CallNode { + New224() { this.getTarget().hasQualifiedName("crypto/sha256", "New224") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA224") } + } + } + + private module Sha3 { + private class Sum224 extends DirectHashOperation instanceof DataFlow::CallNode { + Sum224() { this.getTarget().hasQualifiedName("crypto/sha3", "Sum224") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA3224") } + } + + private class Sum256 extends DirectHashOperation instanceof DataFlow::CallNode { + Sum256() { this.getTarget().hasQualifiedName("crypto/sha3", "Sum256") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA3256") } + } + + private class Sum384 extends DirectHashOperation instanceof DataFlow::CallNode { + Sum384() { this.getTarget().hasQualifiedName("crypto/sha3", "Sum384") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA3384") } + } + + private class Sum512 extends DirectHashOperation instanceof DataFlow::CallNode { + Sum512() { this.getTarget().hasQualifiedName("crypto/sha3", "Sum512") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA3512") } + } + + private class SumShake128 extends DirectHashOperation instanceof DataFlow::CallNode { + SumShake128() { this.getTarget().hasQualifiedName("crypto/sha3", "SumSHAKE128") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHAKE128") } + } + + private class SumShake256 extends DirectHashOperation instanceof DataFlow::CallNode { + SumShake256() { this.getTarget().hasQualifiedName("crypto/sha3", "SumSHAKE256") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHAKE256") } + } + + private class New224 extends HashAlgorithmInit instanceof DataFlow::CallNode { + New224() { this.getTarget().hasQualifiedName("crypto/sha3", "New224") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA3224") } + } + + private class New256 extends HashAlgorithmInit instanceof DataFlow::CallNode { + New256() { this.getTarget().hasQualifiedName("crypto/sha3", "New256") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA3256") } + } + + private class New384 extends HashAlgorithmInit instanceof DataFlow::CallNode { + New384() { this.getTarget().hasQualifiedName("crypto/sha3", "New384") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA3384") } + } + + private class New512 extends HashAlgorithmInit instanceof DataFlow::CallNode { + New512() { this.getTarget().hasQualifiedName("crypto/sha3", "New512") } - override Expr getInput() { result = this.getArgument(0).asExpr() } + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA3512") } + } - override CryptographicAlgorithm getAlgorithm() { - result.matchesName(this.getTarget().getPackage().getName()) + private class NewShake128 extends HashAlgorithmInit instanceof DataFlow::CallNode { + NewShake128() { + this.getTarget().hasQualifiedName("crypto/sha3", ["NewCSHAKE128", "NewSHAKE128"]) + } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHAKE128") } + } + + private class NewShake256 extends HashAlgorithmInit instanceof DataFlow::CallNode { + NewShake256() { + this.getTarget().hasQualifiedName("crypto/sha3", ["NewCSHAKE256", "NewSHAKE256"]) + } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHAKE256") } + } + + private class ShakeWrite extends HashOperation instanceof DataFlow::MethodCallNode { + ShakeWrite() { this.getTarget().hasQualifiedName("crypto/sha3", "SHAKE", "Write") } + + override HashAlgorithmInit getInitialization() { HashFlow::flow(result, super.getReceiver()) } + + override HashingAlgorithm getAlgorithm() { result = this.getInitialization().getAlgorithm() } + + override DataFlow::Node getAnInput() { result = super.getArgument(0) } + } } + + private module Sha512 { + private class Sum384 extends DirectHashOperation instanceof DataFlow::CallNode { + Sum384() { this.getTarget().hasQualifiedName("crypto/sha512", "Sum384") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA384") } + } + + private class Sum512 extends DirectHashOperation instanceof DataFlow::CallNode { + Sum512() { this.getTarget().hasQualifiedName("crypto/sha512", "Sum512") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA512") } + } + + private class Sum512_224 extends DirectHashOperation instanceof DataFlow::CallNode { + Sum512_224() { this.getTarget().hasQualifiedName("crypto/sha512", "Sum512_224") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA512224") } + } + + private class Sum512_256 extends DirectHashOperation instanceof DataFlow::CallNode { + Sum512_256() { this.getTarget().hasQualifiedName("crypto/sha512", "Sum512_256") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA512256") } + } + + private class New extends HashAlgorithmInit instanceof DataFlow::CallNode { + New() { this.getTarget().hasQualifiedName("crypto/sha512", "New") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA512") } + } + + private class New384 extends HashAlgorithmInit instanceof DataFlow::CallNode { + New384() { this.getTarget().hasQualifiedName("crypto/sha512", "New384") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA384") } + } + + private class New512_224 extends HashAlgorithmInit instanceof DataFlow::CallNode { + New512_224() { this.getTarget().hasQualifiedName("crypto/sha512", "New512_224") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA512224") } + } + + private class New512_256 extends HashAlgorithmInit instanceof DataFlow::CallNode { + New512_256() { this.getTarget().hasQualifiedName("crypto/sha512", "New512_256") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA512256") } + } + } + + private module Cipher { + private class NewCBCEncrypter extends StdLibNewEncrypter { + NewCBCEncrypter() { this.getTarget().hasQualifiedName("crypto/cipher", "NewCBCEncrypter") } + + override BlockMode getMode() { result = "CBC" } + } + + private class NewCFBEncrypter extends StdLibNewEncrypter { + NewCFBEncrypter() { this.getTarget().hasQualifiedName("crypto/cipher", "NewCFBEncrypter") } + + override BlockMode getMode() { result = "CFB" } + } + + private class NewCTR extends StdLibNewEncrypter { + NewCTR() { this.getTarget().hasQualifiedName("crypto/cipher", "NewCTR") } + + override BlockMode getMode() { result = "CTR" } + } + + private class NewGCM extends StdLibNewEncrypter { + NewGCM() { + exists(string name | this.getTarget().hasQualifiedName("crypto/cipher", name) | + name = ["NewGCM", "NewGCMWithNonceSize", "NewGCMWithRandomNonce", "NewGCMWithTagSize"] + ) + } + + override BlockMode getMode() { result = "GCM" } + } + + private class NewOFB extends StdLibNewEncrypter { + NewOFB() { this.getTarget().hasQualifiedName("crypto/cipher", "NewOFB") } + + override BlockMode getMode() { result = "OFB" } + } + + private class AeadSeal extends EncryptionMethodCall { + AeadSeal() { + this.(DataFlow::MethodCallNode) + .getTarget() + .hasQualifiedName("crypto/cipher", "AEAD", "Seal") and + inputArg = 2 + } + } + + private class BlockEncrypt extends EncryptionMethodCall { + BlockEncrypt() { + this.(DataFlow::MethodCallNode) + .getTarget() + .hasQualifiedName("crypto/cipher", "Block", "Encrypt") and + inputArg = 1 + } + } + + private class BlockModeCryptBlocks extends EncryptionMethodCall { + BlockModeCryptBlocks() { + this.(DataFlow::MethodCallNode) + .getTarget() + .hasQualifiedName("crypto/cipher", "BlockMode", "CryptBlocks") and + inputArg = 1 + } + } + + private class StreamXORKeyStream extends EncryptionMethodCall { + StreamXORKeyStream() { + this.(DataFlow::MethodCallNode) + .getTarget() + .hasQualifiedName("crypto/cipher", "Stream", "XORKeyStream") and + inputArg = 1 + } + } + + private class StreamReader extends EncryptionOperation { + StreamReader() { + lookThroughPointerType(this.getType()).hasQualifiedName("crypto/cipher", "StreamReader") and + exists(DataFlow::Write w, DataFlow::Node base, Field f | + f.hasQualifiedName("crypto/cipher", "StreamReader", "S") and + w.writesField(base, f, encryptionFlowTarget) and + DataFlow::localFlow(base, this) + ) and + exists(DataFlow::Write w, DataFlow::Node base, Field f | + f.hasQualifiedName("crypto/cipher", "StreamReader", "R") and + w.writesField(base, f, inputNode) and + DataFlow::localFlow(base, this) + ) + } + } + + /** + * Limitation: StreamWriter wraps a Writer, so we need to look forward to + * where the Writer is written to. Currently this is done using local flow, + * so it only works within one function. + */ + private class StreamWriter extends EncryptionOperation { + StreamWriter() { + lookThroughPointerType(this.getType()).hasQualifiedName("crypto/cipher", "StreamWriter") and + inputNode = this and + exists(DataFlow::Write w, DataFlow::Node base, Field f | + w.writesField(base, f, encryptionFlowTarget) and + f.hasQualifiedName("crypto/cipher", "StreamWriter", "S") + | + base = this or + TaintTracking::localTaint(base, this.(DataFlow::PostUpdateNode).getPreUpdateNode()) + ) + } + } + } +} + +private module Hash { + private class HashSum extends HashOperation instanceof DataFlow::MethodCallNode { + HashSum() { this.getTarget().implements("hash", "Hash", "Sum") } + + override HashAlgorithmInit getInitialization() { HashFlow::flow(result, super.getReceiver()) } + + override HashingAlgorithm getAlgorithm() { result = this.getInitialization().getAlgorithm() } + + override DataFlow::Node getAnInput() { result = super.getArgument(0) } + } +} + +private DataFlow::Node getANonIoWriterPredecessor(DataFlow::Node node) { + node.getType().implements("io", "Writer") and + exists(DataFlow::Node pre | TaintTracking::localTaintStep(pre, node) | + if pre.getType().implements("io", "Writer") + then result = getANonIoWriterPredecessor(pre) + else result = pre + ) } /** - * A cryptographic operation from the `crypto/rc4` package. + * Taint flowing to an `io.Writer` (such as `hash.Hash` or `*sha3.SHAKE`) via + * its implementation of the `io.Writer` interface. */ -class Rc4 extends CryptographicOperation::Range instanceof DataFlow::CallNode { - Rc4() { this.getTarget().hasQualifiedName("crypto/rc4", "NewCipher") } +private class FlowToIoWriter extends HashOperation instanceof DataFlow::Node { + HashAlgorithmInit init; + DataFlow::Node input; + + FlowToIoWriter() { + this.getType().implements("io", "Writer") and + HashFlow::flow(init, this) and + // If we have `h.Write(taint)` or `io.WriteString(h, taint)` then it's + // the post-update node of `h` that gets tainted. + exists(DataFlow::PostUpdateNode pun | pun.getPreUpdateNode() = this | + input = getANonIoWriterPredecessor(pun) + ) + } + + override HashAlgorithmInit getInitialization() { result = init } + + override HashingAlgorithm getAlgorithm() { result = this.getInitialization().getAlgorithm() } + + override DataFlow::Node getAnInput() { result = input } +} + +/** + * Currently only weak algorithms from the `golang.org/x/crypto` module are + * modeled here. + */ +private module GolangOrgXCrypto { + private module Md4 { + private class New extends HashAlgorithmInit instanceof DataFlow::CallNode { + New() { this.getTarget().hasQualifiedName("golang.org/x/crypto/md4", "New") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("MD4") } + } + } - override Expr getInput() { result = this.getArgument(0).asExpr() } + private module Ripemd160 { + private class New extends HashAlgorithmInit instanceof DataFlow::CallNode { + New() { this.getTarget().hasQualifiedName("golang.org/x/crypto/ripemd160", "New") } - override CryptographicAlgorithm getAlgorithm() { - result.matchesName(this.getTarget().getPackage().getName()) + override HashingAlgorithm getAlgorithm() { result.matchesName("RIPEMD160") } + } } } diff --git a/go/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.expected b/go/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.expected index 6f40dfcc7ad0..4fbf77dda629 100644 --- a/go/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.expected +++ b/go/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.expected @@ -1,17 +1,83 @@ #select -| Crypto.go:21:25:21:27 | buf | Crypto.go:18:9:18:16 | password | Crypto.go:21:25:21:27 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:18:9:18:16 | password | Sensitive data | -| Crypto.go:24:10:24:12 | buf | Crypto.go:18:9:18:16 | password | Crypto.go:24:10:24:12 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:18:9:18:16 | password | Sensitive data | -| Crypto.go:27:16:27:18 | buf | Crypto.go:18:9:18:16 | password | Crypto.go:27:16:27:18 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:18:9:18:16 | password | Sensitive data | -| Crypto.go:30:11:30:13 | buf | Crypto.go:18:9:18:16 | password | Crypto.go:30:11:30:13 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:18:9:18:16 | password | Sensitive data | +| Crypto.go:36:21:36:28 | password | Crypto.go:36:21:36:28 | password | Crypto.go:36:21:36:28 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:36:21:36:28 | password | Sensitive data | +| Crypto.go:41:22:41:29 | password | Crypto.go:41:22:41:29 | password | Crypto.go:41:22:41:29 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:41:22:41:29 | password | Sensitive data | +| Crypto.go:46:22:46:29 | password | Crypto.go:46:22:46:29 | password | Crypto.go:46:22:46:29 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:46:22:46:29 | password | Sensitive data | +| Crypto.go:51:22:51:29 | password | Crypto.go:51:22:51:29 | password | Crypto.go:51:22:51:29 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:51:22:51:29 | password | Sensitive data | +| Crypto.go:56:22:56:29 | password | Crypto.go:56:22:56:29 | password | Crypto.go:56:22:56:29 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:56:22:56:29 | password | Sensitive data | +| Crypto.go:61:32:61:39 | password | Crypto.go:61:32:61:39 | password | Crypto.go:61:32:61:39 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:61:32:61:39 | password | Sensitive data | +| Crypto.go:66:30:66:37 | password | Crypto.go:66:30:66:37 | password | Crypto.go:66:30:66:37 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:66:30:66:37 | password | Sensitive data | +| Crypto.go:68:59:68:83 | call to NewReader | Crypto.go:68:75:68:82 | password | Crypto.go:68:59:68:83 | call to NewReader | $@ is used in a weak cryptographic algorithm. | Crypto.go:68:75:68:82 | password | Sensitive data | +| Crypto.go:72:10:72:24 | ctrStreamWriter [postupdate] | Crypto.go:72:43:72:50 | password | Crypto.go:72:10:72:24 | ctrStreamWriter [postupdate] | $@ is used in a weak cryptographic algorithm. | Crypto.go:72:43:72:50 | password | Sensitive data | +| Crypto.go:78:30:78:37 | password | Crypto.go:78:30:78:37 | password | Crypto.go:78:30:78:37 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:78:30:78:37 | password | Sensitive data | +| Crypto.go:83:30:83:37 | password | Crypto.go:83:30:83:37 | password | Crypto.go:83:30:83:37 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:83:30:83:37 | password | Sensitive data | +| Crypto.go:91:21:91:33 | call to getPassword | Crypto.go:91:21:91:33 | call to getPassword | Crypto.go:91:21:91:33 | call to getPassword | $@ is used in a weak cryptographic algorithm. | Crypto.go:91:21:91:33 | call to getPassword | Sensitive data | +| Crypto.go:96:22:96:34 | call to getPassword | Crypto.go:96:22:96:34 | call to getPassword | Crypto.go:96:22:96:34 | call to getPassword | $@ is used in a weak cryptographic algorithm. | Crypto.go:96:22:96:34 | call to getPassword | Sensitive data | +| Crypto.go:101:22:101:34 | call to getPassword | Crypto.go:101:22:101:34 | call to getPassword | Crypto.go:101:22:101:34 | call to getPassword | $@ is used in a weak cryptographic algorithm. | Crypto.go:101:22:101:34 | call to getPassword | Sensitive data | +| Crypto.go:106:22:106:29 | password | Crypto.go:106:22:106:29 | password | Crypto.go:106:22:106:29 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:106:22:106:29 | password | Sensitive data | +| Crypto.go:111:22:111:29 | password | Crypto.go:111:22:111:29 | password | Crypto.go:111:22:111:29 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:111:22:111:29 | password | Sensitive data | +| Crypto.go:116:32:116:44 | call to getPassword | Crypto.go:116:32:116:44 | call to getPassword | Crypto.go:116:32:116:44 | call to getPassword | $@ is used in a weak cryptographic algorithm. | Crypto.go:116:32:116:44 | call to getPassword | Sensitive data | +| Crypto.go:121:30:121:42 | call to getPassword | Crypto.go:121:30:121:42 | call to getPassword | Crypto.go:121:30:121:42 | call to getPassword | $@ is used in a weak cryptographic algorithm. | Crypto.go:121:30:121:42 | call to getPassword | Sensitive data | +| Crypto.go:123:59:123:88 | call to NewReader | Crypto.go:123:75:123:87 | call to getPassword | Crypto.go:123:59:123:88 | call to NewReader | $@ is used in a weak cryptographic algorithm. | Crypto.go:123:75:123:87 | call to getPassword | Sensitive data | +| Crypto.go:127:10:127:24 | ctrStreamWriter [postupdate] | Crypto.go:127:43:127:55 | call to getPassword | Crypto.go:127:10:127:24 | ctrStreamWriter [postupdate] | $@ is used in a weak cryptographic algorithm. | Crypto.go:127:43:127:55 | call to getPassword | Sensitive data | +| Crypto.go:133:30:133:37 | password | Crypto.go:133:30:133:37 | password | Crypto.go:133:30:133:37 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:133:30:133:37 | password | Sensitive data | +| Crypto.go:138:30:138:37 | password | Crypto.go:138:30:138:37 | password | Crypto.go:138:30:138:37 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:138:30:138:37 | password | Sensitive data | +| Crypto.go:198:22:198:34 | call to getPassword | Crypto.go:198:22:198:34 | call to getPassword | Crypto.go:198:22:198:34 | call to getPassword | $@ is used in a weak cryptographic algorithm. | Crypto.go:198:22:198:34 | call to getPassword | Sensitive data | +| Crypto.go:205:8:205:10 | buf | Crypto.go:202:9:202:16 | password | Crypto.go:205:8:205:10 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:202:9:202:16 | password | Sensitive data | +| Crypto.go:206:10:206:12 | buf | Crypto.go:202:9:202:16 | password | Crypto.go:206:10:206:12 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:202:9:202:16 | password | Sensitive data | +| Crypto.go:207:20:207:33 | passwordString | Crypto.go:207:20:207:33 | passwordString | Crypto.go:207:20:207:33 | passwordString | $@ is used in a weak cryptographic algorithm. | Crypto.go:207:20:207:33 | passwordString | Sensitive data | +| Crypto.go:208:10:208:12 | buf | Crypto.go:202:9:202:16 | password | Crypto.go:208:10:208:12 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:202:9:202:16 | password | Sensitive data | +| Crypto.go:210:17:210:19 | buf | Crypto.go:202:9:202:16 | password | Crypto.go:210:17:210:19 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:202:9:202:16 | password | Sensitive data | +| Crypto.go:211:11:211:13 | buf | Crypto.go:202:9:202:16 | password | Crypto.go:211:11:211:13 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:202:9:202:16 | password | Sensitive data | edges -| Crypto.go:18:9:18:16 | password | Crypto.go:21:25:21:27 | buf | provenance | | -| Crypto.go:18:9:18:16 | password | Crypto.go:24:10:24:12 | buf | provenance | | -| Crypto.go:18:9:18:16 | password | Crypto.go:27:16:27:18 | buf | provenance | | -| Crypto.go:18:9:18:16 | password | Crypto.go:30:11:30:13 | buf | provenance | | +| Crypto.go:68:75:68:82 | password | Crypto.go:68:59:68:83 | call to NewReader | provenance | MaD:1 | +| Crypto.go:72:27:72:51 | call to NewReader | Crypto.go:72:10:72:24 | ctrStreamWriter [postupdate] | provenance | MaD:2 | +| Crypto.go:72:43:72:50 | password | Crypto.go:72:27:72:51 | call to NewReader | provenance | MaD:1 | +| Crypto.go:123:75:123:87 | call to getPassword | Crypto.go:123:59:123:88 | call to NewReader | provenance | MaD:1 | +| Crypto.go:127:27:127:56 | call to NewReader | Crypto.go:127:10:127:24 | ctrStreamWriter [postupdate] | provenance | MaD:2 | +| Crypto.go:127:43:127:55 | call to getPassword | Crypto.go:127:27:127:56 | call to NewReader | provenance | MaD:1 | +| Crypto.go:202:9:202:16 | password | Crypto.go:205:8:205:10 | buf | provenance | | +| Crypto.go:202:9:202:16 | password | Crypto.go:206:10:206:12 | buf | provenance | | +| Crypto.go:202:9:202:16 | password | Crypto.go:208:10:208:12 | buf | provenance | | +| Crypto.go:202:9:202:16 | password | Crypto.go:210:17:210:19 | buf | provenance | | +| Crypto.go:202:9:202:16 | password | Crypto.go:211:11:211:13 | buf | provenance | | +models +| 1 | Summary: bytes; ; false; NewReader; ; ; Argument[0]; ReturnValue; taint; manual | +| 2 | Summary: io; ; false; Copy; ; ; Argument[1]; Argument[0]; taint; manual | nodes -| Crypto.go:18:9:18:16 | password | semmle.label | password | -| Crypto.go:21:25:21:27 | buf | semmle.label | buf | -| Crypto.go:24:10:24:12 | buf | semmle.label | buf | -| Crypto.go:27:16:27:18 | buf | semmle.label | buf | -| Crypto.go:30:11:30:13 | buf | semmle.label | buf | +| Crypto.go:36:21:36:28 | password | semmle.label | password | +| Crypto.go:41:22:41:29 | password | semmle.label | password | +| Crypto.go:46:22:46:29 | password | semmle.label | password | +| Crypto.go:51:22:51:29 | password | semmle.label | password | +| Crypto.go:56:22:56:29 | password | semmle.label | password | +| Crypto.go:61:32:61:39 | password | semmle.label | password | +| Crypto.go:66:30:66:37 | password | semmle.label | password | +| Crypto.go:68:59:68:83 | call to NewReader | semmle.label | call to NewReader | +| Crypto.go:68:75:68:82 | password | semmle.label | password | +| Crypto.go:72:10:72:24 | ctrStreamWriter [postupdate] | semmle.label | ctrStreamWriter [postupdate] | +| Crypto.go:72:27:72:51 | call to NewReader | semmle.label | call to NewReader | +| Crypto.go:72:43:72:50 | password | semmle.label | password | +| Crypto.go:78:30:78:37 | password | semmle.label | password | +| Crypto.go:83:30:83:37 | password | semmle.label | password | +| Crypto.go:91:21:91:33 | call to getPassword | semmle.label | call to getPassword | +| Crypto.go:96:22:96:34 | call to getPassword | semmle.label | call to getPassword | +| Crypto.go:101:22:101:34 | call to getPassword | semmle.label | call to getPassword | +| Crypto.go:106:22:106:29 | password | semmle.label | password | +| Crypto.go:111:22:111:29 | password | semmle.label | password | +| Crypto.go:116:32:116:44 | call to getPassword | semmle.label | call to getPassword | +| Crypto.go:121:30:121:42 | call to getPassword | semmle.label | call to getPassword | +| Crypto.go:123:59:123:88 | call to NewReader | semmle.label | call to NewReader | +| Crypto.go:123:75:123:87 | call to getPassword | semmle.label | call to getPassword | +| Crypto.go:127:10:127:24 | ctrStreamWriter [postupdate] | semmle.label | ctrStreamWriter [postupdate] | +| Crypto.go:127:27:127:56 | call to NewReader | semmle.label | call to NewReader | +| Crypto.go:127:43:127:55 | call to getPassword | semmle.label | call to getPassword | +| Crypto.go:133:30:133:37 | password | semmle.label | password | +| Crypto.go:138:30:138:37 | password | semmle.label | password | +| Crypto.go:198:22:198:34 | call to getPassword | semmle.label | call to getPassword | +| Crypto.go:202:9:202:16 | password | semmle.label | password | +| Crypto.go:205:8:205:10 | buf | semmle.label | buf | +| Crypto.go:206:10:206:12 | buf | semmle.label | buf | +| Crypto.go:207:20:207:33 | passwordString | semmle.label | passwordString | +| Crypto.go:208:10:208:12 | buf | semmle.label | buf | +| Crypto.go:210:17:210:19 | buf | semmle.label | buf | +| Crypto.go:211:11:211:13 | buf | semmle.label | buf | subpaths diff --git a/go/ql/test/query-tests/Security/CWE-327/Crypto.go b/go/ql/test/query-tests/Security/CWE-327/Crypto.go index a58052df38d7..1a7afc35cb99 100644 --- a/go/ql/test/query-tests/Security/CWE-327/Crypto.go +++ b/go/ql/test/query-tests/Security/CWE-327/Crypto.go @@ -1,55 +1,254 @@ package main import ( + "bytes" "crypto/aes" + "crypto/cipher" "crypto/des" "crypto/md5" "crypto/rc4" "crypto/sha1" "crypto/sha256" + "crypto/sha3" + "crypto/sha512" + "io" + "os" ) -func crypto() { - public := []byte("hello") +var dst []byte = make([]byte, 16) +var password []byte = []byte("123456") - password := []byte("123456") +const passwordString string = "correct horse battery staple" - // testing dataflow by passing into different variable +var public []byte = []byte("hello") + +func getPassword() []byte { + return []byte("123456") +} + +// Note that we do not alert on decryption as we may need to decrypt legacy formats + +func BlockCipherDes() { + // BAD, des is a weak crypto algorithm + block, _ := des.NewCipher(nil) + + block.Encrypt(dst, public) // $ CryptographicOperation="DES. init from line 33." + block.Encrypt(dst, password) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. init from line 33." + block.Decrypt(dst, password) + + gcm1, _ := cipher.NewGCM(block) + gcm1.Seal(nil, nil, public, nil) // $ CryptographicOperation="DES. init from line 33." + gcm1.Seal(nil, nil, password, nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. init from line 33." + gcm1.Open(nil, nil, password, nil) + + gcm2, _ := cipher.NewGCMWithNonceSize(block, 12) + gcm2.Seal(nil, nil, public, nil) // $ CryptographicOperation="DES. init from line 33." + gcm2.Seal(nil, nil, password, nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. init from line 33." + gcm2.Open(nil, nil, password, nil) + + gcm3, _ := cipher.NewGCMWithRandomNonce(block) + gcm3.Seal(nil, nil, public, nil) // $ CryptographicOperation="DES. init from line 33." + gcm3.Seal(nil, nil, password, nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. init from line 33." + gcm3.Open(nil, nil, password, nil) + + gcm4, _ := cipher.NewGCMWithTagSize(block, 12) + gcm4.Seal(nil, nil, public, nil) // $ CryptographicOperation="DES. init from line 33." + gcm4.Seal(nil, nil, password, nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. init from line 33." + gcm4.Open(nil, nil, password, nil) + + cbcEncrypter := cipher.NewCBCEncrypter(block, nil) + cbcEncrypter.CryptBlocks(dst, public) // $ CryptographicOperation="DES. blockMode: CBC. init from lines 33,59." + cbcEncrypter.CryptBlocks(dst, password) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. blockMode: CBC. init from lines 33,59." + cipher.NewCBCDecrypter(block, nil).CryptBlocks(dst, password) + + ctrStream := cipher.NewCTR(block, nil) + ctrStream.XORKeyStream(dst, public) // $ CryptographicOperation="DES. blockMode: CTR. init from lines 33,64." + ctrStream.XORKeyStream(dst, password) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. blockMode: CTR. init from lines 33,64." + + ctrStreamReader := &cipher.StreamReader{S: ctrStream, R: bytes.NewReader(password)} // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. blockMode: CTR. init from lines 33,64." + io.Copy(os.Stdout, ctrStreamReader) + + ctrStreamWriter := &cipher.StreamWriter{S: ctrStream, W: os.Stdout} // $ CryptographicOperation="DES. blockMode: CTR. init from lines 33,64." + io.Copy(ctrStreamWriter, bytes.NewReader(password)) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. blockMode: CTR. init from lines 33,64." + + // deprecated + + cfbStream := cipher.NewCFBEncrypter(block, nil) + cfbStream.XORKeyStream(dst, public) // $ CryptographicOperation="DES. blockMode: CFB. init from lines 33,76." + cfbStream.XORKeyStream(dst, password) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. blockMode: CFB. init from lines 33,76." + cipher.NewCFBDecrypter(block, nil).XORKeyStream(dst, password) + + ofbStream := cipher.NewOFB(block, nil) + ofbStream.XORKeyStream(dst, public) // $ CryptographicOperation="DES. blockMode: OFB. init from lines 33,81." + ofbStream.XORKeyStream(dst, password) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. blockMode: OFB. init from lines 33,81." +} + +func BlockCipherTripleDes() { + // BAD, triple des is a weak crypto algorithm and password is sensitive data + block, _ := des.NewTripleDESCipher(nil) + + block.Encrypt(dst, public) // $ CryptographicOperation="TRIPLEDES. init from line 88." + block.Encrypt(dst, getPassword()) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. init from line 88." + block.Decrypt(dst, getPassword()) + + gcm1, _ := cipher.NewGCM(block) + gcm1.Seal(nil, nil, public, nil) // $ CryptographicOperation="TRIPLEDES. init from line 88." + gcm1.Seal(nil, nil, getPassword(), nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. init from line 88." + gcm1.Open(nil, nil, getPassword(), nil) + + gcm2, _ := cipher.NewGCMWithNonceSize(block, 12) + gcm2.Seal(nil, nil, public, nil) // $ CryptographicOperation="TRIPLEDES. init from line 88." + gcm2.Seal(nil, nil, getPassword(), nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. init from line 88." + gcm2.Open(nil, nil, getPassword(), nil) + + gcm3, _ := cipher.NewGCMWithRandomNonce(block) + gcm3.Seal(nil, nil, public, nil) // $ CryptographicOperation="TRIPLEDES. init from line 88." + gcm3.Seal(nil, nil, password, nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. init from line 88." + gcm3.Open(nil, nil, password, nil) + + gcm4, _ := cipher.NewGCMWithTagSize(block, 12) + gcm4.Seal(nil, nil, public, nil) // $ CryptographicOperation="TRIPLEDES. init from line 88." + gcm4.Seal(nil, nil, password, nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. init from line 88." + gcm4.Open(nil, nil, password, nil) + + cbcEncrypter := cipher.NewCBCEncrypter(block, nil) + cbcEncrypter.CryptBlocks(dst, public) // $ CryptographicOperation="TRIPLEDES. blockMode: CBC. init from lines 114,88." + cbcEncrypter.CryptBlocks(dst, getPassword()) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CBC. init from lines 114,88." + cipher.NewCBCDecrypter(block, nil).CryptBlocks(dst, getPassword()) + + ctrStream := cipher.NewCTR(block, nil) + ctrStream.XORKeyStream(dst, public) // $ CryptographicOperation="TRIPLEDES. blockMode: CTR. init from lines 119,88." + ctrStream.XORKeyStream(dst, getPassword()) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CTR. init from lines 119,88." + + ctrStreamReader := &cipher.StreamReader{S: ctrStream, R: bytes.NewReader(getPassword())} // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CTR. init from lines 119,88." + io.Copy(os.Stdout, ctrStreamReader) + + ctrStreamWriter := &cipher.StreamWriter{S: ctrStream, W: os.Stdout} // $ CryptographicOperation="TRIPLEDES. blockMode: CTR. init from lines 119,88." + io.Copy(ctrStreamWriter, bytes.NewReader(getPassword())) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CTR. init from lines 119,88." + + // deprecated + + cfbStream := cipher.NewCFBEncrypter(block, nil) + cfbStream.XORKeyStream(dst, public) // $ CryptographicOperation="TRIPLEDES. blockMode: CFB. init from lines 131,88." + cfbStream.XORKeyStream(dst, password) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CFB. init from lines 131,88." + cipher.NewCFBDecrypter(block, nil).XORKeyStream(dst, password) + + ofbStream := cipher.NewOFB(block, nil) + ofbStream.XORKeyStream(dst, public) // $ CryptographicOperation="TRIPLEDES. blockMode: OFB. init from lines 136,88." + ofbStream.XORKeyStream(dst, password) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. blockMode: OFB. init from lines 136,88." +} + +func BlockCipherAes() { + // GOOD, aes is a strong crypto algorithm + block, _ := aes.NewCipher(nil) + + block.Encrypt(dst, public) // $ CryptographicOperation="AES. init from line 143." + block.Encrypt(dst, password) // $ CryptographicOperation="AES. init from line 143." + block.Decrypt(dst, password) + + gcm1, _ := cipher.NewGCM(block) + gcm1.Seal(nil, nil, public, nil) // $ CryptographicOperation="AES. init from line 143." + gcm1.Seal(nil, nil, password, nil) // $ CryptographicOperation="AES. init from line 143." + gcm1.Open(nil, nil, password, nil) + + gcm2, _ := cipher.NewGCMWithNonceSize(block, 12) + gcm2.Seal(nil, nil, public, nil) // $ CryptographicOperation="AES. init from line 143." + gcm2.Seal(nil, nil, password, nil) // $ CryptographicOperation="AES. init from line 143." + gcm2.Open(nil, nil, password, nil) + + gcm3, _ := cipher.NewGCMWithRandomNonce(block) + gcm3.Seal(nil, nil, public, nil) // $ CryptographicOperation="AES. init from line 143." + gcm3.Seal(nil, nil, password, nil) // $ CryptographicOperation="AES. init from line 143." + gcm3.Open(nil, nil, password, nil) + + gcm4, _ := cipher.NewGCMWithTagSize(block, 12) + gcm4.Seal(nil, nil, public, nil) // $ CryptographicOperation="AES. init from line 143." + gcm4.Seal(nil, nil, password, nil) // $ CryptographicOperation="AES. init from line 143." + gcm4.Open(nil, nil, password, nil) + + cbcEncrypter := cipher.NewCBCEncrypter(block, nil) + cbcEncrypter.CryptBlocks(dst, public) // $ CryptographicOperation="AES. blockMode: CBC. init from lines 143,169." + cbcEncrypter.CryptBlocks(dst, password) // $ CryptographicOperation="AES. blockMode: CBC. init from lines 143,169." + cipher.NewCBCDecrypter(block, nil).CryptBlocks(dst, password) + + ctrStream := cipher.NewCTR(block, nil) + ctrStream.XORKeyStream(dst, public) // $ CryptographicOperation="AES. blockMode: CTR. init from lines 143,174." + ctrStream.XORKeyStream(dst, password) // $ CryptographicOperation="AES. blockMode: CTR. init from lines 143,174." + + ctrStreamReader := &cipher.StreamReader{S: ctrStream, R: bytes.NewReader(password)} // $ CryptographicOperation="AES. blockMode: CTR. init from lines 143,174." + io.Copy(os.Stdout, ctrStreamReader) + + ctrStreamWriter := &cipher.StreamWriter{S: ctrStream, W: os.Stdout} // $ CryptographicOperation="AES. blockMode: CTR. init from lines 143,174." + io.Copy(ctrStreamWriter, bytes.NewReader(password)) // $ CryptographicOperation="AES. blockMode: CTR. init from lines 143,174." + + // deprecated + + cfbStream := cipher.NewCFBEncrypter(block, nil) + cfbStream.XORKeyStream(dst, public) // $ CryptographicOperation="AES. blockMode: CFB. init from lines 143,186." + cfbStream.XORKeyStream(dst, password) // $ CryptographicOperation="AES. blockMode: CFB. init from lines 143,186." + cipher.NewCFBDecrypter(block, nil).XORKeyStream(dst, password) + + ofbStream := cipher.NewOFB(block, nil) + ofbStream.XORKeyStream(dst, public) // $ CryptographicOperation="AES. blockMode: OFB. init from lines 143,191." + ofbStream.XORKeyStream(dst, password) // $ CryptographicOperation="AES. blockMode: OFB. init from lines 143,191." +} + +func CipherRc4() { + c, _ := rc4.NewCipher(nil) + c.XORKeyStream(dst, getPassword()) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="RC4. init from line 198." +} + +func WeakHashes() { buf := password // $ Source - // BAD, des is a weak crypto algorithm and password is sensitive data - des.NewTripleDESCipher(buf) // $ Alert + h := md5.New() + h.Sum(buf) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="MD5. init from line 204." + h.Write(buf) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="MD5. init from line 204." + io.WriteString(h, passwordString) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="MD5. init from line 204." + md5.Sum(buf) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="MD5. init from line 208." + + sha1.New().Sum(buf) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="SHA1. init from line 210." + sha1.Sum(buf) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="SHA1. init from line 211." +} + +func StrongHashes() { + buf := password + + sha256.New224().Sum(buf) // $ CryptographicOperation="SHA224. init from line 217." + sha256.Sum224(buf) // $ CryptographicOperation="SHA224. init from line 218." - // BAD, md5 is a weak crypto algorithm and password is sensitive data - md5.Sum(buf) // $ Alert + sha256.New().Sum(buf) // $ CryptographicOperation="SHA256. init from line 220." + sha256.Sum256(buf) // $ CryptographicOperation="SHA256. init from line 221." - // BAD, rc4 is a weak crypto algorithm and password is sensitive data - rc4.NewCipher(buf) // $ Alert + sha512.New().Sum(buf) // $ CryptographicOperation="SHA512. init from line 223." + sha512.Sum512(buf) // $ CryptographicOperation="SHA512. init from line 224." - // BAD, sha1 is a weak crypto algorithm and password is sensitive data - sha1.Sum(buf) // $ Alert + sha512.New384().Sum(buf) // $ CryptographicOperation="SHA384. init from line 226." + sha512.Sum384(buf) // $ CryptographicOperation="SHA384. init from line 227." - // GOOD, password is sensitive data but aes is a strong crypto algorithm - aes.NewCipher(buf) + sha512.New512_224().Sum(buf) // $ CryptographicOperation="SHA512224. init from line 229." + sha512.Sum512_224(buf) // $ CryptographicOperation="SHA512224. init from line 230." - // GOOD, password is sensitive data but sha256 is a strong crypto algorithm - sha256.Sum256(buf) + sha512.New512_256().Sum(buf) // $ CryptographicOperation="SHA512256. init from line 232." + sha512.Sum512_256(buf) // $ CryptographicOperation="SHA512256. init from line 233." - // GOOD, des is a weak crypto algorithm but public is not sensitive data - des.NewTripleDESCipher(public) + sha3.New224().Sum(buf) // $ CryptographicOperation="SHA3224. init from line 235." + sha3.Sum224(buf) // $ CryptographicOperation="SHA3224. init from line 236." - // GOOD, md5 is a weak crypto algorithm but public is not sensitive data - md5.Sum(public) + sha3.New256().Sum(buf) // $ CryptographicOperation="SHA3256. init from line 238." + sha3.Sum256(buf) // $ CryptographicOperation="SHA3256. init from line 239." - // GOOD, rc4 is a weak crypto algorithm but public is not sensitive data - rc4.NewCipher(public) + sha3.New384().Sum(buf) // $ CryptographicOperation="SHA3384. init from line 241." + sha3.Sum384(buf) // $ CryptographicOperation="SHA3384. init from line 242." - // GOOD, sha1 is a weak crypto algorithm but public is not sensitive data - sha1.Sum(public) + sha3.New512().Sum(buf) // $ CryptographicOperation="SHA3512. init from line 244." + sha3.Sum512(buf) // $ CryptographicOperation="SHA3512. init from line 245." - // GOOD, aes is a strong crypto algorithm and public is not sensitive data - aes.NewCipher(public) + sha3.NewSHAKE128().Write(buf) // $ CryptographicOperation="SHAKE128. init from line 247." + sha3.NewCSHAKE128(nil, nil).Write(buf) // $ CryptographicOperation="SHAKE128. init from line 248." + sha3.SumSHAKE128(buf, 100) // $ CryptographicOperation="SHAKE128. init from line 249." - // GOOD, sha256 is a strong crypto algorithm and public is not sensitive data - sha256.Sum256(public) + sha3.NewSHAKE256().Write(buf) // $ CryptographicOperation="SHAKE256. init from line 251." + sha3.NewCSHAKE256(nil, nil).Write(buf) // $ CryptographicOperation="SHAKE256. init from line 252." + sha3.SumSHAKE256(buf, 100) // $ CryptographicOperation="SHAKE256. init from line 253." } diff --git a/go/ql/test/query-tests/Security/CWE-327/CryptoAlgorithm.expected b/go/ql/test/query-tests/Security/CWE-327/CryptoAlgorithm.expected new file mode 100644 index 000000000000..55e9aed2e93c --- /dev/null +++ b/go/ql/test/query-tests/Security/CWE-327/CryptoAlgorithm.expected @@ -0,0 +1,2 @@ +testFailures +invalidModelRow diff --git a/go/ql/test/query-tests/Security/CWE-327/CryptoAlgorithm.ql b/go/ql/test/query-tests/Security/CWE-327/CryptoAlgorithm.ql new file mode 100644 index 000000000000..ab096f9df2ac --- /dev/null +++ b/go/ql/test/query-tests/Security/CWE-327/CryptoAlgorithm.ql @@ -0,0 +1,47 @@ +import go +import ModelValidation +import utils.test.InlineExpectationsTest + +module Test implements TestSig { + string getARelevantTag() { result = "CryptographicOperation" } + + predicate hasActualResult(Location location, string element, string tag, string value) { + tag = "CryptographicOperation" and + exists( + CryptographicOperation::Range ho, string algorithm, string initialization, string blockMode + | + algorithm = ho.getAlgorithm().toString() + "." and + ( + blockMode = " blockMode: " + ho.getBlockMode().toString() + "." + or + not exists(ho.getBlockMode()) and blockMode = "" + ) and + exists(int c | c = count(ho.getInitialization()) | + c = 0 and initialization = "" + or + c = 1 and + initialization = + " init from line " + + strictconcat(DataFlow::Node init | + init = ho.getInitialization() + | + init.getStartLine().toString(), "," + ) + "." + or + c > 1 and + initialization = + " init from lines " + + strictconcat(DataFlow::Node init | + init = ho.getInitialization() + | + init.getStartLine().toString(), "," + ) + "." + ) and + ho.getLocation() = location and + element = ho.toString() and + value = "\"" + algorithm + blockMode + initialization + "\"" + ) + } +} + +import MakeTest diff --git a/go/ql/test/query-tests/Security/CWE-327/go.mod b/go/ql/test/query-tests/Security/CWE-327/go.mod new file mode 100644 index 000000000000..bf42b84feefd --- /dev/null +++ b/go/ql/test/query-tests/Security/CWE-327/go.mod @@ -0,0 +1,3 @@ +module test + +go 1.24 diff --git a/shared/concepts/codeql/concepts/internal/CryptoAlgorithmNames.qll b/shared/concepts/codeql/concepts/internal/CryptoAlgorithmNames.qll index efcd870c724a..86392890cafb 100644 --- a/shared/concepts/codeql/concepts/internal/CryptoAlgorithmNames.qll +++ b/shared/concepts/codeql/concepts/internal/CryptoAlgorithmNames.qll @@ -23,7 +23,8 @@ predicate isStrongHashingAlgorithm(string name) { "BLAKE3", // "DSA", "ED25519", "ES256", "ECDSA256", "ES384", "ECDSA384", "ES512", "ECDSA512", "SHA2", - "SHA224", "SHA256", "SHA384", "SHA512", "SHA3", "SHA3224", "SHA3256", "SHA3384", "SHA3512", + "SHA224", "SHA256", "SHA384", "SHA512", "SHA512224", "SHA512256", "SHA3", "SHA3224", + "SHA3256", "SHA3384", "SHA3512", // see https://cryptography.io/en/latest/hazmat/primitives/cryptographic-hashes/#cryptography.hazmat.primitives.hashes.SHAKE128 "SHAKE128", "SHAKE256", // see https://cryptography.io/en/latest/hazmat/primitives/cryptographic-hashes/#sm3 From b5271a6afe450b11ecfcf3e2866e344f880ef72a Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Fri, 31 Oct 2025 15:55:54 +0000 Subject: [PATCH 09/18] Make non-path query for encryption only --- .../BrokenCryptoAlgorithmCustomizations.qll | 58 ---- .../security/BrokenCryptoAlgorithmQuery.qll | 43 --- .../Security/CWE-327/BrokenCryptoAlgorithm.ql | 28 +- .../CWE-327/BrokenCryptoAlgorithm.expected | 112 ++------ .../query-tests/Security/CWE-327/Crypto.go | 254 ------------------ .../Security/CWE-327/encryption.go | 167 ++++++++++++ 6 files changed, 216 insertions(+), 446 deletions(-) delete mode 100644 go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmCustomizations.qll delete mode 100644 go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmQuery.qll delete mode 100644 go/ql/test/query-tests/Security/CWE-327/Crypto.go create mode 100644 go/ql/test/query-tests/Security/CWE-327/encryption.go diff --git a/go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmCustomizations.qll b/go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmCustomizations.qll deleted file mode 100644 index e5ac77fbb75f..000000000000 --- a/go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmCustomizations.qll +++ /dev/null @@ -1,58 +0,0 @@ -/** - * Provides default sources, sinks and sanitizers for reasoning about - * sensitive information in weak cryptographic algorithms, - * as well as extension points for adding your own. - */ - -import go -private import semmle.go.security.SensitiveActions - -/** - * Provides default sources, sinks and sanitizers for reasoning about - * sensitive information in weak cryptographic algorithms, - * as well as extension points for adding your own. - */ -module BrokenCryptoAlgorithm { - /** - * A data flow source for sensitive information in broken or weak cryptographic algorithms. - */ - abstract class Source extends DataFlow::Node { } - - /** - * A data flow sink for sensitive information in broken or weak cryptographic algorithms. - */ - abstract class Sink extends DataFlow::Node { - /** Gets the data-flow node where the cryptographic algorithm used in this operation is configured. */ - abstract DataFlow::Node getInitialization(); - } - - /** - * A sanitizer for sensitive information in broken or weak cryptographic algorithms. - */ - abstract class Sanitizer extends DataFlow::Node { } - - /** - * A sensitive source. - */ - class SensitiveSource extends Source { - SensitiveSource() { this.asExpr() instanceof SensitiveExpr } - } - - /** - * An expression used by a broken or weak cryptographic algorithm. - */ - class WeakCryptographicOperationSink extends Sink { - CryptographicOperation application; - - WeakCryptographicOperationSink() { - ( - application.getAlgorithm().isWeak() - or - application.getBlockMode().isWeak() - ) and - this = application.getAnInput() - } - - override DataFlow::Node getInitialization() { result = application.getInitialization() } - } -} diff --git a/go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmQuery.qll b/go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmQuery.qll deleted file mode 100644 index ba24dcf57078..000000000000 --- a/go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmQuery.qll +++ /dev/null @@ -1,43 +0,0 @@ -/** - * Provides a taint tracking configuration for reasoning about - * sensitive information in broken or weak cryptographic algorithms. - * - * Note, for performance reasons: only import this file if - * `BrokenCryptoAlgorithm::Configuration` is needed, otherwise - * `BrokenCryptoAlgorithmCustomizations` should be imported instead. - */ - -import go -import BrokenCryptoAlgorithmCustomizations::BrokenCryptoAlgorithm - -/** - * A taint tracking configuration for sensitive information in broken or weak cryptographic algorithms. - * - * This configuration identifies flows from `Source`s, which are sources of - * sensitive data, to `Sink`s, which is an abstract class representing all - * the places sensitive data may used in broken or weak cryptographic algorithms. Additional sources or sinks can be - * added either by extending the relevant class, or by subclassing this configuration itself, - * and amending the sources and sinks. - */ -private module BrokenCryptoAlgorithmConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { source instanceof Source } - - predicate isSink(DataFlow::Node sink) { sink instanceof Sink } - - predicate isBarrierIn(DataFlow::Node node) { isSource(node) } - - predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } - - predicate observeDiffInformedIncrementalMode() { any() } - - Location getASelectedSinkLocation(DataFlow::Node sink) { - result = sink.(Sink).getLocation() - or - result = sink.(Sink).getInitialization().getLocation() - } -} - -/** - * Taint tracking flow for sensitive information in broken or weak cryptographic algorithms. - */ -module BrokenCryptoAlgorithmFlow = TaintTracking::Global; diff --git a/go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql b/go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql index 714a1cc7b3ec..67b2d0d6d2b4 100644 --- a/go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql +++ b/go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql @@ -1,21 +1,33 @@ /** * @name Use of a broken or weak cryptographic algorithm * @description Using broken or weak cryptographic algorithms can compromise security. - * @kind path-problem + * @kind problem * @problem.severity warning * @security-severity 7.5 * @precision high - * @id go/weak-crypto-algorithm + * @id go/weak-cryptographic-algorithm * @tags security * external/cwe/cwe-327 * external/cwe/cwe-328 */ import go -import semmle.go.security.BrokenCryptoAlgorithmQuery -import BrokenCryptoAlgorithmFlow::PathGraph -from BrokenCryptoAlgorithmFlow::PathNode source, BrokenCryptoAlgorithmFlow::PathNode sink -where BrokenCryptoAlgorithmFlow::flowPath(source, sink) -select sink.getNode(), source, sink, "$@ is used in a weak cryptographic algorithm.", - source.getNode(), "Sensitive data" +from Cryptography::CryptographicOperation operation, string msgPrefix, DataFlow::Node init +where + init = operation.getInitialization() and + // `init` may be a `BlockModeInit`, a `EncryptionAlgorithmInit`, or `operation` itself. + ( + not init instanceof BlockModeInit and + exists(Cryptography::CryptographicAlgorithm algorithm | + algorithm = operation.getAlgorithm() and + algorithm.isWeak() and + msgPrefix = "The cryptographic algorithm " + algorithm.getName() and + not algorithm instanceof Cryptography::HashingAlgorithm + ) + or + not init instanceof EncryptionAlgorithmInit and + operation.getBlockMode().isWeak() and + msgPrefix = "The block mode " + operation.getBlockMode() + ) +select operation, "$@ is broken or weak, and should not be used.", init, msgPrefix diff --git a/go/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.expected b/go/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.expected index 4fbf77dda629..00eb67fea0ba 100644 --- a/go/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.expected +++ b/go/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.expected @@ -1,83 +1,29 @@ -#select -| Crypto.go:36:21:36:28 | password | Crypto.go:36:21:36:28 | password | Crypto.go:36:21:36:28 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:36:21:36:28 | password | Sensitive data | -| Crypto.go:41:22:41:29 | password | Crypto.go:41:22:41:29 | password | Crypto.go:41:22:41:29 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:41:22:41:29 | password | Sensitive data | -| Crypto.go:46:22:46:29 | password | Crypto.go:46:22:46:29 | password | Crypto.go:46:22:46:29 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:46:22:46:29 | password | Sensitive data | -| Crypto.go:51:22:51:29 | password | Crypto.go:51:22:51:29 | password | Crypto.go:51:22:51:29 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:51:22:51:29 | password | Sensitive data | -| Crypto.go:56:22:56:29 | password | Crypto.go:56:22:56:29 | password | Crypto.go:56:22:56:29 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:56:22:56:29 | password | Sensitive data | -| Crypto.go:61:32:61:39 | password | Crypto.go:61:32:61:39 | password | Crypto.go:61:32:61:39 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:61:32:61:39 | password | Sensitive data | -| Crypto.go:66:30:66:37 | password | Crypto.go:66:30:66:37 | password | Crypto.go:66:30:66:37 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:66:30:66:37 | password | Sensitive data | -| Crypto.go:68:59:68:83 | call to NewReader | Crypto.go:68:75:68:82 | password | Crypto.go:68:59:68:83 | call to NewReader | $@ is used in a weak cryptographic algorithm. | Crypto.go:68:75:68:82 | password | Sensitive data | -| Crypto.go:72:10:72:24 | ctrStreamWriter [postupdate] | Crypto.go:72:43:72:50 | password | Crypto.go:72:10:72:24 | ctrStreamWriter [postupdate] | $@ is used in a weak cryptographic algorithm. | Crypto.go:72:43:72:50 | password | Sensitive data | -| Crypto.go:78:30:78:37 | password | Crypto.go:78:30:78:37 | password | Crypto.go:78:30:78:37 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:78:30:78:37 | password | Sensitive data | -| Crypto.go:83:30:83:37 | password | Crypto.go:83:30:83:37 | password | Crypto.go:83:30:83:37 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:83:30:83:37 | password | Sensitive data | -| Crypto.go:91:21:91:33 | call to getPassword | Crypto.go:91:21:91:33 | call to getPassword | Crypto.go:91:21:91:33 | call to getPassword | $@ is used in a weak cryptographic algorithm. | Crypto.go:91:21:91:33 | call to getPassword | Sensitive data | -| Crypto.go:96:22:96:34 | call to getPassword | Crypto.go:96:22:96:34 | call to getPassword | Crypto.go:96:22:96:34 | call to getPassword | $@ is used in a weak cryptographic algorithm. | Crypto.go:96:22:96:34 | call to getPassword | Sensitive data | -| Crypto.go:101:22:101:34 | call to getPassword | Crypto.go:101:22:101:34 | call to getPassword | Crypto.go:101:22:101:34 | call to getPassword | $@ is used in a weak cryptographic algorithm. | Crypto.go:101:22:101:34 | call to getPassword | Sensitive data | -| Crypto.go:106:22:106:29 | password | Crypto.go:106:22:106:29 | password | Crypto.go:106:22:106:29 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:106:22:106:29 | password | Sensitive data | -| Crypto.go:111:22:111:29 | password | Crypto.go:111:22:111:29 | password | Crypto.go:111:22:111:29 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:111:22:111:29 | password | Sensitive data | -| Crypto.go:116:32:116:44 | call to getPassword | Crypto.go:116:32:116:44 | call to getPassword | Crypto.go:116:32:116:44 | call to getPassword | $@ is used in a weak cryptographic algorithm. | Crypto.go:116:32:116:44 | call to getPassword | Sensitive data | -| Crypto.go:121:30:121:42 | call to getPassword | Crypto.go:121:30:121:42 | call to getPassword | Crypto.go:121:30:121:42 | call to getPassword | $@ is used in a weak cryptographic algorithm. | Crypto.go:121:30:121:42 | call to getPassword | Sensitive data | -| Crypto.go:123:59:123:88 | call to NewReader | Crypto.go:123:75:123:87 | call to getPassword | Crypto.go:123:59:123:88 | call to NewReader | $@ is used in a weak cryptographic algorithm. | Crypto.go:123:75:123:87 | call to getPassword | Sensitive data | -| Crypto.go:127:10:127:24 | ctrStreamWriter [postupdate] | Crypto.go:127:43:127:55 | call to getPassword | Crypto.go:127:10:127:24 | ctrStreamWriter [postupdate] | $@ is used in a weak cryptographic algorithm. | Crypto.go:127:43:127:55 | call to getPassword | Sensitive data | -| Crypto.go:133:30:133:37 | password | Crypto.go:133:30:133:37 | password | Crypto.go:133:30:133:37 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:133:30:133:37 | password | Sensitive data | -| Crypto.go:138:30:138:37 | password | Crypto.go:138:30:138:37 | password | Crypto.go:138:30:138:37 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:138:30:138:37 | password | Sensitive data | -| Crypto.go:198:22:198:34 | call to getPassword | Crypto.go:198:22:198:34 | call to getPassword | Crypto.go:198:22:198:34 | call to getPassword | $@ is used in a weak cryptographic algorithm. | Crypto.go:198:22:198:34 | call to getPassword | Sensitive data | -| Crypto.go:205:8:205:10 | buf | Crypto.go:202:9:202:16 | password | Crypto.go:205:8:205:10 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:202:9:202:16 | password | Sensitive data | -| Crypto.go:206:10:206:12 | buf | Crypto.go:202:9:202:16 | password | Crypto.go:206:10:206:12 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:202:9:202:16 | password | Sensitive data | -| Crypto.go:207:20:207:33 | passwordString | Crypto.go:207:20:207:33 | passwordString | Crypto.go:207:20:207:33 | passwordString | $@ is used in a weak cryptographic algorithm. | Crypto.go:207:20:207:33 | passwordString | Sensitive data | -| Crypto.go:208:10:208:12 | buf | Crypto.go:202:9:202:16 | password | Crypto.go:208:10:208:12 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:202:9:202:16 | password | Sensitive data | -| Crypto.go:210:17:210:19 | buf | Crypto.go:202:9:202:16 | password | Crypto.go:210:17:210:19 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:202:9:202:16 | password | Sensitive data | -| Crypto.go:211:11:211:13 | buf | Crypto.go:202:9:202:16 | password | Crypto.go:211:11:211:13 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:202:9:202:16 | password | Sensitive data | -edges -| Crypto.go:68:75:68:82 | password | Crypto.go:68:59:68:83 | call to NewReader | provenance | MaD:1 | -| Crypto.go:72:27:72:51 | call to NewReader | Crypto.go:72:10:72:24 | ctrStreamWriter [postupdate] | provenance | MaD:2 | -| Crypto.go:72:43:72:50 | password | Crypto.go:72:27:72:51 | call to NewReader | provenance | MaD:1 | -| Crypto.go:123:75:123:87 | call to getPassword | Crypto.go:123:59:123:88 | call to NewReader | provenance | MaD:1 | -| Crypto.go:127:27:127:56 | call to NewReader | Crypto.go:127:10:127:24 | ctrStreamWriter [postupdate] | provenance | MaD:2 | -| Crypto.go:127:43:127:55 | call to getPassword | Crypto.go:127:27:127:56 | call to NewReader | provenance | MaD:1 | -| Crypto.go:202:9:202:16 | password | Crypto.go:205:8:205:10 | buf | provenance | | -| Crypto.go:202:9:202:16 | password | Crypto.go:206:10:206:12 | buf | provenance | | -| Crypto.go:202:9:202:16 | password | Crypto.go:208:10:208:12 | buf | provenance | | -| Crypto.go:202:9:202:16 | password | Crypto.go:210:17:210:19 | buf | provenance | | -| Crypto.go:202:9:202:16 | password | Crypto.go:211:11:211:13 | buf | provenance | | -models -| 1 | Summary: bytes; ; false; NewReader; ; ; Argument[0]; ReturnValue; taint; manual | -| 2 | Summary: io; ; false; Copy; ; ; Argument[1]; Argument[0]; taint; manual | -nodes -| Crypto.go:36:21:36:28 | password | semmle.label | password | -| Crypto.go:41:22:41:29 | password | semmle.label | password | -| Crypto.go:46:22:46:29 | password | semmle.label | password | -| Crypto.go:51:22:51:29 | password | semmle.label | password | -| Crypto.go:56:22:56:29 | password | semmle.label | password | -| Crypto.go:61:32:61:39 | password | semmle.label | password | -| Crypto.go:66:30:66:37 | password | semmle.label | password | -| Crypto.go:68:59:68:83 | call to NewReader | semmle.label | call to NewReader | -| Crypto.go:68:75:68:82 | password | semmle.label | password | -| Crypto.go:72:10:72:24 | ctrStreamWriter [postupdate] | semmle.label | ctrStreamWriter [postupdate] | -| Crypto.go:72:27:72:51 | call to NewReader | semmle.label | call to NewReader | -| Crypto.go:72:43:72:50 | password | semmle.label | password | -| Crypto.go:78:30:78:37 | password | semmle.label | password | -| Crypto.go:83:30:83:37 | password | semmle.label | password | -| Crypto.go:91:21:91:33 | call to getPassword | semmle.label | call to getPassword | -| Crypto.go:96:22:96:34 | call to getPassword | semmle.label | call to getPassword | -| Crypto.go:101:22:101:34 | call to getPassword | semmle.label | call to getPassword | -| Crypto.go:106:22:106:29 | password | semmle.label | password | -| Crypto.go:111:22:111:29 | password | semmle.label | password | -| Crypto.go:116:32:116:44 | call to getPassword | semmle.label | call to getPassword | -| Crypto.go:121:30:121:42 | call to getPassword | semmle.label | call to getPassword | -| Crypto.go:123:59:123:88 | call to NewReader | semmle.label | call to NewReader | -| Crypto.go:123:75:123:87 | call to getPassword | semmle.label | call to getPassword | -| Crypto.go:127:10:127:24 | ctrStreamWriter [postupdate] | semmle.label | ctrStreamWriter [postupdate] | -| Crypto.go:127:27:127:56 | call to NewReader | semmle.label | call to NewReader | -| Crypto.go:127:43:127:55 | call to getPassword | semmle.label | call to getPassword | -| Crypto.go:133:30:133:37 | password | semmle.label | password | -| Crypto.go:138:30:138:37 | password | semmle.label | password | -| Crypto.go:198:22:198:34 | call to getPassword | semmle.label | call to getPassword | -| Crypto.go:202:9:202:16 | password | semmle.label | password | -| Crypto.go:205:8:205:10 | buf | semmle.label | buf | -| Crypto.go:206:10:206:12 | buf | semmle.label | buf | -| Crypto.go:207:20:207:33 | passwordString | semmle.label | passwordString | -| Crypto.go:208:10:208:12 | buf | semmle.label | buf | -| Crypto.go:210:17:210:19 | buf | semmle.label | buf | -| Crypto.go:211:11:211:13 | buf | semmle.label | buf | -subpaths +| encryption.go:30:2:30:36 | call to Encrypt | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES | +| encryption.go:34:2:34:42 | call to Seal | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES | +| encryption.go:38:2:38:42 | call to Seal | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES | +| encryption.go:42:2:42:42 | call to Seal | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES | +| encryption.go:46:2:46:42 | call to Seal | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES | +| encryption.go:50:2:50:47 | call to CryptBlocks | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES | +| encryption.go:54:2:54:45 | call to XORKeyStream | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES | +| encryption.go:56:22:56:91 | struct literal | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES | +| encryption.go:59:21:59:68 | &... [postupdate] | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES | +| encryption.go:59:22:59:68 | struct literal | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES | +| encryption.go:59:22:59:68 | struct literal [postupdate] | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES | +| encryption.go:60:10:60:24 | ctrStreamWriter [postupdate] | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES | +| encryption.go:65:2:65:45 | call to XORKeyStream | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES | +| encryption.go:69:2:69:45 | call to XORKeyStream | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES | +| encryption.go:76:2:76:32 | call to Encrypt | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES | +| encryption.go:80:2:80:38 | call to Seal | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES | +| encryption.go:84:2:84:38 | call to Seal | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES | +| encryption.go:88:2:88:42 | call to Seal | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES | +| encryption.go:92:2:92:42 | call to Seal | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES | +| encryption.go:96:2:96:43 | call to CryptBlocks | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES | +| encryption.go:100:2:100:41 | call to XORKeyStream | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES | +| encryption.go:102:22:102:87 | struct literal | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES | +| encryption.go:105:21:105:68 | &... [postupdate] | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES | +| encryption.go:105:22:105:68 | struct literal | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES | +| encryption.go:105:22:105:68 | struct literal [postupdate] | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES | +| encryption.go:106:10:106:24 | ctrStreamWriter [postupdate] | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES | +| encryption.go:111:2:111:45 | call to XORKeyStream | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES | +| encryption.go:115:2:115:45 | call to XORKeyStream | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES | +| encryption.go:166:2:166:33 | call to XORKeyStream | $@ is broken or weak, and should not be used. | encryption.go:166:2:166:33 | call to XORKeyStream | The cryptographic algorithm RC4 | diff --git a/go/ql/test/query-tests/Security/CWE-327/Crypto.go b/go/ql/test/query-tests/Security/CWE-327/Crypto.go deleted file mode 100644 index 1a7afc35cb99..000000000000 --- a/go/ql/test/query-tests/Security/CWE-327/Crypto.go +++ /dev/null @@ -1,254 +0,0 @@ -package main - -import ( - "bytes" - "crypto/aes" - "crypto/cipher" - "crypto/des" - "crypto/md5" - "crypto/rc4" - "crypto/sha1" - "crypto/sha256" - "crypto/sha3" - "crypto/sha512" - "io" - "os" -) - -var dst []byte = make([]byte, 16) -var password []byte = []byte("123456") - -const passwordString string = "correct horse battery staple" - -var public []byte = []byte("hello") - -func getPassword() []byte { - return []byte("123456") -} - -// Note that we do not alert on decryption as we may need to decrypt legacy formats - -func BlockCipherDes() { - // BAD, des is a weak crypto algorithm - block, _ := des.NewCipher(nil) - - block.Encrypt(dst, public) // $ CryptographicOperation="DES. init from line 33." - block.Encrypt(dst, password) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. init from line 33." - block.Decrypt(dst, password) - - gcm1, _ := cipher.NewGCM(block) - gcm1.Seal(nil, nil, public, nil) // $ CryptographicOperation="DES. init from line 33." - gcm1.Seal(nil, nil, password, nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. init from line 33." - gcm1.Open(nil, nil, password, nil) - - gcm2, _ := cipher.NewGCMWithNonceSize(block, 12) - gcm2.Seal(nil, nil, public, nil) // $ CryptographicOperation="DES. init from line 33." - gcm2.Seal(nil, nil, password, nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. init from line 33." - gcm2.Open(nil, nil, password, nil) - - gcm3, _ := cipher.NewGCMWithRandomNonce(block) - gcm3.Seal(nil, nil, public, nil) // $ CryptographicOperation="DES. init from line 33." - gcm3.Seal(nil, nil, password, nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. init from line 33." - gcm3.Open(nil, nil, password, nil) - - gcm4, _ := cipher.NewGCMWithTagSize(block, 12) - gcm4.Seal(nil, nil, public, nil) // $ CryptographicOperation="DES. init from line 33." - gcm4.Seal(nil, nil, password, nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. init from line 33." - gcm4.Open(nil, nil, password, nil) - - cbcEncrypter := cipher.NewCBCEncrypter(block, nil) - cbcEncrypter.CryptBlocks(dst, public) // $ CryptographicOperation="DES. blockMode: CBC. init from lines 33,59." - cbcEncrypter.CryptBlocks(dst, password) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. blockMode: CBC. init from lines 33,59." - cipher.NewCBCDecrypter(block, nil).CryptBlocks(dst, password) - - ctrStream := cipher.NewCTR(block, nil) - ctrStream.XORKeyStream(dst, public) // $ CryptographicOperation="DES. blockMode: CTR. init from lines 33,64." - ctrStream.XORKeyStream(dst, password) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. blockMode: CTR. init from lines 33,64." - - ctrStreamReader := &cipher.StreamReader{S: ctrStream, R: bytes.NewReader(password)} // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. blockMode: CTR. init from lines 33,64." - io.Copy(os.Stdout, ctrStreamReader) - - ctrStreamWriter := &cipher.StreamWriter{S: ctrStream, W: os.Stdout} // $ CryptographicOperation="DES. blockMode: CTR. init from lines 33,64." - io.Copy(ctrStreamWriter, bytes.NewReader(password)) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. blockMode: CTR. init from lines 33,64." - - // deprecated - - cfbStream := cipher.NewCFBEncrypter(block, nil) - cfbStream.XORKeyStream(dst, public) // $ CryptographicOperation="DES. blockMode: CFB. init from lines 33,76." - cfbStream.XORKeyStream(dst, password) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. blockMode: CFB. init from lines 33,76." - cipher.NewCFBDecrypter(block, nil).XORKeyStream(dst, password) - - ofbStream := cipher.NewOFB(block, nil) - ofbStream.XORKeyStream(dst, public) // $ CryptographicOperation="DES. blockMode: OFB. init from lines 33,81." - ofbStream.XORKeyStream(dst, password) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. blockMode: OFB. init from lines 33,81." -} - -func BlockCipherTripleDes() { - // BAD, triple des is a weak crypto algorithm and password is sensitive data - block, _ := des.NewTripleDESCipher(nil) - - block.Encrypt(dst, public) // $ CryptographicOperation="TRIPLEDES. init from line 88." - block.Encrypt(dst, getPassword()) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. init from line 88." - block.Decrypt(dst, getPassword()) - - gcm1, _ := cipher.NewGCM(block) - gcm1.Seal(nil, nil, public, nil) // $ CryptographicOperation="TRIPLEDES. init from line 88." - gcm1.Seal(nil, nil, getPassword(), nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. init from line 88." - gcm1.Open(nil, nil, getPassword(), nil) - - gcm2, _ := cipher.NewGCMWithNonceSize(block, 12) - gcm2.Seal(nil, nil, public, nil) // $ CryptographicOperation="TRIPLEDES. init from line 88." - gcm2.Seal(nil, nil, getPassword(), nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. init from line 88." - gcm2.Open(nil, nil, getPassword(), nil) - - gcm3, _ := cipher.NewGCMWithRandomNonce(block) - gcm3.Seal(nil, nil, public, nil) // $ CryptographicOperation="TRIPLEDES. init from line 88." - gcm3.Seal(nil, nil, password, nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. init from line 88." - gcm3.Open(nil, nil, password, nil) - - gcm4, _ := cipher.NewGCMWithTagSize(block, 12) - gcm4.Seal(nil, nil, public, nil) // $ CryptographicOperation="TRIPLEDES. init from line 88." - gcm4.Seal(nil, nil, password, nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. init from line 88." - gcm4.Open(nil, nil, password, nil) - - cbcEncrypter := cipher.NewCBCEncrypter(block, nil) - cbcEncrypter.CryptBlocks(dst, public) // $ CryptographicOperation="TRIPLEDES. blockMode: CBC. init from lines 114,88." - cbcEncrypter.CryptBlocks(dst, getPassword()) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CBC. init from lines 114,88." - cipher.NewCBCDecrypter(block, nil).CryptBlocks(dst, getPassword()) - - ctrStream := cipher.NewCTR(block, nil) - ctrStream.XORKeyStream(dst, public) // $ CryptographicOperation="TRIPLEDES. blockMode: CTR. init from lines 119,88." - ctrStream.XORKeyStream(dst, getPassword()) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CTR. init from lines 119,88." - - ctrStreamReader := &cipher.StreamReader{S: ctrStream, R: bytes.NewReader(getPassword())} // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CTR. init from lines 119,88." - io.Copy(os.Stdout, ctrStreamReader) - - ctrStreamWriter := &cipher.StreamWriter{S: ctrStream, W: os.Stdout} // $ CryptographicOperation="TRIPLEDES. blockMode: CTR. init from lines 119,88." - io.Copy(ctrStreamWriter, bytes.NewReader(getPassword())) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CTR. init from lines 119,88." - - // deprecated - - cfbStream := cipher.NewCFBEncrypter(block, nil) - cfbStream.XORKeyStream(dst, public) // $ CryptographicOperation="TRIPLEDES. blockMode: CFB. init from lines 131,88." - cfbStream.XORKeyStream(dst, password) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CFB. init from lines 131,88." - cipher.NewCFBDecrypter(block, nil).XORKeyStream(dst, password) - - ofbStream := cipher.NewOFB(block, nil) - ofbStream.XORKeyStream(dst, public) // $ CryptographicOperation="TRIPLEDES. blockMode: OFB. init from lines 136,88." - ofbStream.XORKeyStream(dst, password) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. blockMode: OFB. init from lines 136,88." -} - -func BlockCipherAes() { - // GOOD, aes is a strong crypto algorithm - block, _ := aes.NewCipher(nil) - - block.Encrypt(dst, public) // $ CryptographicOperation="AES. init from line 143." - block.Encrypt(dst, password) // $ CryptographicOperation="AES. init from line 143." - block.Decrypt(dst, password) - - gcm1, _ := cipher.NewGCM(block) - gcm1.Seal(nil, nil, public, nil) // $ CryptographicOperation="AES. init from line 143." - gcm1.Seal(nil, nil, password, nil) // $ CryptographicOperation="AES. init from line 143." - gcm1.Open(nil, nil, password, nil) - - gcm2, _ := cipher.NewGCMWithNonceSize(block, 12) - gcm2.Seal(nil, nil, public, nil) // $ CryptographicOperation="AES. init from line 143." - gcm2.Seal(nil, nil, password, nil) // $ CryptographicOperation="AES. init from line 143." - gcm2.Open(nil, nil, password, nil) - - gcm3, _ := cipher.NewGCMWithRandomNonce(block) - gcm3.Seal(nil, nil, public, nil) // $ CryptographicOperation="AES. init from line 143." - gcm3.Seal(nil, nil, password, nil) // $ CryptographicOperation="AES. init from line 143." - gcm3.Open(nil, nil, password, nil) - - gcm4, _ := cipher.NewGCMWithTagSize(block, 12) - gcm4.Seal(nil, nil, public, nil) // $ CryptographicOperation="AES. init from line 143." - gcm4.Seal(nil, nil, password, nil) // $ CryptographicOperation="AES. init from line 143." - gcm4.Open(nil, nil, password, nil) - - cbcEncrypter := cipher.NewCBCEncrypter(block, nil) - cbcEncrypter.CryptBlocks(dst, public) // $ CryptographicOperation="AES. blockMode: CBC. init from lines 143,169." - cbcEncrypter.CryptBlocks(dst, password) // $ CryptographicOperation="AES. blockMode: CBC. init from lines 143,169." - cipher.NewCBCDecrypter(block, nil).CryptBlocks(dst, password) - - ctrStream := cipher.NewCTR(block, nil) - ctrStream.XORKeyStream(dst, public) // $ CryptographicOperation="AES. blockMode: CTR. init from lines 143,174." - ctrStream.XORKeyStream(dst, password) // $ CryptographicOperation="AES. blockMode: CTR. init from lines 143,174." - - ctrStreamReader := &cipher.StreamReader{S: ctrStream, R: bytes.NewReader(password)} // $ CryptographicOperation="AES. blockMode: CTR. init from lines 143,174." - io.Copy(os.Stdout, ctrStreamReader) - - ctrStreamWriter := &cipher.StreamWriter{S: ctrStream, W: os.Stdout} // $ CryptographicOperation="AES. blockMode: CTR. init from lines 143,174." - io.Copy(ctrStreamWriter, bytes.NewReader(password)) // $ CryptographicOperation="AES. blockMode: CTR. init from lines 143,174." - - // deprecated - - cfbStream := cipher.NewCFBEncrypter(block, nil) - cfbStream.XORKeyStream(dst, public) // $ CryptographicOperation="AES. blockMode: CFB. init from lines 143,186." - cfbStream.XORKeyStream(dst, password) // $ CryptographicOperation="AES. blockMode: CFB. init from lines 143,186." - cipher.NewCFBDecrypter(block, nil).XORKeyStream(dst, password) - - ofbStream := cipher.NewOFB(block, nil) - ofbStream.XORKeyStream(dst, public) // $ CryptographicOperation="AES. blockMode: OFB. init from lines 143,191." - ofbStream.XORKeyStream(dst, password) // $ CryptographicOperation="AES. blockMode: OFB. init from lines 143,191." -} - -func CipherRc4() { - c, _ := rc4.NewCipher(nil) - c.XORKeyStream(dst, getPassword()) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="RC4. init from line 198." -} - -func WeakHashes() { - buf := password // $ Source - - h := md5.New() - h.Sum(buf) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="MD5. init from line 204." - h.Write(buf) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="MD5. init from line 204." - io.WriteString(h, passwordString) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="MD5. init from line 204." - md5.Sum(buf) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="MD5. init from line 208." - - sha1.New().Sum(buf) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="SHA1. init from line 210." - sha1.Sum(buf) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="SHA1. init from line 211." -} - -func StrongHashes() { - buf := password - - sha256.New224().Sum(buf) // $ CryptographicOperation="SHA224. init from line 217." - sha256.Sum224(buf) // $ CryptographicOperation="SHA224. init from line 218." - - sha256.New().Sum(buf) // $ CryptographicOperation="SHA256. init from line 220." - sha256.Sum256(buf) // $ CryptographicOperation="SHA256. init from line 221." - - sha512.New().Sum(buf) // $ CryptographicOperation="SHA512. init from line 223." - sha512.Sum512(buf) // $ CryptographicOperation="SHA512. init from line 224." - - sha512.New384().Sum(buf) // $ CryptographicOperation="SHA384. init from line 226." - sha512.Sum384(buf) // $ CryptographicOperation="SHA384. init from line 227." - - sha512.New512_224().Sum(buf) // $ CryptographicOperation="SHA512224. init from line 229." - sha512.Sum512_224(buf) // $ CryptographicOperation="SHA512224. init from line 230." - - sha512.New512_256().Sum(buf) // $ CryptographicOperation="SHA512256. init from line 232." - sha512.Sum512_256(buf) // $ CryptographicOperation="SHA512256. init from line 233." - - sha3.New224().Sum(buf) // $ CryptographicOperation="SHA3224. init from line 235." - sha3.Sum224(buf) // $ CryptographicOperation="SHA3224. init from line 236." - - sha3.New256().Sum(buf) // $ CryptographicOperation="SHA3256. init from line 238." - sha3.Sum256(buf) // $ CryptographicOperation="SHA3256. init from line 239." - - sha3.New384().Sum(buf) // $ CryptographicOperation="SHA3384. init from line 241." - sha3.Sum384(buf) // $ CryptographicOperation="SHA3384. init from line 242." - - sha3.New512().Sum(buf) // $ CryptographicOperation="SHA3512. init from line 244." - sha3.Sum512(buf) // $ CryptographicOperation="SHA3512. init from line 245." - - sha3.NewSHAKE128().Write(buf) // $ CryptographicOperation="SHAKE128. init from line 247." - sha3.NewCSHAKE128(nil, nil).Write(buf) // $ CryptographicOperation="SHAKE128. init from line 248." - sha3.SumSHAKE128(buf, 100) // $ CryptographicOperation="SHAKE128. init from line 249." - - sha3.NewSHAKE256().Write(buf) // $ CryptographicOperation="SHAKE256. init from line 251." - sha3.NewCSHAKE256(nil, nil).Write(buf) // $ CryptographicOperation="SHAKE256. init from line 252." - sha3.SumSHAKE256(buf, 100) // $ CryptographicOperation="SHAKE256. init from line 253." -} diff --git a/go/ql/test/query-tests/Security/CWE-327/encryption.go b/go/ql/test/query-tests/Security/CWE-327/encryption.go new file mode 100644 index 000000000000..2a1bd53440b8 --- /dev/null +++ b/go/ql/test/query-tests/Security/CWE-327/encryption.go @@ -0,0 +1,167 @@ +package main + +import ( + "bytes" + "crypto/aes" + "crypto/cipher" + "crypto/des" + "crypto/rc4" + "io" + "os" +) + +var dst []byte = make([]byte, 16) +var secretByteSlice []byte = []byte("") + +const secretString string = "" + +var public []byte = []byte("") + +func getUserID() []byte { + return []byte("") +} + +// Note that we do not alert on decryption as we may need to decrypt legacy formats + +func BlockCipherDes() { + // BAD, des is a weak crypto algorithm + block, _ := des.NewCipher(nil) + + block.Encrypt(dst, secretByteSlice) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. init from line 28." + block.Decrypt(dst, secretByteSlice) + + gcm1, _ := cipher.NewGCM(block) + gcm1.Seal(nil, nil, secretByteSlice, nil) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. init from line 28." + gcm1.Open(nil, nil, secretByteSlice, nil) + + gcm2, _ := cipher.NewGCMWithNonceSize(block, 12) + gcm2.Seal(nil, nil, secretByteSlice, nil) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. init from line 28." + gcm2.Open(nil, nil, secretByteSlice, nil) + + gcm3, _ := cipher.NewGCMWithRandomNonce(block) + gcm3.Seal(nil, nil, secretByteSlice, nil) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. init from line 28." + gcm3.Open(nil, nil, secretByteSlice, nil) + + gcm4, _ := cipher.NewGCMWithTagSize(block, 12) + gcm4.Seal(nil, nil, secretByteSlice, nil) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. init from line 28." + gcm4.Open(nil, nil, secretByteSlice, nil) + + cbcEncrypter := cipher.NewCBCEncrypter(block, nil) + cbcEncrypter.CryptBlocks(dst, secretByteSlice) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. blockMode: CBC. init from lines 28,49." + cipher.NewCBCDecrypter(block, nil).CryptBlocks(dst, secretByteSlice) + + ctrStream := cipher.NewCTR(block, nil) + ctrStream.XORKeyStream(dst, secretByteSlice) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. blockMode: CTR. init from lines 28,53." + + ctrStreamReader := &cipher.StreamReader{S: ctrStream, R: bytes.NewReader(secretByteSlice)} // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. blockMode: CTR. init from lines 28,53." + io.Copy(os.Stdout, ctrStreamReader) + + ctrStreamWriter := &cipher.StreamWriter{S: ctrStream, W: os.Stdout} // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. blockMode: CTR. init from lines 28,53." + io.Copy(ctrStreamWriter, bytes.NewReader(secretByteSlice)) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. blockMode: CTR. init from lines 28,53." + + // deprecated + + cfbStream := cipher.NewCFBEncrypter(block, nil) + cfbStream.XORKeyStream(dst, secretByteSlice) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. blockMode: CFB. init from lines 28,64." + cipher.NewCFBDecrypter(block, nil).XORKeyStream(dst, secretByteSlice) + + ofbStream := cipher.NewOFB(block, nil) + ofbStream.XORKeyStream(dst, secretByteSlice) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. blockMode: OFB. init from lines 28,68." +} + +func BlockCipherTripleDes() { + // BAD, triple des is a weak crypto algorithm and secretByteSlice is sensitive data + block, _ := des.NewTripleDESCipher(nil) + + block.Encrypt(dst, getUserID()) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. init from line 74." + block.Decrypt(dst, getUserID()) + + gcm1, _ := cipher.NewGCM(block) + gcm1.Seal(nil, nil, getUserID(), nil) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. init from line 74." + gcm1.Open(nil, nil, getUserID(), nil) + + gcm2, _ := cipher.NewGCMWithNonceSize(block, 12) + gcm2.Seal(nil, nil, getUserID(), nil) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. init from line 74." + gcm2.Open(nil, nil, getUserID(), nil) + + gcm3, _ := cipher.NewGCMWithRandomNonce(block) + gcm3.Seal(nil, nil, secretByteSlice, nil) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. init from line 74." + gcm3.Open(nil, nil, secretByteSlice, nil) + + gcm4, _ := cipher.NewGCMWithTagSize(block, 12) + gcm4.Seal(nil, nil, secretByteSlice, nil) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. init from line 74." + gcm4.Open(nil, nil, secretByteSlice, nil) + + cbcEncrypter := cipher.NewCBCEncrypter(block, nil) + cbcEncrypter.CryptBlocks(dst, getUserID()) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CBC. init from lines 74,95." + cipher.NewCBCDecrypter(block, nil).CryptBlocks(dst, getUserID()) + + ctrStream := cipher.NewCTR(block, nil) + ctrStream.XORKeyStream(dst, getUserID()) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CTR. init from lines 74,99." + + ctrStreamReader := &cipher.StreamReader{S: ctrStream, R: bytes.NewReader(getUserID())} // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CTR. init from lines 74,99." + io.Copy(os.Stdout, ctrStreamReader) + + ctrStreamWriter := &cipher.StreamWriter{S: ctrStream, W: os.Stdout} // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CTR. init from lines 74,99." + io.Copy(ctrStreamWriter, bytes.NewReader(getUserID())) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CTR. init from lines 74,99." + + // deprecated + + cfbStream := cipher.NewCFBEncrypter(block, nil) + cfbStream.XORKeyStream(dst, secretByteSlice) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CFB. init from lines 110,74." + cipher.NewCFBDecrypter(block, nil).XORKeyStream(dst, secretByteSlice) + + ofbStream := cipher.NewOFB(block, nil) + ofbStream.XORKeyStream(dst, secretByteSlice) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. blockMode: OFB. init from lines 114,74." +} + +func BlockCipherAes() { + // GOOD, aes is a strong crypto algorithm + block, _ := aes.NewCipher(nil) + + block.Encrypt(dst, secretByteSlice) // $ CryptographicOperation="AES. init from line 120." + block.Decrypt(dst, secretByteSlice) + + gcm1, _ := cipher.NewGCM(block) + gcm1.Seal(nil, nil, secretByteSlice, nil) // $ CryptographicOperation="AES. init from line 120." + gcm1.Open(nil, nil, secretByteSlice, nil) + + gcm2, _ := cipher.NewGCMWithNonceSize(block, 12) + gcm2.Seal(nil, nil, secretByteSlice, nil) // $ CryptographicOperation="AES. init from line 120." + gcm2.Open(nil, nil, secretByteSlice, nil) + + gcm3, _ := cipher.NewGCMWithRandomNonce(block) + gcm3.Seal(nil, nil, secretByteSlice, nil) // $ CryptographicOperation="AES. init from line 120." + gcm3.Open(nil, nil, secretByteSlice, nil) + + gcm4, _ := cipher.NewGCMWithTagSize(block, 12) + gcm4.Seal(nil, nil, secretByteSlice, nil) // $ CryptographicOperation="AES. init from line 120." + gcm4.Open(nil, nil, secretByteSlice, nil) + + cbcEncrypter := cipher.NewCBCEncrypter(block, nil) + cbcEncrypter.CryptBlocks(dst, secretByteSlice) // $ CryptographicOperation="AES. blockMode: CBC. init from lines 120,141." + cipher.NewCBCDecrypter(block, nil).CryptBlocks(dst, secretByteSlice) + + ctrStream := cipher.NewCTR(block, nil) + ctrStream.XORKeyStream(dst, secretByteSlice) // $ CryptographicOperation="AES. blockMode: CTR. init from lines 120,145." + + ctrStreamReader := &cipher.StreamReader{S: ctrStream, R: bytes.NewReader(secretByteSlice)} // $ CryptographicOperation="AES. blockMode: CTR. init from lines 120,145." + io.Copy(os.Stdout, ctrStreamReader) + + ctrStreamWriter := &cipher.StreamWriter{S: ctrStream, W: os.Stdout} // $ CryptographicOperation="AES. blockMode: CTR. init from lines 120,145." + io.Copy(ctrStreamWriter, bytes.NewReader(secretByteSlice)) // $ CryptographicOperation="AES. blockMode: CTR. init from lines 120,145." + + // deprecated + + cfbStream := cipher.NewCFBEncrypter(block, nil) + cfbStream.XORKeyStream(dst, secretByteSlice) // $ CryptographicOperation="AES. blockMode: CFB. init from lines 120,156." + cipher.NewCFBDecrypter(block, nil).XORKeyStream(dst, secretByteSlice) + + ofbStream := cipher.NewOFB(block, nil) + ofbStream.XORKeyStream(dst, secretByteSlice) // $ CryptographicOperation="AES. blockMode: OFB. init from lines 120,160." +} + +func CipherRc4() { + c, _ := rc4.NewCipher(nil) + c.XORKeyStream(dst, getUserID()) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="RC4. init from line 166." +} From 099dda128561ce349aa4041e1548405a32b48645 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Fri, 31 Oct 2025 15:56:18 +0000 Subject: [PATCH 10/18] Add query for hashing sensitive data with weak hashing algorithm --- ...WeakSensitiveDataHashingCustomizations.qll | 172 ++++++++++++++++++ .../CWE-327/WeakSensitiveDataHashing.qhelp | 104 +++++++++++ .../CWE-327/WeakSensitiveDataHashing.ql | 114 ++++++++++++ .../CWE-327/WeakSensitiveDataHashing.expected | 22 +++ .../CWE-327/WeakSensitiveDataHashing.qlref | 4 + .../test/query-tests/Security/CWE-327/go.mod | 4 +- .../query-tests/Security/CWE-327/hashing.go | 81 +++++++++ .../vendor/golang.org/x/crypto/md4/stub.go | 16 ++ .../golang.org/x/crypto/ripemd160/stub.go | 16 ++ .../Security/CWE-327/vendor/modules.txt | 4 + 10 files changed, 536 insertions(+), 1 deletion(-) create mode 100644 go/ql/lib/semmle/go/security/WeakSensitiveDataHashingCustomizations.qll create mode 100644 go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.qhelp create mode 100644 go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.ql create mode 100644 go/ql/test/query-tests/Security/CWE-327/WeakSensitiveDataHashing.expected create mode 100644 go/ql/test/query-tests/Security/CWE-327/WeakSensitiveDataHashing.qlref create mode 100644 go/ql/test/query-tests/Security/CWE-327/hashing.go create mode 100644 go/ql/test/query-tests/Security/CWE-327/vendor/golang.org/x/crypto/md4/stub.go create mode 100644 go/ql/test/query-tests/Security/CWE-327/vendor/golang.org/x/crypto/ripemd160/stub.go create mode 100644 go/ql/test/query-tests/Security/CWE-327/vendor/modules.txt diff --git a/go/ql/lib/semmle/go/security/WeakSensitiveDataHashingCustomizations.qll b/go/ql/lib/semmle/go/security/WeakSensitiveDataHashingCustomizations.qll new file mode 100644 index 000000000000..f03b93eb7c2e --- /dev/null +++ b/go/ql/lib/semmle/go/security/WeakSensitiveDataHashingCustomizations.qll @@ -0,0 +1,172 @@ +/** + * 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 go +import semmle.go.dataflow.internal.DataFlowPrivate +private import semmle.go.security.SensitiveActions + +/** + * 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 { + SensitiveExpr::Classification classification; + + SensitiveDataAsSource() { + classification = this.asExpr().(SensitiveExpr).getClassification() and + not classification = SensitiveExpr::password() and // (covered in ComputationallyExpensiveHashFunction) + not classification = SensitiveExpr::id() // (not accurate enough) + } + + override SensitiveExpr::Classification getClassification() { result = classification } + } + + /** + * 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 { + SensitiveExpr::Classification classification; + + PasswordAsSource() { + classification = this.asExpr().(SensitiveExpr).getClassification() and + classification = SensitiveExpr::password() + } + + override SensitiveExpr::Classification getClassification() { result = classification } + } + + /** + * 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 + } + } +} diff --git a/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.qhelp b/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.qhelp new file mode 100644 index 000000000000..422cbb835141 --- /dev/null +++ b/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.qhelp @@ -0,0 +1,104 @@ + + + +

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

+
    +
  • + pre-image attacks: if you know a hash value h(x), + you should not be able to easily find the input x. +
    • +
  • + collision attacks: if you know a hash value h(x), + you should not be able to easily find a different input y + with the same hash value h(x) = h(y). +
    • +
+

+ In cases with a limited input space, such as for passwords, the hash + function also needs to be computationally expensive to be resistant to + brute-force attacks. Passwords should also have an unique salt applied + before hashing, but that is not considered by this query. +

+ +

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

+ +

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

+ +

+ Use of broken or weak cryptographic algorithms that are not hashing algorithms, is + handled by the rb/weak-cryptographic-algorithm query. +

+ + + + +

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

+ +
    +
  • + such as Argon2, scrypt, bcrypt, or PBKDF2 for passwords and other data with limited input space. +
    • +
  • + such as SHA-2, or SHA-3 in other cases. +
    • +
+ + + + +

+ The following example shows two functions for checking whether the hash + of a certificate matches a known value -- to prevent tampering. + + The first function uses MD5 that is known to be vulnerable to collision attacks. + + The second function uses SHA-256 that is a strong cryptographic hashing function. +

+ + + + + +

+ The following example shows two functions for hashing passwords. + + The first function uses SHA-256 to hash passwords. Although SHA-256 is a + strong cryptographic hash function, it is not suitable for password + hashing since it is not computationally expensive. +

+ + + + +

+ The second function uses Argon2 (through the argon2 + gem), which is a strong password hashing algorithm (and + includes a per-password salt by default). +

+ + + + + + +
  • OWASP: Password Storage Cheat Sheet
    • +     + +     diff --git a/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.ql b/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.ql new file mode 100644 index 000000000000..bd46bd50a837 --- /dev/null +++ b/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.ql @@ -0,0 +1,114 @@ +/** + * @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 go/weak-sensitive-data-hashing + * @tags security + * external/cwe/cwe-327 + * external/cwe/cwe-328 + * external/cwe/cwe-916 + */ + +import go +import semmle.go.security.WeakSensitiveDataHashingCustomizations + +/** + * 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/go/ql/test/query-tests/Security/CWE-327/WeakSensitiveDataHashing.expected b/go/ql/test/query-tests/Security/CWE-327/WeakSensitiveDataHashing.expected new file mode 100644 index 000000000000..301658e22cb8 --- /dev/null +++ b/go/ql/test/query-tests/Security/CWE-327/WeakSensitiveDataHashing.expected @@ -0,0 +1,22 @@ +#select +| hashing.go:20:8:20:22 | secretByteSlice | hashing.go:20:8:20:22 | secretByteSlice | hashing.go:20:8:20:22 | secretByteSlice | $@ is used in a hashing algorithm (MD5) that is insecure. | hashing.go:20:8:20:22 | secretByteSlice | Sensitive data (secret) | +| hashing.go:21:10:21:24 | secretByteSlice | hashing.go:21:10:21:24 | secretByteSlice | hashing.go:21:10:21:24 | secretByteSlice | $@ is used in a hashing algorithm (MD5) that is insecure. | hashing.go:21:10:21:24 | secretByteSlice | Sensitive data (secret) | +| hashing.go:22:20:22:31 | secretString | hashing.go:22:20:22:31 | secretString | hashing.go:22:20:22:31 | secretString | $@ is used in a hashing algorithm (MD5) that is insecure. | hashing.go:22:20:22:31 | secretString | Sensitive data (secret) | +| hashing.go:23:10:23:24 | secretByteSlice | hashing.go:23:10:23:24 | secretByteSlice | hashing.go:23:10:23:24 | secretByteSlice | $@ is used in a hashing algorithm (MD5) that is insecure. | hashing.go:23:10:23:24 | secretByteSlice | Sensitive data (secret) | +| hashing.go:25:17:25:31 | secretByteSlice | hashing.go:25:17:25:31 | secretByteSlice | hashing.go:25:17:25:31 | secretByteSlice | $@ is used in a hashing algorithm (SHA1) that is insecure. | hashing.go:25:17:25:31 | secretByteSlice | Sensitive data (secret) | +| hashing.go:26:11:26:25 | secretByteSlice | hashing.go:26:11:26:25 | secretByteSlice | hashing.go:26:11:26:25 | secretByteSlice | $@ is used in a hashing algorithm (SHA1) that is insecure. | hashing.go:26:11:26:25 | secretByteSlice | Sensitive data (secret) | +| hashing.go:28:16:28:30 | secretByteSlice | hashing.go:28:16:28:30 | secretByteSlice | hashing.go:28:16:28:30 | secretByteSlice | $@ is used in a hashing algorithm (MD4) that is insecure. | hashing.go:28:16:28:30 | secretByteSlice | Sensitive data (secret) | +| hashing.go:29:22:29:36 | secretByteSlice | hashing.go:29:22:29:36 | secretByteSlice | hashing.go:29:22:29:36 | secretByteSlice | $@ is used in a hashing algorithm (RIPEMD160) that is insecure. | hashing.go:29:22:29:36 | secretByteSlice | Sensitive data (secret) | +| hashing.go:80:16:80:23 | password | hashing.go:80:16:80:23 | password | hashing.go:80:16:80:23 | password | $@ is used in a hashing algorithm (SHA256) that is insecure for password hashing, since it is not a computationally expensive hash function. | hashing.go:80:16:80:23 | password | Sensitive data (password) | +edges +nodes +| hashing.go:20:8:20:22 | secretByteSlice | semmle.label | secretByteSlice | +| hashing.go:21:10:21:24 | secretByteSlice | semmle.label | secretByteSlice | +| hashing.go:22:20:22:31 | secretString | semmle.label | secretString | +| hashing.go:23:10:23:24 | secretByteSlice | semmle.label | secretByteSlice | +| hashing.go:25:17:25:31 | secretByteSlice | semmle.label | secretByteSlice | +| hashing.go:26:11:26:25 | secretByteSlice | semmle.label | secretByteSlice | +| hashing.go:28:16:28:30 | secretByteSlice | semmle.label | secretByteSlice | +| hashing.go:29:22:29:36 | secretByteSlice | semmle.label | secretByteSlice | +| hashing.go:80:16:80:23 | password | semmle.label | password | +subpaths diff --git a/go/ql/test/query-tests/Security/CWE-327/WeakSensitiveDataHashing.qlref b/go/ql/test/query-tests/Security/CWE-327/WeakSensitiveDataHashing.qlref new file mode 100644 index 000000000000..ec9ae0993a26 --- /dev/null +++ b/go/ql/test/query-tests/Security/CWE-327/WeakSensitiveDataHashing.qlref @@ -0,0 +1,4 @@ +query: Security/CWE-327/WeakSensitiveDataHashing.ql +postprocess: + - utils/test/PrettyPrintModels.ql + - utils/test/InlineExpectationsTestQuery.ql diff --git a/go/ql/test/query-tests/Security/CWE-327/go.mod b/go/ql/test/query-tests/Security/CWE-327/go.mod index bf42b84feefd..59c69723ef55 100644 --- a/go/ql/test/query-tests/Security/CWE-327/go.mod +++ b/go/ql/test/query-tests/Security/CWE-327/go.mod @@ -1,3 +1,5 @@ module test -go 1.24 +go 1.24.0 + +require golang.org/x/crypto v0.43.0 diff --git a/go/ql/test/query-tests/Security/CWE-327/hashing.go b/go/ql/test/query-tests/Security/CWE-327/hashing.go new file mode 100644 index 000000000000..4943da353846 --- /dev/null +++ b/go/ql/test/query-tests/Security/CWE-327/hashing.go @@ -0,0 +1,81 @@ +package main + +//go:generate depstubber -vendor golang.org/x/crypto/md4 "" New +//go:generate depstubber -vendor golang.org/x/crypto/ripemd160 "" New + +import ( + "crypto/md5" + "crypto/sha1" + "crypto/sha256" + "crypto/sha3" + "crypto/sha512" + "io" + + "golang.org/x/crypto/md4" + "golang.org/x/crypto/ripemd160" +) + +func WeakHashes() { + h := md5.New() + h.Sum(secretByteSlice) // $ Alert[go/weak-sensitive-data-hashing] CryptographicOperation="MD5. init from line 19." + h.Write(secretByteSlice) // $ Alert[go/weak-sensitive-data-hashing] CryptographicOperation="MD5. init from line 19." + io.WriteString(h, secretString) // $ Alert[go/weak-sensitive-data-hashing] CryptographicOperation="MD5. init from line 19." + md5.Sum(secretByteSlice) // $ Alert[go/weak-sensitive-data-hashing] CryptographicOperation="MD5. init from line 23." + + sha1.New().Sum(secretByteSlice) // $ Alert[go/weak-sensitive-data-hashing] CryptographicOperation="SHA1. init from line 25." + sha1.Sum(secretByteSlice) // $ Alert[go/weak-sensitive-data-hashing] CryptographicOperation="SHA1. init from line 26." + + md4.New().Sum(secretByteSlice) // $ Alert[go/weak-sensitive-data-hashing] CryptographicOperation="MD4. init from line 28." + ripemd160.New().Sum(secretByteSlice) // $ Alert[go/weak-sensitive-data-hashing] CryptographicOperation="RIPEMD160. init from line 29." + + // Only alert when sensitive data is hashed. + md5.New().Sum(public) // $ CryptographicOperation="MD5. init from line 32." + md5.Sum(public) // $ CryptographicOperation="MD5. init from line 33." + sha1.New().Sum(public) // $ CryptographicOperation="SHA1. init from line 34." + sha1.Sum(public) // $ CryptographicOperation="SHA1. init from line 35." +} + +func StrongHashes() { + sha256.New224().Sum(secretByteSlice) // $ CryptographicOperation="SHA224. init from line 39." + sha256.Sum224(secretByteSlice) // $ CryptographicOperation="SHA224. init from line 40." + + sha256.New().Sum(secretByteSlice) // $ CryptographicOperation="SHA256. init from line 42." + sha256.Sum256(secretByteSlice) // $ CryptographicOperation="SHA256. init from line 43." + + sha512.New().Sum(secretByteSlice) // $ CryptographicOperation="SHA512. init from line 45." + sha512.Sum512(secretByteSlice) // $ CryptographicOperation="SHA512. init from line 46." + + sha512.New384().Sum(secretByteSlice) // $ CryptographicOperation="SHA384. init from line 48." + sha512.Sum384(secretByteSlice) // $ CryptographicOperation="SHA384. init from line 49." + + sha512.New512_224().Sum(secretByteSlice) // $ CryptographicOperation="SHA512224. init from line 51." + sha512.Sum512_224(secretByteSlice) // $ CryptographicOperation="SHA512224. init from line 52." + + sha512.New512_256().Sum(secretByteSlice) // $ CryptographicOperation="SHA512256. init from line 54." + sha512.Sum512_256(secretByteSlice) // $ CryptographicOperation="SHA512256. init from line 55." + + sha3.New224().Sum(secretByteSlice) // $ CryptographicOperation="SHA3224. init from line 57." + sha3.Sum224(secretByteSlice) // $ CryptographicOperation="SHA3224. init from line 58." + + sha3.New256().Sum(secretByteSlice) // $ CryptographicOperation="SHA3256. init from line 60." + sha3.Sum256(secretByteSlice) // $ CryptographicOperation="SHA3256. init from line 61." + + sha3.New384().Sum(secretByteSlice) // $ CryptographicOperation="SHA3384. init from line 63." + sha3.Sum384(secretByteSlice) // $ CryptographicOperation="SHA3384. init from line 64." + + sha3.New512().Sum(secretByteSlice) // $ CryptographicOperation="SHA3512. init from line 66." + sha3.Sum512(secretByteSlice) // $ CryptographicOperation="SHA3512. init from line 67." + + sha3.NewSHAKE128().Write(secretByteSlice) // $ CryptographicOperation="SHAKE128. init from line 69." + sha3.NewCSHAKE128(nil, nil).Write(secretByteSlice) // $ CryptographicOperation="SHAKE128. init from line 70." + sha3.SumSHAKE128(secretByteSlice, 100) // $ CryptographicOperation="SHAKE128. init from line 71." + + sha3.NewSHAKE256().Write(secretByteSlice) // $ CryptographicOperation="SHAKE256. init from line 73." + sha3.NewCSHAKE256(nil, nil).Write(secretByteSlice) // $ CryptographicOperation="SHAKE256. init from line 74." + sha3.SumSHAKE256(secretByteSlice, 100) // $ CryptographicOperation="SHAKE256. init from line 75." +} + +func PasswordHashing() { + password := []byte("") + sha256.Sum256(password) // $ Alert[go/weak-sensitive-data-hashing] CryptographicOperation="SHA256. init from line 80." +} diff --git a/go/ql/test/query-tests/Security/CWE-327/vendor/golang.org/x/crypto/md4/stub.go b/go/ql/test/query-tests/Security/CWE-327/vendor/golang.org/x/crypto/md4/stub.go new file mode 100644 index 000000000000..777cc50c0c1c --- /dev/null +++ b/go/ql/test/query-tests/Security/CWE-327/vendor/golang.org/x/crypto/md4/stub.go @@ -0,0 +1,16 @@ +// Code generated by depstubber. DO NOT EDIT. +// This is a simple stub for golang.org/x/crypto/md4, strictly for use in testing. + +// See the LICENSE file for information about the licensing of the original library. +// Source: golang.org/x/crypto/md4 (exports: ; functions: New) + +// Package md4 is a stub of golang.org/x/crypto/md4, generated by depstubber. +package md4 + +import ( + hash "hash" +) + +func New() hash.Hash { + return nil +} diff --git a/go/ql/test/query-tests/Security/CWE-327/vendor/golang.org/x/crypto/ripemd160/stub.go b/go/ql/test/query-tests/Security/CWE-327/vendor/golang.org/x/crypto/ripemd160/stub.go new file mode 100644 index 000000000000..995d3339d8cd --- /dev/null +++ b/go/ql/test/query-tests/Security/CWE-327/vendor/golang.org/x/crypto/ripemd160/stub.go @@ -0,0 +1,16 @@ +// Code generated by depstubber. DO NOT EDIT. +// This is a simple stub for golang.org/x/crypto/ripemd160, strictly for use in testing. + +// See the LICENSE file for information about the licensing of the original library. +// Source: golang.org/x/crypto/ripemd160 (exports: ; functions: New) + +// Package ripemd160 is a stub of golang.org/x/crypto/ripemd160, generated by depstubber. +package ripemd160 + +import ( + hash "hash" +) + +func New() hash.Hash { + return nil +} diff --git a/go/ql/test/query-tests/Security/CWE-327/vendor/modules.txt b/go/ql/test/query-tests/Security/CWE-327/vendor/modules.txt new file mode 100644 index 000000000000..1927762821ca --- /dev/null +++ b/go/ql/test/query-tests/Security/CWE-327/vendor/modules.txt @@ -0,0 +1,4 @@ +# golang.org/x/crypto v0.43.0 +## explicit +golang.org/x/crypto/md4 +golang.org/x/crypto/ripemd160 From ac3920c53e7c5c4162572543c4a0257e016a0e64 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 21 Oct 2025 16:15:43 +0100 Subject: [PATCH 11/18] Add change note --- .../src/change-notes/2025-10-21-go-weak-crypto-algorithm.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 go/ql/src/change-notes/2025-10-21-go-weak-crypto-algorithm.md diff --git a/go/ql/src/change-notes/2025-10-21-go-weak-crypto-algorithm.md b/go/ql/src/change-notes/2025-10-21-go-weak-crypto-algorithm.md new file mode 100644 index 000000000000..d5f9a08fb7f8 --- /dev/null +++ b/go/ql/src/change-notes/2025-10-21-go-weak-crypto-algorithm.md @@ -0,0 +1,5 @@ +--- +category: newQuery +--- +* Added a new query, `go/weak-crypto-algorithm`, to detect the use of a broken or weak cryptographic algorithm. A very simple version of this query was originally contributed as an [experimental query by @dilanbhalla](https://github.com/github/codeql-go/pull/284). +* Added a new query, `go/weak-sensitive-data-hashing`, to detect the use of a broken or weak cryptographic hash algorithm on sensitive data. From 42bd86b58120ce133e4c977ed911e2aae5de6192 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 21 Oct 2025 21:28:19 +0100 Subject: [PATCH 12/18] Fix failing ruby crypto test that lists all algorithms --- ruby/ql/test/library-tests/security/CryptoAlgorithms.expected | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ruby/ql/test/library-tests/security/CryptoAlgorithms.expected b/ruby/ql/test/library-tests/security/CryptoAlgorithms.expected index eedddb2df9ff..3f1ba8e12c67 100644 --- a/ruby/ql/test/library-tests/security/CryptoAlgorithms.expected +++ b/ruby/ql/test/library-tests/security/CryptoAlgorithms.expected @@ -34,6 +34,8 @@ strongHashingAlgorithms | SHA3256 | | SHA3384 | | SHA3512 | +| SHA512224 | +| SHA512256 | | SHAKE128 | | SHAKE256 | | SM3 | From 7aa1d17023f7bfa49ae035830a320cd30ad671b8 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 21 Oct 2025 21:59:46 +0100 Subject: [PATCH 13/18] Fix query suite integration tests --- .../integration-tests/query-suite/go-code-scanning.qls.expected | 2 ++ .../query-suite/go-security-and-quality.qls.expected | 2 ++ .../query-suite/go-security-extended.qls.expected | 2 ++ .../integration-tests/query-suite/not_included_in_qls.expected | 1 - 4 files changed, 6 insertions(+), 1 deletion(-) diff --git a/go/ql/integration-tests/query-suite/go-code-scanning.qls.expected b/go/ql/integration-tests/query-suite/go-code-scanning.qls.expected index 20fcacbc3896..d8d3e0a13e75 100644 --- a/go/ql/integration-tests/query-suite/go-code-scanning.qls.expected +++ b/go/ql/integration-tests/query-suite/go-code-scanning.qls.expected @@ -18,7 +18,9 @@ ql/go/ql/src/Security/CWE-295/DisabledCertificateCheck.ql ql/go/ql/src/Security/CWE-312/CleartextLogging.ql ql/go/ql/src/Security/CWE-322/InsecureHostKeyCallback.ql ql/go/ql/src/Security/CWE-326/InsufficientKeySize.ql +ql/go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql ql/go/ql/src/Security/CWE-327/InsecureTLS.ql +ql/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.ql ql/go/ql/src/Security/CWE-338/InsecureRandomness.ql ql/go/ql/src/Security/CWE-347/MissingJwtSignatureCheck.ql ql/go/ql/src/Security/CWE-352/ConstantOauth2State.ql diff --git a/go/ql/integration-tests/query-suite/go-security-and-quality.qls.expected b/go/ql/integration-tests/query-suite/go-security-and-quality.qls.expected index ee0ec8f42ba7..71bcb2b03300 100644 --- a/go/ql/integration-tests/query-suite/go-security-and-quality.qls.expected +++ b/go/ql/integration-tests/query-suite/go-security-and-quality.qls.expected @@ -41,7 +41,9 @@ ql/go/ql/src/Security/CWE-295/DisabledCertificateCheck.ql ql/go/ql/src/Security/CWE-312/CleartextLogging.ql ql/go/ql/src/Security/CWE-322/InsecureHostKeyCallback.ql ql/go/ql/src/Security/CWE-326/InsufficientKeySize.ql +ql/go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql ql/go/ql/src/Security/CWE-327/InsecureTLS.ql +ql/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.ql ql/go/ql/src/Security/CWE-338/InsecureRandomness.ql ql/go/ql/src/Security/CWE-347/MissingJwtSignatureCheck.ql ql/go/ql/src/Security/CWE-352/ConstantOauth2State.ql diff --git a/go/ql/integration-tests/query-suite/go-security-extended.qls.expected b/go/ql/integration-tests/query-suite/go-security-extended.qls.expected index 9116f7b7ebf6..b0447a6c3eb3 100644 --- a/go/ql/integration-tests/query-suite/go-security-extended.qls.expected +++ b/go/ql/integration-tests/query-suite/go-security-extended.qls.expected @@ -19,7 +19,9 @@ ql/go/ql/src/Security/CWE-295/DisabledCertificateCheck.ql ql/go/ql/src/Security/CWE-312/CleartextLogging.ql ql/go/ql/src/Security/CWE-322/InsecureHostKeyCallback.ql ql/go/ql/src/Security/CWE-326/InsufficientKeySize.ql +ql/go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql ql/go/ql/src/Security/CWE-327/InsecureTLS.ql +ql/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.ql ql/go/ql/src/Security/CWE-338/InsecureRandomness.ql ql/go/ql/src/Security/CWE-347/MissingJwtSignatureCheck.ql ql/go/ql/src/Security/CWE-352/ConstantOauth2State.ql diff --git a/go/ql/integration-tests/query-suite/not_included_in_qls.expected b/go/ql/integration-tests/query-suite/not_included_in_qls.expected index 3b79e47da44f..7b8b86a7a533 100644 --- a/go/ql/integration-tests/query-suite/not_included_in_qls.expected +++ b/go/ql/integration-tests/query-suite/not_included_in_qls.expected @@ -14,7 +14,6 @@ ql/go/ql/src/experimental/CWE-203/Timing.ql ql/go/ql/src/experimental/CWE-285/PamAuthBypass.ql ql/go/ql/src/experimental/CWE-287/ImproperLdapAuth.ql ql/go/ql/src/experimental/CWE-321-V2/HardCodedKeys.ql -ql/go/ql/src/experimental/CWE-327/WeakCryptoAlgorithm.ql ql/go/ql/src/experimental/CWE-369/DivideByZero.ql ql/go/ql/src/experimental/CWE-400/DatabaseCallInLoop.ql ql/go/ql/src/experimental/CWE-522-DecompressionBombs/DecompressionBombs.ql From 867ee66d1516e616fd9bb7688619491b2e928f1b Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Fri, 31 Oct 2025 16:06:24 +0000 Subject: [PATCH 14/18] Fix capitalization of class names --- .../semmle/go/frameworks/CryptoLibraries.qll | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll b/go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll index 0c56d8c7e6a5..6fe299c39ed4 100644 --- a/go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll +++ b/go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll @@ -109,9 +109,9 @@ private module Crypto { } private module Rc4 { - private class CipherXORKeyStream extends CryptographicOperation::Range instanceof DataFlow::CallNode + private class CipherXorKeyStream extends CryptographicOperation::Range instanceof DataFlow::CallNode { - CipherXORKeyStream() { + CipherXorKeyStream() { this.(DataFlow::MethodCallNode) .getTarget() .hasQualifiedName("crypto/rc4", "Cipher", "XORKeyStream") @@ -312,26 +312,26 @@ private module Crypto { } private module Cipher { - private class NewCBCEncrypter extends StdLibNewEncrypter { - NewCBCEncrypter() { this.getTarget().hasQualifiedName("crypto/cipher", "NewCBCEncrypter") } + private class NewCbcEncrypter extends StdLibNewEncrypter { + NewCbcEncrypter() { this.getTarget().hasQualifiedName("crypto/cipher", "NewCBCEncrypter") } override BlockMode getMode() { result = "CBC" } } - private class NewCFBEncrypter extends StdLibNewEncrypter { - NewCFBEncrypter() { this.getTarget().hasQualifiedName("crypto/cipher", "NewCFBEncrypter") } + private class NewCfbEncrypter extends StdLibNewEncrypter { + NewCfbEncrypter() { this.getTarget().hasQualifiedName("crypto/cipher", "NewCFBEncrypter") } override BlockMode getMode() { result = "CFB" } } - private class NewCTR extends StdLibNewEncrypter { - NewCTR() { this.getTarget().hasQualifiedName("crypto/cipher", "NewCTR") } + private class NewCtr extends StdLibNewEncrypter { + NewCtr() { this.getTarget().hasQualifiedName("crypto/cipher", "NewCTR") } override BlockMode getMode() { result = "CTR" } } - private class NewGCM extends StdLibNewEncrypter { - NewGCM() { + private class NewGcm extends StdLibNewEncrypter { + NewGcm() { exists(string name | this.getTarget().hasQualifiedName("crypto/cipher", name) | name = ["NewGCM", "NewGCMWithNonceSize", "NewGCMWithRandomNonce", "NewGCMWithTagSize"] ) @@ -340,8 +340,8 @@ private module Crypto { override BlockMode getMode() { result = "GCM" } } - private class NewOFB extends StdLibNewEncrypter { - NewOFB() { this.getTarget().hasQualifiedName("crypto/cipher", "NewOFB") } + private class NewOfb extends StdLibNewEncrypter { + NewOfb() { this.getTarget().hasQualifiedName("crypto/cipher", "NewOFB") } override BlockMode getMode() { result = "OFB" } } @@ -373,8 +373,8 @@ private module Crypto { } } - private class StreamXORKeyStream extends EncryptionMethodCall { - StreamXORKeyStream() { + private class StreamXorKeyStream extends EncryptionMethodCall { + StreamXorKeyStream() { this.(DataFlow::MethodCallNode) .getTarget() .hasQualifiedName("crypto/cipher", "Stream", "XORKeyStream") and From 55ef5416deb6b0f95482ba60cbcce651b963621e Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Mon, 3 Nov 2025 13:23:11 +0000 Subject: [PATCH 15/18] Add query help examples --- .../CWE-327/BrokenCryptoAlgorithm.qhelp | 8 +-- .../CWE-327/WeakSensitiveDataHashing.qhelp | 21 +++---- go/ql/src/Security/CWE-327/examples/Crypto.go | 55 ++++--------------- .../CWE-327/examples/WeakPasswordHashing.go | 21 +++++++ .../CWE-327/examples/WeakSecretHashing.go | 19 +++++++ 5 files changed, 62 insertions(+), 62 deletions(-) create mode 100644 go/ql/src/Security/CWE-327/examples/WeakPasswordHashing.go create mode 100644 go/ql/src/Security/CWE-327/examples/WeakSecretHashing.go diff --git a/go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.qhelp b/go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.qhelp index 0283e142fbad..a65af78c0aec 100644 --- a/go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.qhelp +++ b/go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.qhelp @@ -28,10 +28,10 @@

    - The following code uses the different packages to encrypt/hash - some secret data. The first few examples uses DES, MD5, RC4, and SHA1, - which are older algorithms that are now considered weak. The following - examples use AES and SHA256, which are stronger, more modern algorithms. + The following code uses the different packages to encrypt + some secret data. The first example uses DES, + which is an older algorithm that is now considered weak. The following + example uses AES, which is a stronger, more modern algorithm.

    diff --git a/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.qhelp b/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.qhelp index 422cbb835141..1fa64e4adaf2 100644 --- a/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.qhelp +++ b/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.qhelp @@ -65,35 +65,28 @@

    The following example shows two functions for checking whether the hash - of a certificate matches a known value -- to prevent tampering. + of a secret matches a known value. - The first function uses MD5 that is known to be vulnerable to collision attacks. + The first function uses SHA-1 that is known to be vulnerable to collision attacks. The second function uses SHA-256 that is a strong cryptographic hashing function.

    - +

    The following example shows two functions for hashing passwords. - The first function uses SHA-256 to hash passwords. Although SHA-256 is a - strong cryptographic hash function, it is not suitable for password + The first example uses SHA-256 to hash passwords. Although + SHA-256 is a strong cryptographic hash function, it is not suitable for password hashing since it is not computationally expensive. -

    - - - -

    - The second function uses Argon2 (through the argon2 - gem), which is a strong password hashing algorithm (and - includes a per-password salt by default). + The second function uses PBKDF2, which is a strong password hashing algorithm.

    - +     diff --git a/go/ql/src/Security/CWE-327/examples/Crypto.go b/go/ql/src/Security/CWE-327/examples/Crypto.go index bc2b2fdeba44..b3f71f0772bc 100644 --- a/go/ql/src/Security/CWE-327/examples/Crypto.go +++ b/go/ql/src/Security/CWE-327/examples/Crypto.go @@ -3,51 +3,18 @@ package main import ( "crypto/aes" "crypto/des" - "crypto/md5" - "crypto/rc4" - "crypto/sha1" - "crypto/sha256" ) -func main() { - public := []byte("hello") - - password := []byte("123456") - buf := password // testing dataflow by passing into different variable - - // BAD, des is a weak crypto algorithm and password is sensitive data - des.NewTripleDESCipher(buf) - - // BAD, md5 is a weak crypto algorithm and password is sensitive data - md5.Sum(buf) - - // BAD, rc4 is a weak crypto algorithm and password is sensitive data - rc4.NewCipher(buf) - - // BAD, sha1 is a weak crypto algorithm and password is sensitive data - sha1.Sum(buf) - - // GOOD, password is sensitive data but aes is a strong crypto algorithm - aes.NewCipher(buf) - - // GOOD, password is sensitive data but sha256 is a strong crypto algorithm - sha256.Sum256(buf) - - // GOOD, des is a weak crypto algorithm but public is not sensitive data - des.NewTripleDESCipher(public) - - // GOOD, md5 is a weak crypto algorithm but public is not sensitive data - md5.Sum(public) - - // GOOD, rc4 is a weak crypto algorithm but public is not sensitive data - rc4.NewCipher(public) - - // GOOD, sha1 is a weak crypto algorithm but public is not sensitive data - sha1.Sum(public) - - // GOOD, aes is a strong crypto algorithm and public is not sensitive data - aes.NewCipher(public) +func EncryptMessageWeak(key []byte, message []byte) (dst []byte) { + // BAD, DES is a weak crypto algorithm + block, _ := des.NewCipher(key) + block.Encrypt(dst, message) + return +} - // GOOD, sha256 is a strong crypto algorithm and public is not sensitive data - sha256.Sum256(public) +func EncryptMessageStrong(key []byte, message []byte) (dst []byte) { + // GOOD, AES is a weak crypto algorithm + block, _ := aes.NewCipher(key) + block.Encrypt(dst, message) + return } diff --git a/go/ql/src/Security/CWE-327/examples/WeakPasswordHashing.go b/go/ql/src/Security/CWE-327/examples/WeakPasswordHashing.go new file mode 100644 index 000000000000..671edede7d36 --- /dev/null +++ b/go/ql/src/Security/CWE-327/examples/WeakPasswordHashing.go @@ -0,0 +1,21 @@ +package main + +import ( + "crypto/pbkdf2" + "crypto/rand" + "crypto/sha256" + "crypto/sha512" +) + +func GetPasswordHashBad(password string) [32]byte { + // BAD, SHA256 is a strong hashing algorithm but it is not computationally expensive + return sha256.Sum256([]byte(password)) +} + +func GetPasswordHashGood(password string) []byte { + // GOOD, PBKDF2 is a strong hashing algorithm and it is computationally expensive + salt := make([]byte, 16) + rand.Read(salt) + key, _ := pbkdf2.Key(sha512.New, password, salt, 4096, 32) + return key +} diff --git a/go/ql/src/Security/CWE-327/examples/WeakSecretHashing.go b/go/ql/src/Security/CWE-327/examples/WeakSecretHashing.go new file mode 100644 index 000000000000..fd65b802548b --- /dev/null +++ b/go/ql/src/Security/CWE-327/examples/WeakSecretHashing.go @@ -0,0 +1,19 @@ +package main + +import ( + "crypto/sha1" + "crypto/sha256" + "slices" +) + +func SecretMatchesKnownHashBad(secret []byte, known_hash []byte) bool { + // BAD, SHA1 is a weak crypto algorithm and secret is sensitive data + h := sha1.New() + return slices.Equal(h.Sum(secret), known_hash) +} + +func SecretMatchesKnownHashGood(secret []byte, known_hash []byte) bool { + // GOOD, SHA256 is a strong hashing algorithm + h := sha256.New() + return slices.Equal(h.Sum(secret), known_hash) +} From 416baf60bab31a037939f136665127de6165c6c6 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Mon, 3 Nov 2025 13:23:43 +0000 Subject: [PATCH 16/18] Fix small mistake in Ruby query help --- .../src/queries/security/cwe-327/WeakSensitiveDataHashing.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ruby/ql/src/queries/security/cwe-327/WeakSensitiveDataHashing.qhelp b/ruby/ql/src/queries/security/cwe-327/WeakSensitiveDataHashing.qhelp index 422cbb835141..33d0c845a66d 100644 --- a/ruby/ql/src/queries/security/cwe-327/WeakSensitiveDataHashing.qhelp +++ b/ruby/ql/src/queries/security/cwe-327/WeakSensitiveDataHashing.qhelp @@ -67,7 +67,7 @@ The following example shows two functions for checking whether the hash of a certificate matches a known value -- to prevent tampering. - The first function uses MD5 that is known to be vulnerable to collision attacks. + The first function uses SHA-1 that is known to be vulnerable to collision attacks. The second function uses SHA-256 that is a strong cryptographic hashing function.

    From d2cd96ef7b035fd78f05bfb5d4f7e4999f5a78ef Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Mon, 3 Nov 2025 17:20:19 +0000 Subject: [PATCH 17/18] Fix diff-informed predicates --- go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll | 4 ---- go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.ql | 4 ++++ 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll b/go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll index 6fe299c39ed4..154ac82e7a2a 100644 --- a/go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll +++ b/go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll @@ -21,8 +21,6 @@ private module HashConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof HashAlgorithmInit } predicate isSink(DataFlow::Node sink) { any() } - - predicate observeDiffInformedIncrementalMode() { any() } } /** Tracks the flow of hash algorithms. */ @@ -50,8 +48,6 @@ private module EncryptionConfig implements DataFlow::ConfigSig { predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { any(BlockModeInit nbcm).step(node1, node2) } - - predicate observeDiffInformedIncrementalMode() { any() } } /** diff --git a/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.ql b/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.ql index bd46bd50a837..0a38d9729f01 100644 --- a/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.ql +++ b/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.ql @@ -39,6 +39,8 @@ module NormalHashFunctionFlow { // make sinks barriers so that we only report the closest instance isSink(node) } + + predicate observeDiffInformedIncrementalMode() { any() } } import TaintTracking::Global @@ -70,6 +72,8 @@ module ComputationallyExpensiveHashFunctionFlow { // make sinks barriers so that we only report the closest instance isSink(node) } + + predicate observeDiffInformedIncrementalMode() { any() } } import TaintTracking::Global From 0d77b8c77bb2836365f46bbd30476be7a73d51cd Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 4 Nov 2025 10:33:04 +0000 Subject: [PATCH 18/18] Remove unnecessary import --- .../go/security/WeakSensitiveDataHashingCustomizations.qll | 1 - 1 file changed, 1 deletion(-) diff --git a/go/ql/lib/semmle/go/security/WeakSensitiveDataHashingCustomizations.qll b/go/ql/lib/semmle/go/security/WeakSensitiveDataHashingCustomizations.qll index f03b93eb7c2e..4893193ef4fe 100644 --- a/go/ql/lib/semmle/go/security/WeakSensitiveDataHashingCustomizations.qll +++ b/go/ql/lib/semmle/go/security/WeakSensitiveDataHashingCustomizations.qll @@ -9,7 +9,6 @@ */ import go -import semmle.go.dataflow.internal.DataFlowPrivate private import semmle.go.security.SensitiveActions /**