Skip to content

Commit 5ce3af0

Browse files
luchua-bcluchua-bc
committed
Enhance the query and update qldoc
1 parent 23f620d commit 5ce3af0

File tree

6 files changed

+146
-40
lines changed

6 files changed

+146
-40
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-297/InsecureLdapEndpoint.java

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,11 @@ public Hashtable<String, String> createConnectionEnv() {
88
env.put(Context.SECURITY_PRINCIPAL, "username");
99
env.put(Context.SECURITY_CREDENTIALS, "secpassword");
1010

11-
// BAD - Test configuration with disabled SSL endpoint check.
12-
{
13-
System.setProperty("com.sun.jndi.ldap.object.disableEndpointIdentification", "true");
14-
}
15-
16-
// GOOD - No configuration to disable SSL endpoint check since it is enabled by default.
17-
{
18-
}
11+
// BAD - Test configuration with disabled SSL endpoint check.
12+
{
13+
System.setProperty("com.sun.jndi.ldap.object.disableEndpointIdentification", "true");
14+
}
1915

2016
return env;
2117
}
22-
2318
}

‎java/ql/src/experimental/Security/CWE/CWE-297/InsecureLdapEndpoint.qhelp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,19 @@
55

66
<overview>
77
<p>Java versions 8u181 or greater have enabled LDAPS endpoint identification by default. Nowadays infrastructure services like LDAP are commonly deployed behind load balancers therefore the LDAP server name can be different from the FQDN of the LDAPS endpoint. If a service certificate does not properly contain a matching DNS name as part of the certificate, Java will reject it by default.</p>
8-
<p>Instead of addressing the issue properly by having a compliant certificate deployed, frequently developers simply disable SSL endpoint check.</p>
9-
<p>This query checks whether LDAPS endpoint check is disabled in system properties.</p>
8+
<p>Instead of addressing the issue properly by having a compliant certificate deployed, frequently developers simply disable LDAPS endpoint check.</p>
9+
<p>Failing to validate the certificate makes the SSL session susceptible to a man-in-the-middle attack. This query checks whether LDAPS endpoint check is disabled in system properties.</p>
1010
</overview>
1111

1212
<recommendation>
1313
<p>Replace any non-conforming LDAP server certificates to include a DNS name in the subjectAltName field of the certificate that matches the FQDN of the service.</p>
1414
</recommendation>
1515

1616
<example>
17-
<p>The following two examples show two ways of configuring SSL endpoint. In the 'BAD' case,
17+
<p>The following two examples show two ways of configuring LDAPS endpoint. In the 'BAD' case,
1818
endpoint check is disabled. In the 'GOOD' case, endpoint check is left enabled through the default Java configuration.</p>
19-
<sample src="InsecureSSLEndpoint.java" />
19+
<sample src="InsecureLdapEndpoint.java" />
20+
<sample src="InsecureLdapEndpoint2.java" />>
2021
</example>
2122

2223
<references>

‎java/ql/src/experimental/Security/CWE/CWE-297/InsecureLdapEndpoint.ql

Lines changed: 58 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,65 +1,101 @@
11
/**
2-
* @name Insecure LDAP Endpoint Configuration
3-
* @description Java application configured to disable LDAP endpoint identification does not validate the SSL certificate to properly ensure that it is actually associated with that host.
2+
* @name Insecure LDAPS Endpoint Configuration
3+
* @description Java application configured to disable LDAPS endpoint identification does not validate the SSL certificate to properly ensure that it is actually associated with that host.
44
* @kind problem
5-
* @id java/insecure-ldap-endpoint
5+
* @id java/insecure-ldaps-endpoint
66
* @tags security
77
* external/cwe-297
88
*/
99

1010
import java
1111

12-
/**
13-
* The method to set a system property.
14-
*/
12+
/** The method to set a system property. */
1513
class SetSystemPropertyMethod extends Method {
1614
SetSystemPropertyMethod() {
1715
this.hasName("setProperty") and
1816
this.getDeclaringType().hasQualifiedName("java.lang", "System")
1917
}
2018
}
2119

20+
/** The class `java.util.Hashtable`. */
21+
class TypeHashtable extends Class {
22+
TypeHashtable() { this.getSourceDeclaration().hasQualifiedName("java.util", "Hashtable") }
23+
}
24+
2225
/**
23-
* The method to set Java properties.
26+
* The method to set Java properties either through `setProperty` declared in the class `Properties` or `put` declared in its parent class `HashTable`.
2427
*/
2528
class SetPropertyMethod extends Method {
2629
SetPropertyMethod() {
27-
this.hasName("setProperty") and
28-
this.getDeclaringType().hasQualifiedName("java.util", "Properties")
30+
this.getDeclaringType().getAnAncestor() instanceof TypeHashtable and
31+
this.hasName(["put", "setProperty"])
2932
}
3033
}
3134

32-
/**
33-
* The method to set system properties.
34-
*/
35+
/** The method to set system properties. */
3536
class SetSystemPropertiesMethod extends Method {
3637
SetSystemPropertiesMethod() {
3738
this.hasName("setProperties") and
3839
this.getDeclaringType().hasQualifiedName("java.lang", "System")
3940
}
4041
}
4142

