Decouple JndiInjection.qll to reuse the taint tracking configuration

java/ql/src/Security/CWE/CWE-074/JndiInjection.ql

@@ -11,29 +11,9 @@
  */
 
 import java
-import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.security.JndiInjection
+import semmle.code.java.security.JndiInjectionQuery
 import DataFlow::PathGraph
 
-/**
- * A taint-tracking configuration for unvalidated user input that is used in JNDI lookup.
- */
-class JndiInjectionFlowConfig extends TaintTracking::Configuration {
-  JndiInjectionFlowConfig() { this = "JndiInjectionFlowConfig" }
-
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof JndiInjectionSink }
-
-  override predicate isSanitizer(DataFlow::Node node) {
-    node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
-  }
-
-  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
-    any(JndiInjectionAdditionalTaintStep c).step(node1, node2)
-  }
-}
-
 from DataFlow::PathNode source, DataFlow::PathNode sink, JndiInjectionFlowConfig conf
 where conf.hasFlowPath(source, sink)
 select sink.getNode(), source, sink, "JNDI lookup might include name from $@.", source.getNode(),

java/ql/src/semmle/code/java/frameworks/Jndi.qll

@@ -27,6 +27,11 @@ class TypeDirContext extends Interface {
   TypeDirContext() { this.hasQualifiedName("javax.naming.directory", "DirContext") }
 }
 
+/** The class `javax.naming.directory.SearchControls` */
+class TypeSearchControls extends Class {
+  TypeSearchControls() { this.hasQualifiedName("javax.naming.directory", "SearchControls") }
+}
+
 /** The class `javax.naming.ldap.LdapName`. */
 class TypeLdapName extends Class {
   TypeLdapName() { this.hasQualifiedName("javax.naming.ldap", "LdapName") }

java/ql/src/semmle/code/java/frameworks/SpringLdap.qll

@@ -59,6 +59,17 @@ class TypeSpringLdapUtils extends Class {
   TypeSpringLdapUtils() { this.hasQualifiedName("org.springframework.ldap.support", "LdapUtils") }
 }
 
+/**
+ * The interface `org.springframework.ldap.core.LdapOperations` or
+ * `org.springframework.ldap.LdapOperations`
+ */
+class TypeLdapOperations extends Interface {
+  TypeLdapOperations() {
+    this.hasQualifiedName(["org.springframework.ldap.core", "org.springframework.ldap"],
+      "LdapOperations")
+  }
+}
+
 /*--- Methods ---*/
 /**
  * A method with the name `authenticate` declared in

java/ql/src/semmle/code/java/security/JndiInjection.qll

@@ -1,11 +1,10 @@
 /** Provides classes to reason about JNDI injection vulnerabilities. */
 
 import java
-import semmle.code.java.dataflow.DataFlow
-import semmle.code.java.dataflow.DataFlow2
-import semmle.code.java.dataflow.ExternalFlow
-import semmle.code.java.frameworks.Jndi
-import semmle.code.java.frameworks.SpringLdap
+private import semmle.code.java.dataflow.DataFlow
+private import semmle.code.java.dataflow.ExternalFlow
+private import semmle.code.java.frameworks.Jndi
+private import semmle.code.java.frameworks.SpringLdap
 
 /** A data flow sink for unvalidated user input that is used in JNDI lookup. */
 abstract class JndiInjectionSink extends DataFlow::Node { }
@@ -48,19 +47,6 @@ private class ConditionedJndiInjectionSink extends JndiInjectionSink, DataFlow::
   }
 }
 
