From 986f3c30846a0640ff84d55aac9122e60f19e391 Mon Sep 17 00:00:00 2001 From: dilanbhalla Date: Fri, 14 Aug 2020 18:51:56 -0700 Subject: [PATCH] Add experimental query detecting use of an insecure PRNG in a cryptographic context --- .../CWE-327/InsecureRandomness.qhelp | 40 +++++++++++++ .../CWE-327/InsecureRandomness.ql | 18 ++++++ .../InsecureRandomnessCustomizations.qll | 58 +++++++++++++++++++ .../experimental/CWE-327/examples/Crypto.go | 46 ++++++++++----- .../CWE-327/examples/InsecureRandomness.go | 29 ++++++++++ .../CWE-327/InsecureRandomness.expected | 10 ++++ .../CWE-327/InsecureRandomness.go | 29 ++++++++++ .../CWE-327/InsecureRandomness.qlref | 1 + 8 files changed, 218 insertions(+), 13 deletions(-) create mode 100644 ql/src/experimental/CWE-327/InsecureRandomness.qhelp create mode 100644 ql/src/experimental/CWE-327/InsecureRandomness.ql create mode 100644 ql/src/experimental/CWE-327/InsecureRandomnessCustomizations.qll create mode 100644 ql/src/experimental/CWE-327/examples/InsecureRandomness.go create mode 100644 ql/test/experimental/CWE-327/InsecureRandomness.expected create mode 100644 ql/test/experimental/CWE-327/InsecureRandomness.go create mode 100644 ql/test/experimental/CWE-327/InsecureRandomness.qlref diff --git a/ql/src/experimental/CWE-327/InsecureRandomness.qhelp b/ql/src/experimental/CWE-327/InsecureRandomness.qhelp new file mode 100644 index 00000000000..f0f8c59d1eb --- /dev/null +++ b/ql/src/experimental/CWE-327/InsecureRandomness.qhelp @@ -0,0 +1,40 @@ + + + +

+ Generating secure random numbers can be an important part of creating a + secure software system. This can be done using APIs that create + cryptographically secure random numbers. +

+

+ However, using an insecure random number generator can make it easier for an attacker to guess the random + numbers, and thereby break the security of the software system. +

+ + +

+ Be very careful not to use insecure random number generation as keys in cryptographic algorithms. +

+

+ If possible, avoid using math/rand for cryptographic algorithms, and use crypto/rand instead. +

+ + +

+ The example below uses the math/rand package instead of crypto/rand to hash and encrypt a password, + demonstrating an insecure use of random number generation. +

