Java: Insecure LDAP authentication

java/ql/src/experimental/Security/CWE/CWE-522/InsecureBasicAuth.ql

@@ -14,14 +14,6 @@ import semmle.code.java.frameworks.ApacheHttp
 import semmle.code.java.dataflow.TaintTracking
 import DataFlow::PathGraph
 
-/**
- * Gets a regular expression for matching private hosts, which only matches the host portion therefore checking for port is not necessary.
- */
-private string getPrivateHostRegex() {
-  result =
-    "(?i)localhost(?:[:/?#].*)?|127\\.0\\.0\\.1(?:[:/?#].*)?|10(?:\\.[0-9]+){3}(?:[:/?#].*)?|172\\.16(?:\\.[0-9]+){2}(?:[:/?#].*)?|192.168(?:\\.[0-9]+){2}(?:[:/?#].*)?|\\[?0:0:0:0:0:0:0:1\\]?(?:[:/?#].*)?|\\[?::1\\]?(?:[:/?#].*)?"
-}
-
 /**
  * Class of Java URL constructor.
  */
@@ -76,7 +68,7 @@ class HttpStringLiteral extends StringLiteral {
     // Match URLs with the HTTP protocol and without private IP addresses to reduce false positives.
     exists(string s | this.getRepresentedString() = s |
       s.regexpMatch("(?i)http://[\\[a-zA-Z0-9].*") and
-      not s.substring(7, s.length()).regexpMatch(getPrivateHostRegex())
+      not s.substring(7, s.length()) instanceof PrivateHostName
     )
   }
 }
@@ -101,7 +93,7 @@ predicate concatHttpString(Expr protocol, Expr host) {
       host.(VarAccess).getVariable().getAnAssignedValue().(CompileTimeConstantExpr).getStringValue()
   |
     hostString.length() = 0 or // Empty host is loopback address
-    hostString.regexpMatch(getPrivateHostRegex())
+    hostString instanceof PrivateHostName
   )
 }
 

java/ql/src/experimental/Security/CWE/CWE-522/InsecureLdapAuth.java

@@ -0,0 +1,24 @@
+public class InsecureLdapAuth {
+	/** LDAP authentication */	
+	public DirContext ldapAuth(String ldapUserName, String password) {
+		{
+			// BAD: LDAP authentication in cleartext
+			String ldapUrl = "ldap://ad.your-server.com:389";
+		}
+
+		{
+			// GOOD: LDAPS authentication over SSL
+			String ldapUrl = "ldaps://ad.your-server.com:636";
+		}
+
+		Hashtable<String, String> environment = new Hashtable<String, String>();
+		environment.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
+		environment.put(Context.PROVIDER_URL, ldapUrl);
+		environment.put(Context.REFERRAL, "follow");
+		environment.put(Context.SECURITY_AUTHENTICATION, "simple");
+		environment.put(Context.SECURITY_PRINCIPAL, ldapUserName);
+		environment.put(Context.SECURITY_CREDENTIALS, password);
+		DirContext dirContext = new InitialDirContext(environment);
+		return dirContext;
+	}
+}

java/ql/src/experimental/Security/CWE/CWE-522/InsecureLdapAuth.qhelp

@@ -0,0 +1,27 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+
+  <overview>
+    <p>When using the Java LDAP API to perform LDAPv3-style extended operations and controls, a context with connection properties including user credentials is started. Transmission of LDAP credentials in cleartext allows remote attackers to obtain sensitive information by sniffing the network.</p>
+  </overview>
+
+  <recommendation>
+    <p>Use LDAPS to send credentials through SSL or use SASL authentication.</p>
+  </recommendation>
+
+  <example>
+    <p>The following example shows two ways of using LDAP authentication. In the 'BAD' case, the credentials are transmitted in cleartext. In the 'GOOD' case, the credentials are transmitted over SSL.</p>
+    <sample src="InsecureLdapAuth.java" />
+  </example>
+
+  <references>
+    <li>
+      Oracle:
+      <a href="https://docs.oracle.com/javase/jndi/tutorial/ldap/misc/url.html">LDAP and LDAPS URLs</a>
+    </li>
+    <li>
+      Oracle:
+      <a href="https://docs.oracle.com/javase/tutorial/jndi/ldap/simple.html">Simple authentication</a>
+    </li>
+  </references>
+</qhelp>

java/ql/src/experimental/Security/CWE/CWE-522/InsecureLdapAuth.ql

