diff --git a/java/ql/src/experimental/Security/CWE/CWE-522/InsecureLdapAuth.java b/java/ql/src/experimental/Security/CWE/CWE-522/InsecureLdapAuth.java index 33764506a1b..3c5f6555100 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-522/InsecureLdapAuth.java +++ b/java/ql/src/experimental/Security/CWE/CWE-522/InsecureLdapAuth.java @@ -15,7 +15,7 @@ public class InsecureLdapAuth { 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); diff --git a/java/ql/src/experimental/Security/CWE/CWE-522/InsecureLdapAuth.ql b/java/ql/src/experimental/Security/CWE/CWE-522/InsecureLdapAuth.ql index 9563c755410..b6e7b5ed702 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-522/InsecureLdapAuth.ql +++ b/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" diff --git a/java/ql/src/semmle/code/java/frameworks/Networking.qll b/java/ql/src/semmle/code/java/frameworks/Networking.qll index 997b8075403..cad948ed2f4 100644 --- a/java/ql/src/semmle/code/java/frameworks/Networking.qll +++ b/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]