-/**
- * A method that does a JNDI lookup when it receives a `SearchControls` argument with `setReturningObjFlag` = `true`
- */
-private class UnsafeSearchControlsSink extends JndiInjectionSink {
-  UnsafeSearchControlsSink() {
-    exists(UnsafeSearchControlsConf conf, MethodAccess ma |
-      conf.hasFlowTo(DataFlow::exprNode(ma.getAnArgument()))
-    |
-      this.asExpr() = ma.getArgument(0)
-    )
-  }
-}
-
 /**
  * Tainted value passed to env `Hashtable` as the provider URL by calling
  * `env.put(Context.PROVIDER_URL, tainted)` or `env.setProperty(Context.PROVIDER_URL, tainted)`.
@@ -93,10 +79,10 @@ private class DefaultJndiInjectionSinkModel extends SinkModelCsv {
       [
         "javax.naming;Context;true;lookup;;;Argument[0];jndi-injection",
         "javax.naming;Context;true;lookupLink;;;Argument[0];jndi-injection",
-        "javax.naming;Context;true;doLookup;;;Argument[0];jndi-injection",
         "javax.naming;Context;true;rename;;;Argument[0];jndi-injection",
         "javax.naming;Context;true;list;;;Argument[0];jndi-injection",
         "javax.naming;Context;true;listBindings;;;Argument[0];jndi-injection",
+        "javax.naming;InitialContext;true;doLookup;;;Argument[0];jndi-injection",
         "javax.management.remote;JMXConnector;true;connect;;;Argument[-1];jndi-injection",
         "javax.management.remote;JMXConnectorFactory;false;connect;;;Argument[0];jndi-injection",
         // Spring
@@ -137,87 +123,6 @@ private class DefaultJndiInjectionSinkModel extends SinkModelCsv {
   }
 }
 
-/**
- * Find flows between a `SearchControls` object with `setReturningObjFlag` = `true`
- * and an argument of a `LdapOperations.search` or `DirContext.search` call.
- */
-private class UnsafeSearchControlsConf extends DataFlow2::Configuration {
-  UnsafeSearchControlsConf() { this = "UnsafeSearchControlsConf" }
-
-  override predicate isSource(DataFlow2::Node source) { source instanceof UnsafeSearchControls }
-
-  override predicate isSink(DataFlow2::Node sink) { sink instanceof UnsafeSearchControlsArgument }
-}
-
-/**
- * An argument of type `SearchControls` of a a `LdapOperations.search` or `DirContext.search` call.
- */
-private class UnsafeSearchControlsArgument extends DataFlow2::ExprNode {
-  UnsafeSearchControlsArgument() {
-    exists(MethodAccess ma, Method m |
-      ma.getMethod() = m and
-      ma.getAnArgument() = this.asExpr() and
-      this.asExpr().getType() instanceof TypeSearchControls and
-      m.hasName("search")
-    |
-      m.getDeclaringType().getASourceSupertype*() instanceof TypeLdapOperations or
-      m.getDeclaringType().getASourceSupertype*() instanceof TypeDirContext
-    )
-  }
-}
-
-/**
- * A `SearchControls` object with `setReturningObjFlag` = `true`.
- */
-private class UnsafeSearchControls extends DataFlow2::ExprNode {
-  UnsafeSearchControls() {
-    exists(MethodAccess ma |
-      ma.getMethod() instanceof SetReturningObjFlagMethod and
-      ma.getArgument(0).(CompileTimeConstantExpr).getBooleanValue() = true and
-      this.asExpr() = ma.getQualifier()
-    )
-    or
-    exists(ConstructorCall cc |
-      cc.getConstructedType() instanceof TypeSearchControls and
-      cc.getArgument(4).(CompileTimeConstantExpr).getBooleanValue() = true and
-      this.asExpr() = cc
-    )
-  }
-}
-
-/**
- * The method `SearchControls.setReturningObjFlag`.
- */
-private class SetReturningObjFlagMethod extends Method {
-  SetReturningObjFlagMethod() {
-    this.getDeclaringType() instanceof TypeSearchControls and
-    this.hasName("setReturningObjFlag")
-  }
-}
-
-/**
- * The class `javax.naming.directory.SearchControls`
- */
-private class TypeSearchControls extends Class {
-  TypeSearchControls() { this.hasQualifiedName("javax.naming.directory", "SearchControls") }
-}
-
-/**
- * The interface `org.springframework.ldap.core.LdapOperations` or
- * `org.springframework.ldap.LdapOperations`
- */
-private class TypeLdapOperations extends Interface {
-  TypeLdapOperations() {
-    this.hasQualifiedName(["org.springframework.ldap.core", "org.springframework.ldap"],
-      "LdapOperations")
-  }
-}
-
-/** The class `java.util.Hashtable`. */
-private class TypeHashtable extends Class {
-  TypeHashtable() { this.getSourceDeclaration().hasQualifiedName("java.util", "Hashtable") }
-}
-
 /** A set of additional taint steps to consider when taint tracking JNDI injection related data flows. */
 private class DefaultJndiInjectionAdditionalTaintStep extends JndiInjectionAdditionalTaintStep {
   override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
@@ -294,3 +199,8 @@ private predicate rmiConnectorStep(DataFlow::ExprNode n1, DataFlow::ExprNode n2)
     n2.asExpr() = cc
   )
 }
+
+/** The class `java.util.Hashtable`. */
+private class TypeHashtable extends Class {
+  TypeHashtable() { this.getSourceDeclaration().hasQualifiedName("java.util", "Hashtable") }
+}

java/ql/src/semmle/code/java/security/JndiInjectionQuery.qll