+ + + + + +
  • OWASP: Insecure Randomness.
    • +
  • OWASP: Secure Random Number Generation. +
    • +     + +     diff --git a/ql/src/experimental/CWE-327/InsecureRandomness.ql b/ql/src/experimental/CWE-327/InsecureRandomness.ql new file mode 100644 index 00000000000..98c841b4cb2 --- /dev/null +++ b/ql/src/experimental/CWE-327/InsecureRandomness.ql @@ -0,0 +1,18 @@ +/** + * @name Use of insufficient randomness as the key of a cryptographic algorithm + * @description Using insufficient randomness as the key of a cryptographic algorithm can allow an attacker to compromise security. + * @kind path-problem + * @problem.severity error + * @id go/insecure-randomness + * @tags security + * external/cwe/cwe-327 + */ + +import go +import InsecureRandomnessCustomizations::InsecureRandomness +import DataFlow::PathGraph + +from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink +where cfg.hasFlowPath(source, sink) +select sink.getNode(), source, sink, "$@ is a weak key used in cryptographic algorithm.", + source.getNode(), "Random number" diff --git a/ql/src/experimental/CWE-327/InsecureRandomnessCustomizations.qll b/ql/src/experimental/CWE-327/InsecureRandomnessCustomizations.qll new file mode 100644 index 00000000000..ca293fd3742 --- /dev/null +++ b/ql/src/experimental/CWE-327/InsecureRandomnessCustomizations.qll @@ -0,0 +1,58 @@ +/** + * Provides default sources, sinks and sanitizers for reasoning about + * insufficient random sources used as keys in cryptographic algorithms, + * as well as extension points for adding your own. + */ + +import go + +module InsecureRandomness { + /** + * A data flow source for insufficient random sources + */ + abstract class Source extends DataFlow::Node { } + + /** + * A data flow sink for cryptographic algorithms that take a key as input + */ + abstract class Sink extends DataFlow::Node { } + + /** + * A sanitizer for insufficient random sources used as cryptographic keys + */ + abstract class Sanitizer extends DataFlow::Node { } + + /** + * A random source that is not sufficient for security use. So far this is only made up + * of the math package's rand function, more insufficient random sources can be added here. + */ + class InsecureRandomSource extends Source, DataFlow::CallNode { + InsecureRandomSource() { this.getTarget().getPackage().getPath() = "math/rand" } + } + + /** + * A cryptographic algorithm. + */ + class CryptographicSink extends Sink, DataFlow::Node { + CryptographicSink() { + exists(DataFlow::CallNode call | + call.getTarget().getPackage().getPath().regexpMatch("crypto/.*") and + this = call.getAnArgument() + ) + } + } + + /** + * A configuration depicting taint flow from insufficient random sources to cryptographic + * algorithms that take a key as input. + */ + class Configuration extends TaintTracking::Configuration { + Configuration() { this = "InsecureRandomness" } + + override predicate isSource(DataFlow::Node source) { source instanceof Source } + + override predicate isSink(DataFlow::Node sink) { sink instanceof Sink } + + override predicate isSanitizer(DataFlow::Node node) { none() } + } +} diff --git a/ql/src/experimental/CWE-327/examples/Crypto.go b/ql/src/experimental/CWE-327/examples/Crypto.go index 3f7ee9b4dd2..bc2b2fdeba4 100644 --- a/ql/src/experimental/CWE-327/examples/Crypto.go +++ b/ql/src/experimental/CWE-327/examples/Crypto.go @@ -10,24 +10,44 @@ import ( ) func main() { - password := []byte("password") + public := []byte("hello") - // BAD, des is a weak crypto algorithm - des.NewTripleDESCipher(password) + password := []byte("123456") + buf := password // testing dataflow by passing into different variable - // BAD, md5 is a weak crypto algorithm - md5.Sum(password) + // BAD, des is a weak crypto algorithm and password is sensitive data + des.NewTripleDESCipher(buf) - // BAD, rc4 is a weak crypto algorithm - rc4.NewCipher(password) + // BAD, md5 is a weak crypto algorithm and password is sensitive data + md5.Sum(buf) - // BAD, sha1 is a weak crypto algorithm - sha1.Sum(password) + // BAD, rc4 is a weak crypto algorithm and password is sensitive data + rc4.NewCipher(buf) - // GOOD, aes is a strong crypto algorithm - aes.NewCipher(password) + // BAD, sha1 is a weak crypto algorithm and password is sensitive data + sha1.Sum(buf) - // GOOD, sha256 is a strong crypto algorithm - sha256.Sum256(password) + // 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/src/experimental/CWE-327/examples/InsecureRandomness.go b/ql/src/experimental/CWE-327/examples/InsecureRandomness.go new file mode 100644 index 00000000000..80f25d9b7fc --- /dev/null +++ b/ql/src/experimental/CWE-327/examples/InsecureRandomness.go @@ -0,0 +1,29 @@ +package main + +import ( + "crypto/aes" + "crypto/cipher" + "crypto/sha256" + "encoding/hex" + "io" + "math/rand" +) + +func createHash(key string) string { + hash := sha256.New() + hash.Write([]byte(key)) + return hex.EncodeToString(hash.Sum(nil)) +} + +func encrypt(data []byte, password string) []byte { + + block, _ := aes.NewCipher([]byte(createHash(password))) + gcm, _ := cipher.NewGCM(block) + + nonce := make([]byte, gcm.NonceSize()) + random := rand.New(rand.NewSource(999)) + io.ReadFull(random, nonce) + + ciphertext := gcm.Seal(nonce, nonce, data, nil) + return ciphertext +} diff --git a/ql/test/experimental/CWE-327/InsecureRandomness.expected b/ql/test/experimental/CWE-327/InsecureRandomness.expected new file mode 100644 index 00000000000..0cdf74cf966 --- /dev/null +++ b/ql/test/experimental/CWE-327/InsecureRandomness.expected @@ -0,0 +1,10 @@ +edges +| InsecureRandomness.go:24:12:24:40 | call to New : pointer type | InsecureRandomness.go:27:25:27:29 | nonce | +| InsecureRandomness.go:24:12:24:40 | call to New : pointer type | InsecureRandomness.go:27:32:27:36 | nonce | +nodes +| InsecureRandomness.go:24:12:24:40 | call to New : pointer type | semmle.label | call to New : pointer type | +| InsecureRandomness.go:27:25:27:29 | nonce | semmle.label | nonce | +| InsecureRandomness.go:27:32:27:36 | nonce | semmle.label | nonce | +#select +| InsecureRandomness.go:27:25:27:29 | nonce | InsecureRandomness.go:24:12:24:40 | call to New : pointer type | InsecureRandomness.go:27:25:27:29 | nonce | $@ is a weak key used in cryptographic algorithm. | InsecureRandomness.go:24:12:24:40 | call to New | Random number | +| InsecureRandomness.go:27:32:27:36 | nonce | InsecureRandomness.go:24:12:24:40 | call to New : pointer type | InsecureRandomness.go:27:32:27:36 | nonce | $@ is a weak key used in cryptographic algorithm. | InsecureRandomness.go:24:12:24:40 | call to New | Random number | diff --git a/ql/test/experimental/CWE-327/InsecureRandomness.go b/ql/test/experimental/CWE-327/InsecureRandomness.go new file mode 100644 index 00000000000..80f25d9b7fc --- /dev/null +++ b/ql/test/experimental/CWE-327/InsecureRandomness.go @@ -0,0 +1,29 @@ +package main + +import ( + "crypto/aes" + "crypto/cipher" + "crypto/sha256" + "encoding/hex" + "io" + "math/rand" +) + +func createHash(key string) string { + hash := sha256.New() + hash.Write([]byte(key)) + return hex.EncodeToString(hash.Sum(nil)) +} + +func encrypt(data []byte, password string) []byte { + + block, _ := aes.NewCipher([]byte(createHash(password))) + gcm, _ := cipher.NewGCM(block) + + nonce := make([]byte, gcm.NonceSize()) + random := rand.New(rand.NewSource(999)) + io.ReadFull(random, nonce) + + ciphertext := gcm.Seal(nonce, nonce, data, nil) + return ciphertext +} diff --git a/ql/test/experimental/CWE-327/InsecureRandomness.qlref b/ql/test/experimental/CWE-327/InsecureRandomness.qlref new file mode 100644 index 00000000000..fae138be15b --- /dev/null +++ b/ql/test/experimental/CWE-327/InsecureRandomness.qlref @@ -0,0 +1 @@ +experimental/CWE-327/InsecureRandomness.ql