CodeQL query to detect JNDI injections

Author: ggolawski

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

@@ -0,0 +1,21 @@
+import javax.naming.Context;
+import javax.naming.InitialContext;
+
+public void jndiLookup(HttpServletRequest request) throws NamingException {
+  String name = request.getParameter("name");
+
+  Hashtable<String, String> env = new Hashtable<String, String>();
+  env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.rmi.registry.RegistryContextFactory");
+  env.put(Context.PROVIDER_URL, "rmi://trusted-server:1099");
+  InitialContext ctx = new InitialContext(env);
+
+  // BAD: User input used in lookup
+  ctx.lookup(name);
+
+  // GOOD: The name is validated before being used in lookup
+  if (isValid(name)) {
+    ctx.lookup(name);
+  } else {
+    // Reject the request
+  }
+}

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

@@ -0,0 +1,36 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>The Java Naming and Directory Interface (JNDI) is a Java API for a directory service that allows
+Java software clients to discover and look up data and resources (in the form of Java objects) via
+a name. If the name being used to look up the data is controlled by the user, it can point to a
+malicious server, which can return an arbitrary object. In the worst case, this can allow remote
+code execution.</p>
+</overview>
+
+<recommendation>
+<p>The general recommendation is to not pass untrusted data to the <code>InitialContext.lookup
+</code> method. If the name being used to look up the object must be provided by the user, make
+sure that it's not in the form of an absolute URL or that it's the URL pointing to a trused server.
+</p>
+</recommendation>
+
+<example>
+<p>In the following examples, the code accepts a name from the user, which it uses to look up an
+object.</p>
+
+<p>In the first example, the user provided name is used to look up an object.</p>
+
+<p>The second example validates the name before using it to look up an object.</p>
+
+<sample src="JndiInjection.java" />
+</example>
+
+<references>
+<li>Oracle: <a href="https://docs.oracle.com/javase/8/docs/technotes/guides/jndi/">Java Naming and Directory Interface (JNDI)</a>.</li>
+<li>Black Hat materials: <a href="https://www.blackhat.com/docs/us-16/materials/us-16-Munoz-A-Journey-From-JNDI-LDAP-Manipulation-To-RCE-wp.pdf">A Journey from JNDI/LDAP Manipulation to Remote Code Execution Dream Land</a>.</li>
+<li>Veracode: <a href="https://www.veracode.com/blog/research/exploiting-jndi-injections-java">Exploiting JNDI Injections in Java</a>.</li>
+</references>
+</qhelp>

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

@@ -0,0 +1,21 @@
+/**
+ * @name JNDI lookup with user-controlled name
+ * @description Doing a JNDI lookup with user-controlled name can lead to download an untrusted
+ *              object and to execution of arbitrary code.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id java/jndi-injection
+ * @tags security
+ *       external/cwe/cwe-074
+ */
+
+import java
+import semmle.code.java.dataflow.FlowSources
+import JndiInjectionLib
+import DataFlow::PathGraph
+
+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(),
+  "this user input"

java/ql/src/experimental/Security/CWE/CWE-074/JndiInjectionLib.qll

