Add Weak Randomness Query

java/ql/src/Security/CWE/CWE-330/WeakRandomness.qhelp

@@ -0,0 +1,65 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+	<overview>
+		<p>
+			Using a cryptographically weak pseudo-random number generator to generate a security-sensitive value,
+			such as a password, makes it easier for an attacker to predict the value.
+		</p>
+		<p>
+			Pseudo-random number generators generate a sequence of numbers that only approximates the properties
+			of random numbers. The sequence is not truly random because it is completely determined by a
+			relatively small set of initial values, the seed. If the random number generator is
+			cryptographically weak, then this sequence may be easily predictable through outside observations.
+		</p>
+
+	</overview>
+	<recommendation>
+		<p>
+			Use a cryptographically secure pseudo-random number generator if the output is to be used in a
+			security-sensitive context. As a rule of thumb, a value should be considered "security-sensitive"
+			if predicting it would allow the attacker to perform an action that they would otherwise be unable
+			to perform. For example, if an attacker could predict the random password generated for a new user,
+			they would be able to log in as that new user.
+		</p>
+
+        <p>
+            For Java, <code>java.util.Random</code> is not cryptographically secure. Use <code>java.security.SecureRandom</code> instead.
+        </p>
+	</recommendation>
+
+	<example>
+
+		<p>
+			The following examples show different ways of generating a cookie with a random value.
+		</p>
+
+        <p>
+            In the first case, we generate a fresh cookie by appending a random integer to the end of a static
+            string. The random number generator used (<code>Random</code>) is not cryptographically secure,
+            so it may be possible for an attacker to predict the generated cookie.
+        </p>
+
+        <sample src="examples/InsecureRandomnessCookie.java" />
+
+        <p>
+            In the second case, we generate a fresh cookie by appending a random integer to the end of a static
+            string. The random number generator used (<code>SecureRandom</code>) is cryptographically secure,
+            so it is not possible for an attacker to predict the generated cookie.
+        </p>
+
+        <sample src="examples/SecureRandomnessCookie.java" />
+
+	</example>
+
+	<references>
+		<li>Wikipedia: <a href="http://en.wikipedia.org/wiki/Pseudorandom_number_generator">Pseudo-random number generator</a>.</li>
+        <li>
+            Java Docs: <a href="http://docs.oracle.com/javase/8/docs/api/java/util/Random.html">Random</a>.
+        </li>
+        <li>
+            Java Docs: <a href="http://docs.oracle.com/javase/8/docs/api/java/security/SecureRandom.html">SecureRandom</a>.
+        </li>
+	</references>
+</qhelp>

java/ql/src/Security/CWE/CWE-330/WeakRandomness.ql

@@ -0,0 +1,113 @@
+/**
+ * @name Weak Randomness
+ * @description Using a weak source of randomness may allow an attacker to predict the generated values.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 8.6
+ * @precision high
+ * @id java/weak-randomness
+ * @tags security
+ *      external/cwe/cwe-330
+ *      external/cwe/cwe-338
+ */
+
+import java
+import semmle.code.java.frameworks.Servlets
+import semmle.code.java.dataflow.TaintTracking
+import semmle.code.java.security.RandomQuery
+import WeakRandomnessFlow::PathGraph
+
+/**
+ * The `java.util.Random` class.
+ */
+class TypeRandom extends RefType {
+  TypeRandom() { this.hasQualifiedName("java.util", "Random") }
+}
+
+abstract class WeakRandomnessSource extends DataFlow::Node { }
+
+private class JavaRandomSource extends WeakRandomnessSource {
+  JavaRandomSource() {
+    this.asExpr().getType() instanceof TypeRandom and this.asExpr() instanceof ConstructorCall
+  }
+}
+
+private class MathRandomMethodAccess extends WeakRandomnessSource {
+  MathRandomMethodAccess() {
+    exists(MethodAccess ma | this.asExpr() = ma |
+      ma.getMethod().hasName("random") and
+      ma.getMethod().getDeclaringType().hasQualifiedName("java.lang", "Math")
+    )
+  }
+}
+
+abstract private class SafeRandomImplementation extends RefType { }
+
+private class TypeSecureRandom extends SafeRandomImplementation {
+  TypeSecureRandom() { this.hasQualifiedName("java.security", "SecureRandom") }
+}
+
+private class TypeHadoopOsSecureRandom extends SafeRandomImplementation {
+  TypeHadoopOsSecureRandom() {
+    this.hasQualifiedName("org.apache.hadoop.crypto.random", "OsSecureRandom")
+  }
+}
+
+abstract class WeakRandomnessAdditionalTaintStep extends Unit {
+  abstract predicate step(DataFlow::Node node1, DataFlow::Node node2);
+}
+
+abstract class WeakRandomnessSink extends DataFlow::Node { }
+
+private class CookieSink extends WeakRandomnessSink {
+  CookieSink() {
+    this.asExpr().getType() instanceof TypeCookie and
+    exists(MethodAccess ma | ma.getMethod().hasName("addCookie") |
+      ma.getArgument(0) = this.asExpr()
+    )
+  }
+}
+
+/**
+ * Holds if there is a method access which converts `bytes` to the string `str`.
+ */
+private predicate covertsBytesToString(DataFlow::Node bytes, DataFlow::Node str) {
+  bytes.getType().(Array).getElementType().(PrimitiveType).hasName("byte") and
+  str.getType() instanceof TypeString and
+  exists(MethodAccess ma | ma = str.asExpr() | bytes.asExpr() = ma.getAnArgument())
+}
+
+/**
+ * A taint-tracking configuration for weak randomness.
+ */
+module WeakRandomnessConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node src) { src instanceof WeakRandomnessSource }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof WeakRandomnessSink }
+
+  predicate isBarrier(DataFlow::Node n) { n.getTypeBound() instanceof SafeRandomImplementation }
+
+  predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
+    exists(MethodAccess ma, Method m |
+      n1.asExpr() = ma.getQualifier() and
+      ma.getMethod() = m and
+      m.getDeclaringType() instanceof TypeRandom and
+      (
+        m.hasName(["nextInt", "nextLong", "nextFloat", "nextDouble", "nextBoolean", "nextGaussian"]) and
+        n2.asExpr() = ma
+        or
+        m.hasName("nextBytes") and
+        n2.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr() = ma.getArgument(0)
+      )
+    )
+    or
+    covertsBytesToString(n1, n2)
+  }
+}
+
+module WeakRandomnessFlow = TaintTracking::Global<WeakRandomnessConfig>;
+
+from WeakRandomnessFlow::PathNode source, WeakRandomnessFlow::PathNode sink
+where WeakRandomnessFlow::flowPath(source, sink)
+select sink.getNode(), source, sink, "Potential weak randomness due to a $@.", source.getNode(),
+  "weak randomness source."

java/ql/src/Security/CWE/CWE-330/examples/InsecureRandomnessCookie.java

@@ -0,0 +1,9 @@
+Random r = new Random();
+
+byte[] bytes = new byte[16];
+r.nextBytes(bytes);
+
+String cookieValue = encode(bytes);
+
+Cookie cookie = new Cookie("name", cookieValue);
+response.addCookie(cookie);

java/ql/src/Security/CWE/CWE-330/examples/SecureRandomnessCookie.java

@@ -0,0 +1,9 @@
+SecureRandom r = new SecureRandom();
+
+byte[] bytes = new byte[16];
+r.nextBytes(bytes);
+
+String cookieValue = encode(bytes);
+
+Cookie cookie = new Cookie("name", cookieValue);
+response.addCookie(cookie);