diff --git a/ql/src/experimental/CWE-327/BrokenCrypto.ql b/ql/src/experimental/CWE-327/BrokenCrypto.ql deleted file mode 100644 index 5e31c8b0b8c..00000000000 --- a/ql/src/experimental/CWE-327/BrokenCrypto.ql +++ /dev/null @@ -1,15 +0,0 @@ -/** - * @name Use of a broken or risky cryptographic algorithm - * @description Using broken or weak cryptographic algorithms can allow an attacker to compromise security. - * @kind problem - * @problem.severity error - * @id go/broken-crypto - * @tags security - */ - -import go -import CryptoLibraries - -from CryptographicOperation badcrypto -where badcrypto.getAlgorithm().isWeak() -select badcrypto, "Use of weak or broken cryptographic algorithm" diff --git a/ql/src/experimental/CWE-327/CryptoLibraries.qll b/ql/src/experimental/CWE-327/CryptoLibraries.qll index 89b1d484b83..4233e42201b 100644 --- a/ql/src/experimental/CWE-327/CryptoLibraries.qll +++ b/ql/src/experimental/CWE-327/CryptoLibraries.qll @@ -6,20 +6,84 @@ import go /** * 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 Wikipedia, OWASP and google (2017). + * + * 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" or + name = "ED25519" or + name = "ES256" or + name = "ECDSA256" or + name = "ES384" or + name = "ECDSA384" or + name = "ES512" or + name = "ECDSA512" or + name = "SHA2" or + name = "SHA224" or + name = "SHA256" or + name = "SHA384" or + name = "SHA512" or + name = "SHA3" + } + predicate isWeakHashingAlgorithm(string name) { + name = "HAVEL128" or + name = "MD2" or + name = "MD4" or name = "MD5" or + name = "PANAMA" or + name = "RIPEMD" or + name = "RIPEMD128" or + name = "RIPEMD256" or + name = "RIPEMD320" or + name = "SHA0" or name = "SHA1" } + predicate isStrongEncryptionAlgorithm(string name) { + name = "AES" or + name = "AES128" or + name = "AES192" or + name = "AES256" or + name = "AES512" or + name = "RSA" or + name = "RABBIT" or + name = "BLOWFISH" + } + predicate isWeakEncryptionAlgorithm(string name) { name = "DES" or - name = "RC4" + name = "3DES" or + name = "TRIPLEDES" or + name = "TDEA" or + name = "TRIPLEDEA" or + name = "ARC2" or + name = "RC2" or + name = "ARC4" or + name = "RC4" or + name = "ARCFOUR" or + name = "ARC5" or + name = "RC5" } + + predicate isStrongPasswordHashingAlgorithm(string name) { + name = "ARGON2" or + name = "PBKDF2" or + name = "BCRYPT" or + name = "SCRYPT" + } + + predicate isWeakPasswordHashingAlgorithm(string name) { none() } } private import AlgorithmNames @@ -28,9 +92,20 @@ private import AlgorithmNames * A cryptographic algorithm. */ private newtype TCryptographicAlgorithm = - MkHashingAlgorithm(string name, boolean isWeak) { isWeakHashingAlgorithm(name) and isWeak = true } or + 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 } /** @@ -46,11 +121,13 @@ abstract class CryptographicAlgorithm extends TCryptographicAlgorithm { abstract string getName(); /** - * Holds if the name of this algorithm matches `name` modulo case, white space, dashes and underscores. + * 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" + getName() + "\\E", _, _)) + // name.toUpperCase().regexpReplaceAll("[-_ ]", "").regexpMatch(".*" + getName() + ".*") } /** @@ -60,7 +137,7 @@ abstract class CryptographicAlgorithm extends TCryptographicAlgorithm { } /** - * A hashing algorithm + * A hashing algorithm such as `MD5` or `SHA512`. */ class HashingAlgorithm extends MkHashingAlgorithm, CryptographicAlgorithm { string name; @@ -74,7 +151,7 @@ class HashingAlgorithm extends MkHashingAlgorithm, CryptographicAlgorithm { } /** - * An encryption algorithm + * An encryption algorithm such as `DES` or `AES512`. */ class EncryptionAlgorithm extends MkEncryptionAlgorithm, CryptographicAlgorithm { string name; @@ -87,10 +164,29 @@ class EncryptionAlgorithm extends MkEncryptionAlgorithm, CryptographicAlgorithm 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. */ -abstract class CryptographicOperation extends Expr { +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(); + /** * Gets the applied algorithm. */ @@ -98,86 +194,70 @@ abstract class CryptographicOperation extends Expr { } /** + * Below are the cryptographic functions that have been implemented so far for this library. * Class that checks for use of Md5 package. */ -class Md5 extends CryptographicOperation { +class Md5 extends CryptographicOperation, DataFlow::CallNode { + Expr input; CryptographicAlgorithm algorithm; - SelectorExpr sel; - CallExpr call; Md5() { - this = call and - algorithm.matchesName(sel.getBase().toString()) and - algorithm.matchesName("MD5") and - sel.getSelector().toString() = call.getCalleeName().toString() and - ( - call.getCalleeName().toString() = "New" or - call.getCalleeName().toString() = "Sum" - ) + getTarget().hasQualifiedName("crypto/md5", ["New", "Sum"]) and + this.getArgument(0).asExpr() = input } + override Expr getInput() { result = input } + override CryptographicAlgorithm getAlgorithm() { result = algorithm } } /** - * Class that checks for use of SHA1 package. + * Class that checks for use of Sha1 package. */ -class Sha1 extends CryptographicOperation { +class Sha1 extends CryptographicOperation, DataFlow::CallNode { + Expr input; CryptographicAlgorithm algorithm; - SelectorExpr sel; - CallExpr call; Sha1() { - this = call and - algorithm.matchesName(sel.getBase().toString()) and - algorithm.matchesName("SHA1") and - sel.getSelector().toString() = call.getCalleeName().toString() and - ( - call.getCalleeName().toString() = "New" or - call.getCalleeName().toString() = "Sum" - ) + getTarget().hasQualifiedName("crypto/sha1", ["New", "Sum"]) and + this.getArgument(0).asExpr() = input } + override Expr getInput() { result = input } + override CryptographicAlgorithm getAlgorithm() { result = algorithm } } /** * Class that checks for use of Des package. */ -class Des extends CryptographicOperation { +class Des extends CryptographicOperation, DataFlow::CallNode { + Expr input; CryptographicAlgorithm algorithm; - SelectorExpr sel; - CallExpr call; Des() { - this = call and - algorithm.matchesName(sel.getBase().toString()) and - algorithm.matchesName("DES") and - sel.getSelector().toString() = call.getCalleeName().toString() and - ( - call.getCalleeName().toString() = "NewCipher" or - call.getCalleeName().toString() = "NewTripleDESCipher" - ) + getTarget().hasQualifiedName("crypto/des", ["NewCipher", "NewTripleDESCipher"]) and + this.getArgument(0).asExpr() = input } + override Expr getInput() { result = input } + override CryptographicAlgorithm getAlgorithm() { result = algorithm } } /** - * Class that checks for use of RC4 package. + * Class that checks for use of Rc4 package. */ -class Rc4 extends CryptographicOperation { +class Rc4 extends CryptographicOperation, DataFlow::CallNode { + Expr input; CryptographicAlgorithm algorithm; - SelectorExpr sel; - CallExpr call; Rc4() { - this = call and - algorithm.matchesName(sel.getBase().toString()) and - algorithm.matchesName("RC4") and - sel.getSelector().toString() = call.getCalleeName().toString() and - call.getCalleeName().toString() = "NewCipher" + getTarget().hasQualifiedName("crypto/rc4", ["NewCipher"]) and + this.getArgument(0).asExpr() = input } + override Expr getInput() { result = input } + override CryptographicAlgorithm getAlgorithm() { result = algorithm } } diff --git a/ql/src/experimental/CWE-327/WeakCryptoAlgorithm.qhelp b/ql/src/experimental/CWE-327/WeakCryptoAlgorithm.qhelp new file mode 100644 index 00000000000..0283e142fba --- /dev/null +++ b/ql/src/experimental/CWE-327/WeakCryptoAlgorithm.qhelp @@ -0,0 +1,55 @@ + + + +

+ Using weak cryptographic algorithms can leave data + vulnerable to being decrypted or forged by an attacker. +

+ +

+ Many cryptographic algorithms provided by cryptography + libraries are known to be weak. Using such an + algorithm means that encrypted or hashed data is less + secure than it appears to be. +

+ + + + +

+ Ensure that you use a strong, modern cryptographic + algorithm. Use at least AES-128 or RSA-2048 for + encryption, and SHA-2 or SHA-3 for secure hashing. +

+ + + + +

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

+ + + + + + +
  • OWASP: Cryptographic Storage Cheat Sheet. +
    • +
  • Wikipedia: Cryptographically Strong Algorithms. +
    • +
  • Wikipedia: Strong Cryptography Examples. +
    • +
  • NIST, FIPS 140 Annex a: Approved Security Functions.
    • +
  • NIST, SP 800-131A: Transitions: Recommendation for Transitioning the Use of Cryptographic Algorithms and Key Lengths.
    • +     + +     diff --git a/ql/src/experimental/CWE-327/WeakCryptoAlgorithm.ql b/ql/src/experimental/CWE-327/WeakCryptoAlgorithm.ql new file mode 100644 index 00000000000..1d24d0e40b6 --- /dev/null +++ b/ql/src/experimental/CWE-327/WeakCryptoAlgorithm.ql @@ -0,0 +1,16 @@ +/** + * @name Use of a weak cryptographic algorithm + * @description Using weak cryptographic algorithms can allow an attacker to compromise security. + * @kind path-problem + * @problem.severity error + * @id go/weak-crypto-algorithm + * @tags security + */ + +import go +import WeakCryptoAlgorithmCustomizations::WeakCryptoAlgorithm +import DataFlow::PathGraph + +from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink +where cfg.hasFlowPath(source, sink) +select sink.getNode(), source, sink, "Sensitive data is used in a weak cryptographic algorithm." diff --git a/ql/src/experimental/CWE-327/WeakCryptoAlgorithmCustomizations.qll b/ql/src/experimental/CWE-327/WeakCryptoAlgorithmCustomizations.qll new file mode 100644 index 00000000000..81bd15a92f3 --- /dev/null +++ b/ql/src/experimental/CWE-327/WeakCryptoAlgorithmCustomizations.qll @@ -0,0 +1,61 @@ +/** + * 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 +private import CryptoLibraries + +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() + ) + } + } + + /** + * A configuration depicting taint flow from sensitive information to weak cryptographic algorithms. + */ + class Configuration extends TaintTracking::Configuration { + Configuration() { this = "WeakCryptoAlgorithm" } + + override predicate isSource(DataFlow::Node source) { source instanceof Source } + + override predicate isSink(DataFlow::Node sink) { sink instanceof Sink } + + override predicate isSanitizer(DataFlow::Node node) { + super.isSanitizer(node) or + node instanceof Sanitizer + } + } +} diff --git a/ql/test/experimental/CWE-327/BadCrypto.go b/ql/src/experimental/CWE-327/examples/Crypto.go similarity index 59% rename from ql/test/experimental/CWE-327/BadCrypto.go rename to ql/src/experimental/CWE-327/examples/Crypto.go index 1feaf165257..3f7ee9b4dd2 100644 --- a/ql/test/experimental/CWE-327/BadCrypto.go +++ b/ql/src/experimental/CWE-327/examples/Crypto.go @@ -1,6 +1,7 @@ package main import ( + "crypto/aes" "crypto/des" "crypto/md5" "crypto/rc4" @@ -9,23 +10,24 @@ import ( ) func main() { - password := []byte("password") + // BAD, des is a weak crypto algorithm des.NewTripleDESCipher(password) // BAD, md5 is a weak crypto algorithm md5.Sum(password) - key := []byte{ 1, 2, 3, 4, 5, 6, 7 } // BAD, rc4 is a weak crypto algorithm - rc4.NewCipher(key) + rc4.NewCipher(password) - data := []byte("this page intentionally left blank.") // BAD, sha1 is a weak crypto algorithm - sha1.Sum(data) + sha1.Sum(password) + + // GOOD, aes is a strong crypto algorithm + aes.NewCipher(password) + + // GOOD, sha256 is a strong crypto algorithm + sha256.Sum256(password) - sha256key := []byte("hello world") - // GOOD, sha256 is a strong crypto algorithm - sha256.Sum256([]byte(sha256key)) } diff --git a/ql/test/experimental/CWE-327/BrokenCrypto.expected b/ql/test/experimental/CWE-327/BrokenCrypto.expected deleted file mode 100644 index 931db0c242d..00000000000 --- a/ql/test/experimental/CWE-327/BrokenCrypto.expected +++ /dev/null @@ -1,4 +0,0 @@ -| BadCrypto.go:15:2:15:33 | call to NewTripleDESCipher | Use of weak or broken cryptographic algorithm | -| BadCrypto.go:18:2:18:18 | call to Sum | Use of weak or broken cryptographic algorithm | -| BadCrypto.go:22:2:22:19 | call to NewCipher | Use of weak or broken cryptographic algorithm | -| BadCrypto.go:26:2:26:15 | call to Sum | Use of weak or broken cryptographic algorithm | diff --git a/ql/test/experimental/CWE-327/BrokenCrypto.qlref b/ql/test/experimental/CWE-327/BrokenCrypto.qlref deleted file mode 100644 index 502955695eb..00000000000 --- a/ql/test/experimental/CWE-327/BrokenCrypto.qlref +++ /dev/null @@ -1 +0,0 @@ -experimental/CWE-327/BrokenCrypto.ql diff --git a/ql/test/experimental/CWE-327/Crypto.go b/ql/test/experimental/CWE-327/Crypto.go new file mode 100644 index 00000000000..bc2b2fdeba4 --- /dev/null +++ b/ql/test/experimental/CWE-327/Crypto.go @@ -0,0 +1,53 @@ +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) + + // GOOD, sha256 is a strong crypto algorithm and public is not sensitive data + sha256.Sum256(public) +} diff --git a/ql/test/experimental/CWE-327/WeakCryptoAlgorithm.expected b/ql/test/experimental/CWE-327/WeakCryptoAlgorithm.expected new file mode 100644 index 00000000000..62f37ef6d6a --- /dev/null +++ b/ql/test/experimental/CWE-327/WeakCryptoAlgorithm.expected @@ -0,0 +1,16 @@ +edges +| Crypto.go:16:9:16:16 | password : slice type | Crypto.go:19:25:19:27 | buf | +| Crypto.go:16:9:16:16 | password : slice type | Crypto.go:22:10:22:12 | buf | +| Crypto.go:16:9:16:16 | password : slice type | Crypto.go:25:16:25:18 | buf | +| Crypto.go:16:9:16:16 | password : slice type | Crypto.go:28:11:28:13 | buf | +nodes +| Crypto.go:16:9:16:16 | password : slice type | semmle.label | password : slice type | +| 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 | +#select +| Crypto.go:19:25:19:27 | buf | Crypto.go:16:9:16:16 | password : slice type | Crypto.go:19:25:19:27 | buf | Sensitive data is used in a weak cryptographic algorithm. | +| Crypto.go:22:10:22:12 | buf | Crypto.go:16:9:16:16 | password : slice type | Crypto.go:22:10:22:12 | buf | Sensitive data is used in a weak cryptographic algorithm. | +| Crypto.go:25:16:25:18 | buf | Crypto.go:16:9:16:16 | password : slice type | Crypto.go:25:16:25:18 | buf | Sensitive data is used in a weak cryptographic algorithm. | +| Crypto.go:28:11:28:13 | buf | Crypto.go:16:9:16:16 | password : slice type | Crypto.go:28:11:28:13 | buf | Sensitive data is used in a weak cryptographic algorithm. | diff --git a/ql/test/experimental/CWE-327/WeakCryptoAlgorithm.qlref b/ql/test/experimental/CWE-327/WeakCryptoAlgorithm.qlref new file mode 100644 index 00000000000..00d68df5a7c --- /dev/null +++ b/ql/test/experimental/CWE-327/WeakCryptoAlgorithm.qlref @@ -0,0 +1 @@ +experimental/CWE-327/WeakCryptoAlgorithm.ql