@@ -0,0 +1,91 @@
+import java
+import semmle.code.java.dataflow.FlowSources
+import DataFlow
+import experimental.semmle.code.java.frameworks.Jndi
+import experimental.semmle.code.java.frameworks.spring.SpringJndi
+import experimental.semmle.code.java.frameworks.Shiro
+
+/**
+ * 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) {
+    compositeNameStep(node1, node2)
+  }
+}
+
+/**
+ * JNDI sink for JNDI injection vulnerabilities, i.e. 1st argument to `lookup`, `lookupLink`,
+ * `doLookup`, `rename`, `list` or `listBindings` method from `InitialContext`.
+ */
+predicate jndiSinkMethod(Method m, int index) {
+  m.getDeclaringType().getAnAncestor() instanceof TypeInitialContext and
+  (
+    m.hasName("lookup") or
+    m.hasName("lookupLink") or
+    m.hasName("doLookup") or
+    m.hasName("rename") or
+    m.hasName("list") or
+    m.hasName("listBindings")
+  ) and
+  index = 0
+}
+
+/**
+ * Spring sink for JNDI injection vulnerabilities, i.e. 1st argument to `lookup` method from
+ * Spring's `JndiTemplate`.
+ */
+predicate springSinkMethod(Method m, int index) {
+  m.getDeclaringType() instanceof TypeSpringJndiTemplate and
+  m.hasName("lookup") and
+  index = 0
+}
+
+/**
+ * Apache Shiro sink for JNDI injection vulnerabilities, i.e. 1st argument to `lookup` method from
+ * Shiro's `JndiTemplate`.
+ */
+predicate shiroSinkMethod(Method m, int index) {
+  m.getDeclaringType() instanceof TypeShiroJndiTemplate and
+  m.hasName("lookup") and
+  index = 0
+}
+
+/** Holds if parameter at index `index` in method `m` is JNDI injection sink. */
+predicate jndiInjectionSinkMethod(Method m, int index) {
+  jndiSinkMethod(m, index) or
+  springSinkMethod(m, index) or
+  shiroSinkMethod(m, index)
+}
+
+/** A data flow sink for unvalidated user input that is used in JNDI lookup. */
+class JndiInjectionSink extends DataFlow::ExprNode {
+  JndiInjectionSink() {
+    exists(MethodAccess ma, Method m, int index |
+      ma.getMethod() = m and
+      ma.getArgument(index) = this.getExpr() and
+      jndiInjectionSinkMethod(m, index)
+    )
+  }
+}
+
+/**
+ * Holds if `n1` to `n2` is a dataflow step that converts between `String` and `CompositeName`,
+ * i.e. `new CompositeName(tainted)`.
+ */
+predicate compositeNameStep(ExprNode n1, ExprNode n2) {
+  exists(ConstructorCall cc | cc.getConstructedType() instanceof TypeCompositeName |
+    n1.asExpr() = cc.getAnArgument() and
+    n2.asExpr() = cc
+  )
+}

java/ql/src/experimental/qlpack.yml

@@ -0,0 +1,4 @@
+name: codeql-java-experimental
+version: 0.0.0
+libraryPathDependencies: codeql-java
+extractor: java

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

@@ -0,0 +1,11 @@
+import java
+
+/** The class `javax.naming.InitialContext`. */
+class TypeInitialContext extends Class {
+  TypeInitialContext() { this.hasQualifiedName("javax.naming", "InitialContext") }
+}
+
+/** The class `javax.naming.CompositeName`. */
+class TypeCompositeName extends Class {
+  TypeCompositeName() { this.hasQualifiedName("javax.naming", "CompositeName") }
+}

java/ql/src/experimental/semmle/code/java/frameworks/Shiro.qll

@@ -0,0 +1,6 @@
+import java
+
+/** The class `org.apache.shiro.jndi.JndiTemplate`. */
+class TypeShiroJndiTemplate extends Class {
+  TypeShiroJndiTemplate() { this.hasQualifiedName("org.apache.shiro.jndi", "JndiTemplate") }
+}

java/ql/src/experimental/semmle/code/java/frameworks/spring/SpringJndi.qll

@@ -0,0 +1,6 @@
+import java
+
+/** The class `org.springframework.jndi.JndiTemplate`. */
+class TypeSpringJndiTemplate extends Class {
+  TypeSpringJndiTemplate() { this.hasQualifiedName("org.springframework.jndi", "JndiTemplate") }
+}

java/ql/test/experimental/qlpack.yml

@@ -0,0 +1,4 @@
+name: codeql-java-experimental-tests
+version: 0.0.0
+libraryPathDependencies: codeql-java-experimental
+extractor: java