Enhance the query

Author: luchua-bc

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

@@ -15,7 +15,7 @@ public DirContext ldapAuth(String ldapUserName, String password) {
 		environment.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
 		environment.put(Context.PROVIDER_URL, ldapUrl);
 		environment.put(Context.REFERRAL, "follow");
-		env.put(Context.SECURITY_AUTHENTICATION, "simple");
+		environment.put(Context.SECURITY_AUTHENTICATION, "simple");
 		environment.put(Context.SECURITY_PRINCIPAL, ldapUserName);
 		environment.put(Context.SECURITY_CREDENTIALS, password);
 		DirContext dirContext = new InitialDirContext(environment);

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

@@ -76,7 +76,7 @@ class InsecureLdapUrl extends Expr {
  */
 predicate isProviderUrlSetter(MethodAccess ma) {
   ma.getMethod().getDeclaringType().getAnAncestor() instanceof TypeHashtable and
-  (ma.getMethod().hasName("put") or ma.getMethod().hasName("setProperty")) and
+  ma.getMethod().hasName(["put", "setProperty"]) and
   (
     ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "java.naming.provider.url"
     or
@@ -89,27 +89,48 @@ predicate isProviderUrlSetter(MethodAccess ma) {
 }
 
 /**
- * Holds if `ma` sets `fieldValue` with attribute name `fieldName` to `envValue` in some `Hashtable`.
+ * Holds if `ma` sets `fieldValue` to `envValue` in some `Hashtable`.
  */
 bindingset[fieldValue, envValue]
-predicate hasEnvWithValue(MethodAccess ma, string fieldValue, string 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") or ma.getMethod().hasName("setProperty")) 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) {
-  hasEnvWithValue(ma, "java.naming.security.authentication", "simple")
+  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) { hasEnvWithValue(ma, "java.naming.security.protocol", "ssl") }
+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.
@@ -141,12 +162,12 @@ class InsecureUrlFlowConfig extends TaintTracking::Configuration {
 /**
  * A taint-tracking configuration for `simple` basic-authentication in LDAP configuration.
  */
-class BasicAuthFlowConfig extends TaintTracking::Configuration {
+class BasicAuthFlowConfig extends DataFlow::Configuration {
   BasicAuthFlowConfig() { this = "InsecureLdapAuth:BasicAuthFlowConfig" }
 
   /** Source of `simple` configuration. */
   override predicate isSource(DataFlow::Node src) {
-    src.asExpr().(CompileTimeConstantExpr).getStringValue() = "simple"
+    exists(MethodAccess ma | isBasicAuthEnv(ma) and ma.getQualifier() = src.asExpr())
   }
 
   /** Sink of directory context creation. */
@@ -156,26 +177,17 @@ class BasicAuthFlowConfig extends TaintTracking::Configuration {
       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
-      isBasicAuthEnv(ma) and
-      succ.asExpr() = ma.getQualifier()
-    )
-  }
 }
 
 /**
  * A taint-tracking configuration for `ssl` configuration in LDAP authentication.
  */
-class SSLFlowConfig extends TaintTracking::Configuration {
+class SSLFlowConfig extends DataFlow::Configuration {
   SSLFlowConfig() { this = "InsecureLdapAuth:SSLFlowConfig" }
 
   /** Source of `ssl` configuration. */
   override predicate isSource(DataFlow::Node src) {
-    src.asExpr().(CompileTimeConstantExpr).getStringValue() = "ssl"
+    exists(MethodAccess ma | isSSLEnv(ma) and ma.getQualifier() = src.asExpr())
   }
 
   /** Sink of directory context creation. */
@@ -185,28 +197,12 @@ class SSLFlowConfig extends TaintTracking::Configuration {
       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
-      isSSLEnv(ma) and
-      succ.asExpr() = ma.getQualifier()
-    )
-  }
 }
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, InsecureUrlFlowConfig config, VarAccess va
+from DataFlow::PathNode source, DataFlow::PathNode sink, InsecureUrlFlowConfig config
 where
   config.hasFlowPath(source, sink) and
-  sink.getNode().asExpr() = va and
-  exists(BasicAuthFlowConfig bc, DataFlow::PathNode source2, DataFlow::PathNode sink2 |
-    bc.hasFlowPath(source2, sink2) and
-    sink2.getNode().asExpr() = va
-  ) and
-  not exists(SSLFlowConfig sc, DataFlow::PathNode source3, DataFlow::PathNode sink3 |
-    sc.hasFlowPath(source3, sink3) and
-    sink3.getNode().asExpr() = va
-  )
+  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

@@ -132,6 +132,7 @@ class UrlOpenConnectionMethod extends Method {
 
 /**
  * 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]