Merge pull request #289 from smowton/gorand

(admin) Slightly cleaned up version of Insufficient Randomness

ql/src/experimental/CWE-327/InsecureRandomness.qhelp

@@ -0,0 +1,40 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+	<overview>
+		<p>
+			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.
+		</p>
+		<p>
+			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.
+		</p>
+	</overview>
+	<recommendation>
+		<p>
+			Be very careful not to use insecure random number generation as keys in cryptographic algorithms.
+		</p>
+		<p>
+			If possible, avoid using math/rand for cryptographic algorithms, and use crypto/rand instead.
+		</p>
+	</recommendation>
+	<example>
+		<p>
+			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.
+		</p>
+		<sample src="examples/InsecureRandomness.go" />
+	</example>
+
+
+	<references>
+		<li>OWASP: <a href="https://owasp.org/www-community/vulnerabilities/Insecure_Randomness">Insecure Randomness</a>.</li>
+		<li>OWASP: <a
+		href="https://cheatsheetseries.owasp.org/cheatsheets/Cryptographic_Storage_Cheat_Sheet.html#secure-random-number-generation">Secure Random Number Generation</a>.
+		</li>
+	</references>
+
+</qhelp>

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"

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() }
+  }
+}

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)
 }

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

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 |

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

ql/test/experimental/CWE-327/InsecureRandomness.qlref

@@ -0,0 +1 @@
+experimental/CWE-327/InsecureRandomness.ql