Improve source of randomness detection

Also sanitize flow out of sinks to avoid overlapping paths
github · Nov 8, 2023 · bafbdc5 · bafbdc5
1 parent cf401d4
commit bafbdc5
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 1 deletion.
diff --git a/java/ql/lib/semmle/code/java/security/RandomDataSource.qll b/java/ql/lib/semmle/code/java/security/RandomDataSource.qll
@@ -3,6 +3,7 @@
  */
 
 import java
+private import semmle.code.java.dataflow.TypeFlow
 
 /**
  * A method access that returns random data or writes random data to an argument.
@@ -43,6 +44,9 @@ abstract class RandomDataSource extends MethodCall {
    * in the case where it writes random data to that argument.
    */
   abstract Expr getOutput();
+
+  /** Gets the type of the source of randomness used by this call. */
+  RefType getSourceOfRandomness() { boundOrStaticType(this.getQualifier(), result) }
 }
 
 /**
@@ -167,4 +171,18 @@ class ApacheCommonsRandomStringSource extends RandomDataSource {
   }
 
   override Expr getOutput() { result = this }
+
+  override RefType getSourceOfRandomness() {
+    if
+      this.getMethod().hasStringSignature("random(int, int, int, boolean, boolean, char[], Random)")
+    then boundOrStaticType(this.getArgument(6), result)
+    else result.hasQualifiedName("java.util", "Random")
+  }
+}
+
+/** Holds if `t` is the static type of `e`, or an upper bound of the runtime type of `e`. */
+private predicate boundOrStaticType(Expr e, RefType t) {
+  exprTypeFlow(e, t, false)
+  or
+  t = e.getType()
 }
diff --git a/java/ql/lib/semmle/code/java/security/WeakRandomnessQuery.qll b/java/ql/lib/semmle/code/java/security/WeakRandomnessQuery.qll
@@ -20,7 +20,7 @@ abstract class WeakRandomnessSource extends DataFlow::Node { }
 private class RandomMethodSource extends WeakRandomnessSource {
   RandomMethodSource() {
     exists(RandomDataSource s | this.asExpr() = s.getOutput() |
-      not s.getQualifier().getType() instanceof SafeRandomImplementation
+      not s.getSourceOfRandomness() instanceof SafeRandomImplementation
     )
   }
 }
@@ -70,6 +70,8 @@ module WeakRandomnessConfig implements DataFlow::ConfigSig {
 
   predicate isBarrierIn(DataFlow::Node n) { isSource(n) }
 
+  predicate isBarrierOut(DataFlow::Node n) { isSink(n) }
+
   predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
     n1.asExpr() = n2.asExpr().(BinaryExpr).getAnOperand()
     or