@@ -0,0 +1,213 @@
+/**
+ * @name Insecure LDAP authentication
+ * @description LDAP authentication with credentials sent in cleartext.
+ * @kind path-problem
+ * @id java/insecure-ldap-auth
+ * @tags security
+ *       external/cwe-522
+ *       external/cwe-319
+ */
+
+import java
+import DataFlow
+import semmle.code.java.frameworks.Jndi
+import semmle.code.java.frameworks.Networking
+import semmle.code.java.dataflow.TaintTracking
+import DataFlow::PathGraph
+
+/**
+ * Insecure (non-SSL, non-private) LDAP URL string literal.
+ */
+class InsecureLdapUrlLiteral extends StringLiteral {
+  InsecureLdapUrlLiteral() {
+    // Match connection strings with the LDAP protocol and without private IP addresses to reduce false positives.
+    exists(string s | this.getRepresentedString() = s |
+      s.regexpMatch("(?i)ldap://[\\[a-zA-Z0-9].*") and
+      not s.substring(7, s.length()) instanceof PrivateHostName
+    )
+  }
+}
+
+/** The interface `javax.naming.Context`. */
+class TypeNamingContext extends Interface {
+  TypeNamingContext() { this.hasQualifiedName("javax.naming", "Context") }
+}
+
+/** The class `java.util.Hashtable`. */
+class TypeHashtable extends Class {
+  TypeHashtable() { this.getSourceDeclaration().hasQualifiedName("java.util", "Hashtable") }
+}
+
+/**
+ * Holds if a non-private LDAP string is concatenated from both protocol and host.
+ */
+predicate concatInsecureLdapString(Expr protocol, Expr host) {
+  protocol.(CompileTimeConstantExpr).getStringValue() = "ldap://" and
+  not exists(string hostString |
+    hostString = host.(CompileTimeConstantExpr).getStringValue() or
+    hostString =
+      host.(VarAccess).getVariable().getAnAssignedValue().(CompileTimeConstantExpr).getStringValue()
+  |
+    hostString.length() = 0 or // Empty host is loopback address
+    hostString instanceof PrivateHostName
+  )
+}
+
+/** Gets the leftmost operand in a concatenated string */
+Expr getLeftmostConcatOperand(Expr expr) {
+  if expr instanceof AddExpr
+  then result = getLeftmostConcatOperand(expr.(AddExpr).getLeftOperand())
+  else result = expr
+}
+
+/**
+ * String concatenated with `InsecureLdapUrlLiteral`.
+ */
+class InsecureLdapUrl extends Expr {
+  InsecureLdapUrl() {
+    this instanceof InsecureLdapUrlLiteral
+    or
+    concatInsecureLdapString(this.(AddExpr).getLeftOperand(),
+      getLeftmostConcatOperand(this.(AddExpr).getRightOperand()))
+  }
+}
+
+/**
+ * Holds if `ma` writes the `java.naming.provider.url` (also known as `Context.PROVIDER_URL`) key of a `Hashtable`.
+ */
+predicate isProviderUrlSetter(MethodAccess ma) {
+  ma.getMethod().getDeclaringType().getAnAncestor() instanceof TypeHashtable and
+  ma.getMethod().hasName(["put", "setProperty"]) and
+  (
+    ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "java.naming.provider.url"
+    or
+    exists(Field f |
+      ma.getArgument(0) = f.getAnAccess() and
+      f.hasName("PROVIDER_URL") and
+      f.getDeclaringType() instanceof TypeNamingContext
+    )
+  )
+}
+
+/**
+ * Holds if `ma` sets `fieldValue` to `envValue` in some `Hashtable`.
+ */
+bindingset[fieldValue, envValue]
+predicate hasFieldValueEnv(MethodAccess ma, string fieldValue, string envValue) {
+  // environment.put("java.naming.security.authentication", "simple")
+  ma.getMethod().getDeclaringType().getAnAncestor() instanceof TypeHashtable and
+  ma.getMethod().hasName(["put", "setProperty"]) and
+  ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = fieldValue and
+  ma.getArgument(1).(CompileTimeConstantExpr).getStringValue() = envValue
+}
+
+/**
+ * Holds if `ma` sets attribute name `fieldName` to `envValue` in some `Hashtable`.
+ */
+bindingset[fieldName, envValue]
+predicate hasFieldNameEnv(MethodAccess ma, string fieldName, string envValue) {
+  // environment.put(Context.SECURITY_AUTHENTICATION, "simple")
+  ma.getMethod().getDeclaringType().getAnAncestor() instanceof TypeHashtable and
+  ma.getMethod().hasName(["put", "setProperty"]) and
+  exists(Field f |
+    ma.getArgument(0) = f.getAnAccess() and
+    f.hasName(fieldName) and
+    f.getDeclaringType() instanceof TypeNamingContext
+  ) and
+  ma.getArgument(1).(CompileTimeConstantExpr).getStringValue() = envValue
+}
+
+/**
+ * Holds if `ma` sets `java.naming.security.authentication` (also known as `Context.SECURITY_AUTHENTICATION`) to `simple` in some `Hashtable`.
+ */
+predicate isBasicAuthEnv(MethodAccess ma) {
+  hasFieldValueEnv(ma, "java.naming.security.authentication", "simple") or
+  hasFieldNameEnv(ma, "SECURITY_AUTHENTICATION", "simple")
+}
+
+/**
+ * Holds if `ma` sets `java.naming.security.protocol` (also known as `Context.SECURITY_PROTOCOL`) to `ssl` in some `Hashtable`.
+ */
+predicate isSSLEnv(MethodAccess ma) {
+  hasFieldValueEnv(ma, "java.naming.security.protocol", "ssl") or
+  hasFieldNameEnv(ma, "SECURITY_PROTOCOL", "ssl")
+}
+
+/**
+ * A taint-tracking configuration for `ldap://` URL in LDAP authentication.
+ */
+class InsecureUrlFlowConfig extends TaintTracking::Configuration {
+  InsecureUrlFlowConfig() { this = "InsecureLdapAuth:InsecureUrlFlowConfig" }
+
+  /** Source of `ldap://` connection string. */
+  override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof InsecureLdapUrl }
+
+  /** Sink of directory context creation. */
+  override predicate isSink(DataFlow::Node sink) {
+    exists(ConstructorCall cc |
+      cc.getConstructedType().getASupertype*() instanceof TypeDirContext and
+      sink.asExpr() = cc.getArgument(0)
+    )
+  }
+
+  /** Method call of `env.put()`. */
+  override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(MethodAccess ma |
+      pred.asExpr() = ma.getArgument(1) and
+      isProviderUrlSetter(ma) and
+      succ.asExpr() = ma.getQualifier()
+    )
+  }
+}
+
+/**
+ * A taint-tracking configuration for `simple` basic-authentication in LDAP configuration.
+ */
+class BasicAuthFlowConfig extends DataFlow::Configuration {
+  BasicAuthFlowConfig() { this = "InsecureLdapAuth:BasicAuthFlowConfig" }
+
+  /** Source of `simple` configuration. */
+  override predicate isSource(DataFlow::Node src) {
+    exists(MethodAccess ma |
+      isBasicAuthEnv(ma) and ma.getQualifier() = src.(PostUpdateNode).getPreUpdateNode().asExpr()
+    )
+  }
+
+  /** Sink of directory context creation. */
+  override predicate isSink(DataFlow::Node sink) {
+    exists(ConstructorCall cc |
+      cc.getConstructedType().getASupertype*() instanceof TypeDirContext and
+      sink.asExpr() = cc.getArgument(0)
+    )
+  }
+}
+
+/**
+ * A taint-tracking configuration for `ssl` configuration in LDAP authentication.
+ */
+class SSLFlowConfig extends DataFlow::Configuration {
+  SSLFlowConfig() { this = "InsecureLdapAuth:SSLFlowConfig" }
+
+  /** Source of `ssl` configuration. */
+  override predicate isSource(DataFlow::Node src) {
+    exists(MethodAccess ma |
+      isSSLEnv(ma) and ma.getQualifier() = src.(PostUpdateNode).getPreUpdateNode().asExpr()
+    )
+  }
+
+  /** Sink of directory context creation. */
+  override predicate isSink(DataFlow::Node sink) {
+    exists(ConstructorCall cc |
+      cc.getConstructedType().getASupertype*() instanceof TypeDirContext and
+      sink.asExpr() = cc.getArgument(0)
+    )
+  }
+}
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, InsecureUrlFlowConfig config
+where
+  config.hasFlowPath(source, sink) and
+  exists(BasicAuthFlowConfig bc | bc.hasFlowTo(sink.getNode())) and
+  not exists(SSLFlowConfig sc | sc.hasFlowTo(sink.getNode()))
+select sink.getNode(), source, sink, "Insecure LDAP authentication from $@.", source.getNode(),
+  "LDAP connection string"

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

@@ -129,3 +129,14 @@ class UrlOpenConnectionMethod extends Method {
     this.getName() = "openConnection"
   }
 }
+
+/**
+ * A string matching private host names of IPv4 and IPv6, which only matches the host portion therefore checking for port is not necessary.
+ * Several examples are localhost, reserved IPv4 IP addresses including 127.0.0.1, 10.x.x.x, 172.16.x,x, 192.168.x,x, and reserved IPv6 addresses including [0:0:0:0:0:0:0:1] and [::1]
+ */
+class PrivateHostName extends string {
+  bindingset[this]
+  PrivateHostName() {
+    this.regexpMatch("(?i)localhost(?:[:/?#].*)?|127\\.0\\.0\\.1(?:[:/?#].*)?|10(?:\\.[0-9]+){3}(?:[:/?#].*)?|172\\.16(?:\\.[0-9]+){2}(?:[:/?#].*)?|192.168(?:\\.[0-9]+){2}(?:[:/?#].*)?|\\[?0:0:0:0:0:0:0:1\\]?(?:[:/?#].*)?|\\[?::1\\]?(?:[:/?#].*)?")
+  }
+}