From 2e96cbb4fb40dc19917051fc3bd132a750628ec3 Mon Sep 17 00:00:00 2001
From: Ed Minnix <egregius313@github.com>
Date: Tue, 11 Jul 2023 17:42:34 -0400
Subject: [PATCH] Add ThreadLocalRandom.current as another source

---
 .../code/java/security/WeakRandomnessQuery.qll   | 16 +++++++++++++++-
 .../security/CWE-330/WeakRandomCookies.java      |  6 ++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/java/ql/lib/semmle/code/java/security/WeakRandomnessQuery.qll b/java/ql/lib/semmle/code/java/security/WeakRandomnessQuery.qll
index 894b9480c52ef..20e8e66a52ce2 100644
--- a/java/ql/lib/semmle/code/java/security/WeakRandomnessQuery.qll
+++ b/java/ql/lib/semmle/code/java/security/WeakRandomnessQuery.qll
@@ -29,6 +29,9 @@ private class JavaRandomSource extends WeakRandomnessSource {
   }
 }
 
+/**
+ * A node representing a call to one of the methods of `org.apache.commons.lang.RandomStringUtils`.
+ */
 private class ApacheRandomStringUtilsMethodAccessSource extends WeakRandomnessSource {
   ApacheRandomStringUtilsMethodAccessSource() {
     exists(MethodAccess ma | this.asExpr() = ma |
@@ -44,6 +47,17 @@ private class ApacheRandomStringUtilsMethodAccessSource extends WeakRandomnessSo
   }
 }
 
+private class ThreadLocalRandomSource extends WeakRandomnessSource {
+  ThreadLocalRandomSource() {
+    exists(MethodAccess ma | this.asExpr() = ma |
+      ma.getMethod().hasName("current") and
+      ma.getMethod()
+          .getDeclaringType()
+          .hasQualifiedName("java.util.concurrent", "ThreadLocalRandom")
+    )
+  }
+}
+
 /**
  * The `random` method of `java.lang.Math`.
  */
@@ -123,7 +137,7 @@ module WeakRandomnessConfig implements DataFlow::ConfigSig {
     exists(MethodAccess ma, Method m |
       n1.asExpr() = ma.getQualifier() and
       ma.getMethod() = m and
-      m.getDeclaringType() instanceof TypeRandom and
+      m.getDeclaringType().getAnAncestor() instanceof TypeRandom and
       (
         m.hasName(["nextInt", "nextLong", "nextFloat", "nextDouble", "nextBoolean", "nextGaussian"]) and
         n2.asExpr() = ma
diff --git a/java/ql/test/query-tests/security/CWE-330/WeakRandomCookies.java b/java/ql/test/query-tests/security/CWE-330/WeakRandomCookies.java
index e1862ca1431cd..92b240c11c9ee 100644
--- a/java/ql/test/query-tests/security/CWE-330/WeakRandomCookies.java
+++ b/java/ql/test/query-tests/security/CWE-330/WeakRandomCookies.java
@@ -1,5 +1,6 @@
 import java.io.IOException;
 import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
 import java.security.SecureRandom;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
@@ -36,5 +37,10 @@ public void doGet() {
         // GOOD: The cookie value is unpredictable.
         Cookie cookie4 = new Cookie("name", new String(bytes2));
         response.addCookie(cookie4);
+
+        ThreadLocalRandom tlr = ThreadLocalRandom.current();
+
+        Cookie cookie5 = new Cookie("name", Integer.toString(tlr.nextInt()));
+        response.addCookie(cookie5); // $hasWeakRandomFlow
     }
 }