@@ -0,0 +1,100 @@
+import java
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.frameworks.Jndi
+import semmle.code.java.frameworks.SpringLdap
+import semmle.code.java.security.JndiInjection
+
+/**
+ * A taint-tracking configuration for unvalidated user input that is used in JNDI lookup.
+ */
+class JndiInjectionFlowConfig extends TaintTracking::Configuration {
+  JndiInjectionFlowConfig() { this = "JndiInjectionFlowConfig" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof JndiInjectionSink }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+    any(JndiInjectionAdditionalTaintStep c).step(node1, node2)
+  }
+}
+
+/**
+ * A method that does a JNDI lookup when it receives a `SearchControls` argument with `setReturningObjFlag` = `true`
+ */
+private class UnsafeSearchControlsSink extends JndiInjectionSink {
+  UnsafeSearchControlsSink() {
+    exists(UnsafeSearchControlsConf conf, MethodAccess ma |
+      conf.hasFlowTo(DataFlow::exprNode(ma.getAnArgument()))
+    |
+      this.asExpr() = ma.getArgument(0)
+    )
+  }
+}
+
+/**
+ * Find flows between a `SearchControls` object with `setReturningObjFlag` = `true`
+ * and an argument of a `LdapOperations.search` or `DirContext.search` call.
+ */
+private class UnsafeSearchControlsConf extends DataFlow2::Configuration {
+  UnsafeSearchControlsConf() { this = "UnsafeSearchControlsConf" }
+
+  override predicate isSource(DataFlow2::Node source) { source instanceof UnsafeSearchControls }
+
+  override predicate isSink(DataFlow2::Node sink) { sink instanceof UnsafeSearchControlsArgument }
+}
+
+/**
+ * An argument of type `SearchControls` of a a `LdapOperations.search` or `DirContext.search` call.
+ */
+private class UnsafeSearchControlsArgument extends DataFlow2::ExprNode {
+  UnsafeSearchControlsArgument() {
+    exists(MethodAccess ma, Method m |
+      ma.getMethod() = m and
+      ma.getAnArgument() = this.asExpr() and
+      this.asExpr().getType() instanceof TypeSearchControls and
+      m.hasName("search")
+    |
+      m.getDeclaringType().getASourceSupertype*() instanceof TypeLdapOperations or
+      m.getDeclaringType().getASourceSupertype*() instanceof TypeDirContext
+    )
+  }
+}
+
+/**
+ * A `SearchControls` object with `setReturningObjFlag` = `true`.
+ */
+private class UnsafeSearchControls extends DataFlow2::ExprNode {
+  UnsafeSearchControls() {
+    exists(MethodAccess ma |
+      ma.getMethod() instanceof SetReturningObjFlagMethod and
+      ma.getArgument(0).(CompileTimeConstantExpr).getBooleanValue() = true and
+      this.asExpr() = ma.getQualifier()
+    )
+    or
+    exists(ConstructorCall cc |
+      cc.getConstructedType() instanceof TypeSearchControls and
+      cc.getArgument(4).(CompileTimeConstantExpr).getBooleanValue() = true and
+      this.asExpr() = cc
+    )
+  }
+}
+
+/**
+ * The method `SearchControls.setReturningObjFlag`.
+ */
+private class SetReturningObjFlagMethod extends Method {
+  SetReturningObjFlagMethod() {
+    this.getDeclaringType() instanceof TypeSearchControls and
+    this.hasName("setReturningObjFlag")
+  }
+}
+
+/** The class `java.util.Hashtable`. */
+private class TypeHashtable extends Class {
+  TypeHashtable() { this.getSourceDeclaration().hasQualifiedName("java.util", "Hashtable") }
+}

java/ql/test/experimental/query-tests/security/CWE-074-JndiInjection/options

@@ -1 +0,0 @@
-//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.3.8:${testdir}/../../../stubs/shiro-core-1.5.2:${testdir}/../../../../stubs/spring-ldap-2.3.2

java/ql/test/query-tests/security/CWE-074/JndiInjectionTest.ql

@@ -1,25 +1,9 @@
 import java
 import semmle.code.java.dataflow.TaintTracking
 import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.security.JndiInjection
+import semmle.code.java.security.JndiInjectionQuery
 import TestUtilities.InlineExpectationsTest
 
-class Conf extends TaintTracking::Configuration {
-  Conf() { this = "test:cwe:jndiinjection" }
-
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof JndiInjectionSink }
-
-  override predicate isSanitizer(DataFlow::Node node) {
-    node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
-  }
-
-  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
-    any(JndiInjectionAdditionalTaintStep c).step(node1, node2)
-  }
-}
-
 class HasJndiInjectionTest extends InlineExpectationsTest {
   HasJndiInjectionTest() { this = "HasJndiInjectionTest" }
 
@@ -27,7 +11,9 @@ class HasJndiInjectionTest extends InlineExpectationsTest {
 
   override predicate hasActualResult(Location location, string element, string tag, string value) {
     tag = "hasJndiInjection" and
-    exists(DataFlow::Node src, DataFlow::Node sink, Conf conf | conf.hasFlow(src, sink) |
+    exists(DataFlow::Node src, DataFlow::Node sink, JndiInjectionFlowConfig conf |
+      conf.hasFlow(src, sink)
+    |
       sink.getLocation() = location and
       element = sink.toString() and
       value = ""

java/ql/test/query-tests/security/CWE-074/options

@@ -1 +1 @@
-//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/springframework-5.2.3:${testdir}/../../../stubs/shiro-core-1.5.2:${testdir}/../../../stubs/spring-ldap-2.3.2
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/springframework-5.3.8:${testdir}/../../../stubs/shiro-core-1.5.2:${testdir}/../../../stubs/spring-ldap-2.3.2