43+
/** Holds if an expression is evaluated to the string literal `com.sun.jndi.ldap.object.disableEndpointIdentification`. */
44+
predicate isPropertyDisableLdapEndpointId(Expr expr) {
45+
expr.(CompileTimeConstantExpr).getStringValue() =
46+
"com.sun.jndi.ldap.object.disableEndpointIdentification"
47+
or
48+
exists(Field f |
49+
expr = f.getAnAccess() and
50+
f.getAnAssignedValue().(StringLiteral).getValue() =
51+
"com.sun.jndi.ldap.object.disableEndpointIdentification"
52+
)
53+
}
54+
55+
/** Holds if an expression is evaluated to the boolean value true. */
56+
predicate isBooleanTrue(Expr expr) {
57+
expr.(CompileTimeConstantExpr).getStringValue() = "true" // "true"
58+
or
59+
expr.(BooleanLiteral).getBooleanValue() = true // true
60+
or
61+
exists(MethodAccess ma |
62+
expr = ma and
63+
ma.getMethod().hasName("toString") and
64+
ma.getQualifier().(FieldAccess).getField().hasName("TRUE") and
65+
ma.getQualifier()
66+
.(FieldAccess)
67+
.getField()
68+
.getDeclaringType()
69+
.hasQualifiedName("java.lang", "Boolean") // Boolean.TRUE.toString()
70+
)
71+
}
72+
73+
/** Holds if `ma` is in a test class or method. */
74+
predicate isTestMethod(MethodAccess ma) {
75+
ma.getMethod() instanceof TestMethod or
76+
ma.getEnclosingCallable().getDeclaringType().getPackage().getName().matches("%test%") or
77+
ma.getEnclosingCallable().getDeclaringType().getName().toLowerCase().matches("%test%")
78+
}
79+
4280
/** Holds if `MethodAccess` ma disables SSL endpoint check. */
4381
predicate isInsecureSSLEndpoint(MethodAccess ma) {
4482
(
4583
ma.getMethod() instanceof SetSystemPropertyMethod and
46-
(
47-
ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() =
48-
"com.sun.jndi.ldap.object.disableEndpointIdentification" and
49-
ma.getArgument(1).(CompileTimeConstantExpr).getStringValue() = "true" //com.sun.jndi.ldap.object.disableEndpointIdentification=true
50-
)
84+
isPropertyDisableLdapEndpointId(ma.getArgument(0)) and
85+
isBooleanTrue(ma.getArgument(1)) //com.sun.jndi.ldap.object.disableEndpointIdentification=true
5186
or
5287
ma.getMethod() instanceof SetSystemPropertiesMethod and
5388
exists(MethodAccess ma2 |
5489
ma2.getMethod() instanceof SetPropertyMethod and
55-
ma2.getArgument(0).(CompileTimeConstantExpr).getStringValue() =
56-
"com.sun.jndi.ldap.object.disableEndpointIdentification" and
57-
ma2.getArgument(1).(CompileTimeConstantExpr).getStringValue() = "true" and //com.sun.jndi.ldap.object.disableEndpointIdentification=true
90+
isPropertyDisableLdapEndpointId(ma2.getArgument(0)) and
91+
isBooleanTrue(ma2.getArgument(1)) and //com.sun.jndi.ldap.object.disableEndpointIdentification=true
5892
ma2.getQualifier().(VarAccess).getVariable().getAnAccess() = ma.getArgument(0) // systemProps.setProperties(properties)
5993
)
6094
)
6195
}
6296

6397
from MethodAccess ma
64-
where isInsecureSSLEndpoint(ma)
65-
select ma, "SSL configuration allows insecure endpoint configuration"
98+
where
99+
isInsecureSSLEndpoint(ma) and
100+
not isTestMethod(ma)
101+
select ma, "LDAPS configuration allows insecure endpoint identification"

‎java/ql/src/experimental/Security/CWE/CWE-297/InsecureLdapEndpoint2.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
public class InsecureLdapEndpoint2 {
2+
public Hashtable<String, String> createConnectionEnv() {
3+
Hashtable<String, String> env = new Hashtable<String, String>();
4+
env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
5+
env.put(Context.PROVIDER_URL, "ldaps://ad.your-server.com:636");
6+
7+
env.put(Context.SECURITY_AUTHENTICATION, "simple");
8+
env.put(Context.SECURITY_PRINCIPAL, "username");
9+
env.put(Context.SECURITY_CREDENTIALS, "secpassword");
10+
11+
// GOOD - No configuration to disable SSL endpoint check since it is enabled by default.
12+
{
13+
}
14+
15+
return env;
16+
}
17+
}

