Add Java heuristic insecure randomness query

java/src/qlpack.yml

@@ -1,6 +1,6 @@
 library: false
 name: githubsecuritylab/codeql-java-queries
-version: 0.7.5
+version: 0.8.0
 suites: suites
 defaultSuiteFile: suites/java.qls
 dependencies:

java/src/security/CWE-338/HeuristicInsecureRandomness.java

@@ -0,0 +1,23 @@
+import java.security.SecureRandom;
+import java.util.Random;
+
+public class HeuristicInsecureRandomness {
+
+  private static final int AUTHORIZATION_CODE_BOUND = 1_000_000;
+
+  // BAD: java.util.Random is used to generate an authorization code. The method name
+  // "generateAuthorizationCode" indicates the result is security-sensitive.
+  public String generateAuthorizationCode(String username) {
+    Random predictableRandom = new Random(username.hashCode());
+    int code = predictableRandom.nextInt(AUTHORIZATION_CODE_BOUND);
+    return String.format("%06d", code);
+  }
+
+  // GOOD: java.security.SecureRandom is a cryptographically strong PRNG, so its output
+  // cannot be predicted even though the method name still looks security-sensitive.
+  public String generateAuthorizationCodeGood(String username) {
+    SecureRandom secureRandom = new SecureRandom();
+    int code = secureRandom.nextInt(AUTHORIZATION_CODE_BOUND);
+    return String.format("%06d", code);
+  }
+}

java/src/security/CWE-338/HeuristicInsecureRandomness.qhelp

@@ -0,0 +1,47 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>Cryptographically weak pseudo-random number generators (PRNGs), such as
+  <code>java.util.Random</code> or <code>Math.random()</code>, produce output that an attacker
+  can predict or reproduce, for example by observing a few outputs or brute-forcing the seed.
+  This is not a problem in most contexts, but if the resulting value is used somewhere that
+  must be unpredictable, such as an authentication token, a password-reset code, an API key, or
+  a session identifier, an attacker who can predict the value may be able to bypass the security
+  control it was meant to protect.</p>
+<p>This query is a heuristic: rather than tracing the random value all the way to a known
+  security-sensitive sink, it flags any value produced by a weak PRNG whose surrounding code
+  (the enclosing method or constructor name, the variable it is assigned to, or the variable
+  holding the <code>Random</code> instance) has a name that suggests a security-sensitive
+  purpose, such as <code>token</code>, <code>secret</code>, <code>otp</code>, or
+  <code>authorizationCode</code>. Review each result to confirm that the value is genuinely used
+  in a security-sensitive way.</p>
+</overview>
+
+<recommendation>
+<p>Use <code>java.security.SecureRandom</code> instead of <code>java.util.Random</code> or
+  <code>Math.random()</code> to generate any value that must be unpredictable.</p>
+</recommendation>
+
+<example>
+<p>In the following example, an authorization code is generated using
+  <code>java.util.Random</code>. Because <code>java.util.Random</code> is seeded from a value
+  that is often predictable (or is easy to brute-force even when it is not), an attacker may be
+  able to predict or reproduce the generated code. The fixed version uses
+  <code>java.security.SecureRandom</code> instead.</p>
+<sample src="HeuristicInsecureRandomness.java" />
+</example>
+
+<references>
+<li>
+  OWASP:
+  <a href="https://owasp.org/www-community/vulnerabilities/Insecure_Randomness">Insecure Randomness</a>
+</li>
+<li>
+  CWE:
+  <a href="https://cwe.mitre.org/data/definitions/338.html">CWE-338: Use of Cryptographically Weak Pseudo-Random Number Generator (PRNG)</a>
+</li>
+</references>
+</qhelp>

java/src/security/CWE-338/HeuristicInsecureRandomness.ql