‎java/ql/test/experimental/query-tests/security/CWE-297/InsecureLdapEndpoint.expected

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,5 @@
1-
| InsecureLdapEndpoint.java:17:9:17:92 | setProperty(...) | SSL configuration allows insecure endpoint configuration |
2-
| InsecureLdapEndpoint.java:48:3:48:34 | setProperties(...) | SSL configuration allows insecure endpoint configuration |
1+
| InsecureLdapEndpoint.java:19:9:19:92 | setProperty(...) | LDAPS configuration allows insecure endpoint identification |
2+
| InsecureLdapEndpoint.java:50:9:50:40 | setProperties(...) | LDAPS configuration allows insecure endpoint identification |
3+
| InsecureLdapEndpoint.java:68:9:68:40 | setProperties(...) | LDAPS configuration allows insecure endpoint identification |
4+
| InsecureLdapEndpoint.java:84:9:84:94 | setProperty(...) | LDAPS configuration allows insecure endpoint identification |
5+
| InsecureLdapEndpoint.java:102:9:102:40 | setProperties(...) | LDAPS configuration allows insecure endpoint identification |

‎java/ql/test/experimental/query-tests/security/CWE-297/InsecureLdapEndpoint.java

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
import javax.naming.Context;
44

55
public class InsecureLdapEndpoint {
6-
// BAD - Test configuration with disabled SSL endpoint check.
6+
private static String PROP_DISABLE_LDAP_ENDPOINT_IDENTIFICATION = "com.sun.jndi.ldap.object.disableEndpointIdentification";
7+
8+
// BAD - Test configuration with disabled LDAPS endpoint check using `System.setProperty()`.
79
public Hashtable<String, String> createConnectionEnv() {
810
Hashtable<String, String> env = new Hashtable<String, String>();
911
env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
@@ -19,7 +21,7 @@ public Hashtable<String, String> createConnectionEnv() {
1921
return env;
2022
}
2123

22-
// GOOD - Test configuration without disabling SSL endpoint check.
24+
// GOOD - Test configuration without disabling LDAPS endpoint check.
2325
public Hashtable<String, String> createConnectionEnv2() {
2426
Hashtable<String, String> env = new Hashtable<String, String>();
2527
env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
@@ -32,7 +34,7 @@ public Hashtable<String, String> createConnectionEnv2() {
3234
return env;
3335
}
3436

35-
// BAD - Test configuration with disabled SSL endpoint check.
37+
// BAD - Test configuration with disabled LDAPS endpoint check using `System.setProperties()`.
3638
public Hashtable<String, String> createConnectionEnv3() {
3739
Hashtable<String, String> env = new Hashtable<String, String>();
3840
env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
@@ -49,4 +51,56 @@ public Hashtable<String, String> createConnectionEnv3() {
4951

5052
return env;
5153
}
54+
55+
// BAD - Test configuration with disabled LDAPS endpoint check using `HashTable.put()`.
56+
public Hashtable<String, String> createConnectionEnv4() {
57+
Hashtable<String, String> env = new Hashtable<String, String>();
58+
env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
59+
env.put(Context.PROVIDER_URL, "ldaps://ad.your-server.com:636");
60+
61+
env.put(Context.SECURITY_AUTHENTICATION, "simple");
62+
env.put(Context.SECURITY_PRINCIPAL, "username");
63+
env.put(Context.SECURITY_CREDENTIALS, "secpassword");
64+
65+
// Disable SSL endpoint check
66+
Properties properties = new Properties();
67+
properties.put("com.sun.jndi.ldap.object.disableEndpointIdentification", "true");
68+
System.setProperties(properties);
69+
70+
return env;
71+
}
72+
73+
// BAD - Test configuration with disabled LDAPS endpoint check using the `TRUE` boolean field.
74+
public Hashtable<String, String> createConnectionEnv5() {
75+
Hashtable<String, String> env = new Hashtable<String, String>();
76+
env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
77+
env.put(Context.PROVIDER_URL, "ldaps://ad.your-server.com:636");
78+
79+
env.put(Context.SECURITY_AUTHENTICATION, "simple");
80+
env.put(Context.SECURITY_PRINCIPAL, "username");
81+
env.put(Context.SECURITY_CREDENTIALS, "secpassword");
82+
83+
// Disable SSL endpoint check
84+
System.setProperty(PROP_DISABLE_LDAP_ENDPOINT_IDENTIFICATION, Boolean.TRUE.toString());
85+
86+
return env;
87+
}
88+
89+
// BAD - Test configuration with disabled LDAPS endpoint check using a boolean value.
90+
public Hashtable<String, String> createConnectionEnv6() {
91+
Hashtable<String, String> env = new Hashtable<String, String>();
92+
env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
93+
env.put(Context.PROVIDER_URL, "ldaps://ad.your-server.com:636");
94+
95+
env.put(Context.SECURITY_AUTHENTICATION, "simple");
96+
env.put(Context.SECURITY_PRINCIPAL, "username");
97+
env.put(Context.SECURITY_CREDENTIALS, "secpassword");
98+
99+
// Disable SSL endpoint check
100+
Properties properties = new Properties();
101+
properties.put("com.sun.jndi.ldap.object.disableEndpointIdentification", true);
102+
System.setProperties(properties);
103+
104+
return env;
105+
}
52106
}

0 commit comments

Comments
 (0)