@@ -0,0 +1,69 @@
+/**
+ * @name Heuristic insecure randomness
+ * @description Using a cryptographically weak pseudo-random number generator, such as
+ *              `java.util.Random`, to produce a value whose surrounding names suggest it is
+ *              security-sensitive may allow an attacker to predict that value.
+ * @kind problem
+ * @problem.severity recommendation
+ * @security-severity 3.7
+ * @precision low
+ * @id githubsecuritylab/java/heuristic-insecure-randomness
+ * @tags security
+ *       external/cwe/cwe-338
+ */
+
+import java
+import semmle.code.java.security.RandomQuery
+import semmle.code.java.security.InsecureRandomnessQuery
+
+/**
+ * Gets a "suspicious" name pattern that suggests a value is security-sensitive.
+ *
+ * This extends the `suspicious()` terms from
+ * `semmle.code.java.security.SensitiveActions` with additional terms that are commonly
+ * used to name values which must be unpredictable, such as tokens, secrets, keys,
+ * nonces, salts, and authorization or verification codes.
+ */
+private string sus() {
+  result =
+    [
+      // Terms taken from the `suspicious()` predicate in `SensitiveActions.qll`.
+      "%password%", "%passwd%", "pwd", "%account%", "%accnt%", "%trusted%", "%refresh%token%",
+      "%secret%token",
+      // Additional terms for values that are expected to be unpredictable.
+      "%token%", "%secret%", "%nonce%", "%salt%", "%otp%", "%passcode%", "%passphrase%",
+      "%credential%", "%apikey%", "%api_key%", "%sessionid%", "%session_id%", "%csrf%", "%xsrf%",
+      "%secretkey%", "%privatekey%", "%signingkey%", "%encryptionkey%", "%authoriz%", "%authcode%",
+      "%verification%code%", "%verificationcode%", "%confirmation%code%", "%confirmationcode%",
+      "%security%code%", "%securitycode%", "%activation%code%", "%activationcode%", "%pincode%"
+    ]
+}
+
+/**
+ * Gets a lowercased identifier name related to the random value produced at `source`
+ * that may indicate the value is used for a security-sensitive purpose.
+ */
+private string getASensitiveContextName(RandomDataSource source) {
+  // The name of the method or constructor that produces the random value.
+  result = source.getEnclosingCallable().getName().toLowerCase()
+  or
+  // The name of a variable that the random value is directly assigned to.
+  exists(Variable v | v.getAnAssignedValue() = source.getOutput() |
+    result = v.getName().toLowerCase()
+  )
+  or
+  // The name of the variable holding the `Random` instance that produced the value.
+  exists(Variable v | source.getQualifier() = v.getAnAccess() | result = v.getName().toLowerCase())
+}
+
+from RandomDataSource source, string name
+where
+  // Restrict to insecure sources of randomness (e.g. `java.util.Random`, `Math.random`),
+  // excluding safe implementations such as `java.security.SecureRandom`.
+  source.getOutput() = any(InsecureRandomnessSource s).asExpr() and
+  // Report each source once, using the alphabetically-first matching name.
+  name = min(string n | n = getASensitiveContextName(source) and n.matches(sus()) | n)
+select source,
+  "Insecure randomness: this value is produced by a cryptographically weak random number generator, but '"
+    + name +
+    "' suggests it is used in a security-sensitive context. Use java.security.SecureRandom instead."

java/test/security/CWE-338/HeuristicInsecureRandomness.expected

@@ -0,0 +1,4 @@
+| HeuristicInsecureRandomness.java:19:16:19:66 | nextInt(...) | Insecure randomness: this value is produced by a cryptographically weak random number generator, but 'generateauthorizationcode' suggests it is used in a security-sensitive context. Use java.security.SecureRandom instead. |
+| HeuristicInsecureRandomness.java:26:17:26:33 | nextInt(...) | Insecure randomness: this value is produced by a cryptographically weak random number generator, but 'token' suggests it is used in a security-sensitive context. Use java.security.SecureRandom instead. |
+| HeuristicInsecureRandomness.java:33:13:33:37 | nextInt(...) | Insecure randomness: this value is produced by a cryptographically weak random number generator, but 'otpgenerator' suggests it is used in a security-sensitive context. Use java.security.SecureRandom instead. |
+| HeuristicInsecureRandomness.java:39:21:39:33 | random(...) | Insecure randomness: this value is produced by a cryptographically weak random number generator, but 'secret' suggests it is used in a security-sensitive context. Use java.security.SecureRandom instead. |

java/test/security/CWE-338/HeuristicInsecureRandomness.java

@@ -0,0 +1,56 @@
+import java.security.SecureRandom;
+import java.util.Random;
+
+/**
+ * Test cases for the `HeuristicInsecureRandomness` query.
+ *
+ * Methods prefixed with a security-sensitive name, or that assign the random value to a
+ * security-sensitive variable, are expected to be reported. Uses of `java.util.Random`
+ * in a non-sensitive context, and any use of `java.security.SecureRandom`, are not.
+ */
+public class HeuristicInsecureRandomness {
+
+  private static final int AUTHORIZATION_CODE_BOUND = 1_000_000;
+
+  // BAD: java.util.Random is used to generate an authorization code (the issue example).
+  // Flagged because the enclosing method name matches a suspicious term ("authoriz").
+  public String generateAuthorizationCode(String username) {
+    Random predictableRandom = new Random(username.hashCode());
+    int code = predictableRandom.nextInt(AUTHORIZATION_CODE_BOUND);
+    return String.format("%06d", code);
+  }
+
+  // BAD: the random value is assigned to a variable named "token".
+  public int nextValue() {
+    Random rng = new Random();
+    int token = rng.nextInt(1000);
+    return token;
+  }
+
+  // BAD: the Random instance is stored in a variable named "otpGenerator".
+  public int compute() {
+    Random otpGenerator = new Random();
+    int n = otpGenerator.nextInt(100);
+    return n;
+  }
+
+  // BAD: Math.random() (which is backed by java.util.Random) produces a value named "secret".
+  public double weakSecret() {
+    double secret = Math.random();
+    return secret;
+  }
+
+  // GOOD: SecureRandom is a safe implementation, so it must not be flagged even though the
+  // method name and variable name both look security-sensitive.
+  public String generatePasswordResetToken() {
+    SecureRandom secureRandom = new SecureRandom();
+    int token = secureRandom.nextInt(1_000_000);
+    return String.format("%06d", token);
+  }
+
+  // GOOD: java.util.Random used in a context that is not security-sensitive.
+  public int rollDice() {
+    Random rng = new Random();
+    return rng.nextInt(6);
+  }
+}

java/test/security/CWE-338/HeuristicInsecureRandomness.qlref

@@ -0,0 +1 @@
+query: security/CWE-338/HeuristicInsecureRandomness.ql