Skip to content

Commit b00104e

Browse files
egregius313egregius313
authored
Merge pull request github#12458 from egregius313/egregius313/promote-insecure-ldap-authentication
Java: Promote LDAP Authentication Query
2 parents 451f6f0 + 97ec808 commit b00104e

File tree

16 files changed

+330
-390
lines changed

16 files changed

+330
-390
lines changed

‎java/ql/lib/semmle/code/java/security/InsecureLdapAuth.qll

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
/** Provides classes to reason about insecure LDAP authentication. */
2+
3+
import java
4+
private import semmle.code.java.dataflow.DataFlow
5+
private import semmle.code.java.frameworks.Networking
6+
private import semmle.code.java.frameworks.Jndi
7+
8+
/**
9+
* An expression that represents an insecure (non-SSL, non-private) LDAP URL.
10+
*/
11+
class InsecureLdapUrl extends Expr {
12+
InsecureLdapUrl() {
13+
this instanceof InsecureLdapUrlLiteral
14+
or
15+
// Concatentation of insecure protcol and non-private host:
16+
// protocol + host + ...
17+
exists(AddExpr e, CompileTimeConstantExpr protocol, Expr rest, Expr host |
18+
e = this and
19+
protocol = e.getLeftOperand() and
20+
rest = e.getRightOperand() and
21+
if rest instanceof AddExpr then host = rest.(AddExpr).getLeftOperand() else host = rest
22+
|
23+
protocol.getStringValue() = "ldap://" and
24+
not exists(string hostString | hostString = getHostname(host) |
25+
hostString.length() = 0 or // Empty host is loopback address
26+
hostString instanceof PrivateHostName
27+
)
28+
)
29+
}
30+
}
31+
32+
/**
33+
* A sink representing the construction of a `DirContextEnvironment`.
34+
*/
35+
class InsecureLdapUrlSink extends DataFlow::Node {
36+
InsecureLdapUrlSink() {
37+
exists(ConstructorCall cc |
38+
cc.getConstructedType().getAnAncestor() instanceof TypeDirContext and
39+
this.asExpr() = cc.getArgument(0)
40+
)
41+
}
42+
}
43+
44+
/**
45+
* Holds if `ma` sets `java.naming.security.authentication` (also known as `Context.SECURITY_AUTHENTICATION`) to `simple` in some `Hashtable`.
46+
*/
47+
predicate isBasicAuthEnv(MethodAccess ma) {
48+
hasFieldValueEnv(ma, "java.naming.security.authentication", "simple") or
49+
hasFieldNameEnv(ma, "SECURITY_AUTHENTICATION", "simple")
50+
}
51+
52+
/**
53+
* Holds if `ma` sets `java.naming.security.protocol` (also known as `Context.SECURITY_PROTOCOL`) to `ssl` in some `Hashtable`.
54+
*/
55+
predicate isSslEnv(MethodAccess ma) {
56+
hasFieldValueEnv(ma, "java.naming.security.protocol", "ssl") or
57+
hasFieldNameEnv(ma, "SECURITY_PROTOCOL", "ssl")
58+
}
59+
60+
/**
61+
* Holds if `ma` writes the `java.naming.provider.url` (also known as `Context.PROVIDER_URL`) key of a `Hashtable`.
62+
*/
63+
predicate isProviderUrlSetter(MethodAccess ma) {
64+
ma.getMethod().getDeclaringType().getAnAncestor() instanceof TypeHashtable and
65+
ma.getMethod().hasName(["put", "setProperty"]) and
66+
(
67+
ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "java.naming.provider.url"
68+
or
69+
exists(Field f |
70+
ma.getArgument(0) = f.getAnAccess() and
71+
f.hasName("PROVIDER_URL") and
72+
f.getDeclaringType() instanceof TypeNamingContext
73+
)
74+
)
75+
}
76+
77+
/**
78+
* An insecure (non-SSL, non-private) LDAP URL string literal.
79+
*/
80+
private class InsecureLdapUrlLiteral extends StringLiteral {
81+
InsecureLdapUrlLiteral() {
82+
// Match connection strings with the LDAP protocol and without private IP addresses to reduce false positives.
83+
exists(string s | this.getValue() = s |
84+
s.regexpMatch("(?i)ldap://[\\[a-zA-Z0-9].*") and
85+
not s.substring(7, s.length()) instanceof PrivateHostName
86+
)
87+
}
88+
}
89+
90+
/** The class `java.util.Hashtable`. */
91+
private class TypeHashtable extends Class {
92+
TypeHashtable() { this.getSourceDeclaration().hasQualifiedName("java.util", "Hashtable") }
93+
}
94+
95+
/** Get the string value of an expression representing a hostname. */
96+
private string getHostname(Expr expr) {
97+
result = expr.(CompileTimeConstantExpr).getStringValue() or
98+
result =
99+
expr.(VarAccess).getVariable().getAnAssignedValue().(CompileTimeConstantExpr).getStringValue()
100+
}
101+
102+
/**
103+
* Holds if `ma` sets `fieldValue` to `envValue` in some `Hashtable`.
104+
*/
105+
bindingset[fieldValue, envValue]
106+
private predicate hasFieldValueEnv(MethodAccess ma, string fieldValue, string envValue) {
107+
// environment.put("java.naming.security.authentication", "simple")
108+
ma.getMethod().getDeclaringType().getAnAncestor() instanceof TypeHashtable and
109+
ma.getMethod().hasName(["put", "setProperty"]) and
110+
ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = fieldValue and
111+
ma.getArgument(1).(CompileTimeConstantExpr).getStringValue() = envValue
112+
}
113+
114+
/**
115+
* Holds if `ma` sets attribute name `fieldName` to `envValue` in some `Hashtable`.
116+
*/
117+
bindingset[fieldName, envValue]
118+
private predicate hasFieldNameEnv(MethodAccess ma, string fieldName, string envValue) {
119+
// environment.put(Context.SECURITY_AUTHENTICATION, "simple")
120+
ma.getMethod().getDeclaringType().getAnAncestor() instanceof TypeHashtable and
121+
ma.getMethod().hasName(["put", "setProperty"]) and
122+
exists(Field f |
123+
ma.getArgument(0) = f.getAnAccess() and
124+
f.hasName(fieldName) and
125+
f.getDeclaringType() instanceof TypeNamingContext
126+
) and
127+
ma.getArgument(1).(CompileTimeConstantExpr).getStringValue() = envValue
128+
}

‎java/ql/lib/semmle/code/java/security/InsecureLdapAuthQuery.qll

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/** Provides dataflow configurations to reason about insecure LDAP authentication. */
2+
3+
import java
4+
import semmle.code.java.dataflow.DataFlow
5+
import semmle.code.java.dataflow.TaintTracking
6+
import semmle.code.java.frameworks.Jndi
7+
import semmle.code.java.security.InsecureLdapAuth
8+
9+
/**
10+
* A taint-tracking configuration for `ldap://` URL in LDAP authentication.
11+
*/
12+
module InsecureLdapUrlConfig implements DataFlow::ConfigSig {
13+
predicate isSource(DataFlow::Node src) { src.asExpr() instanceof InsecureLdapUrl }
14+
15+
predicate isSink(DataFlow::Node sink) { sink instanceof InsecureLdapUrlSink }
16+
17+
/** Method call of `env.put()`. */
18+
predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
19+
exists(MethodAccess ma |
20+
pred.asExpr() = ma.getArgument(1) and
21+
isProviderUrlSetter(ma) and
22+
succ.asExpr() = ma.getQualifier()
23+
)
24+
}
25+
}
26+
27+
module InsecureLdapUrlFlow = TaintTracking::Global<InsecureLdapUrlConfig>;
28+
29+
/**
30+
* A taint-tracking configuration for `simple` basic-authentication in LDAP configuration.
31+
*/
32+
private module BasicAuthConfig implements DataFlow::ConfigSig {
33+
predicate isSource(DataFlow::Node src) {
34+
exists(MethodAccess ma |
35+
isBasicAuthEnv(ma) and
36+
ma.getQualifier() = src.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr()
37+
)
38+
}
39+
40+
predicate isSink(DataFlow::Node sink) { sink instanceof InsecureLdapUrlSink }
41+
}
42+
43+
module BasicAuthFlow = DataFlow::Global<BasicAuthConfig>;
44+
45+
/**
46+
* A taint-tracking configuration for `ssl` configuration in LDAP authentication.
47+
*/
48+
private module RequiresSslConfig implements DataFlow::ConfigSig {
49+
predicate isSource(DataFlow::Node src) {
50+
exists(MethodAccess ma |
51+
isSslEnv(ma) and
52+
ma.getQualifier() = src.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr()
53+
)
54+
}
55+
56+
predicate isSink(DataFlow::Node sink) { sink instanceof InsecureLdapUrlSink }
57+
}
58+
59+
module RequiresSslFlow = DataFlow::Global<RequiresSslConfig>;

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>
6+
When using the Java LDAP API to perform LDAPv3-style extended operations
7+
and controls, a context with connection properties including user
8+
credentials is started. Transmission of LDAP credentials in cleartext
9+
allows remote attackers to obtain sensitive information by sniffing the
10+
network.
11+
</p>
12+
</overview>
13+
14+
<recommendation>
15+
<p>
16+
Use the <code>ldaps://</code> protocol to send credentials through SSL or
17+
use SASL authentication.
18+
</p>
19+
</recommendation>
20+
21+
<example>
22+
<p>
23+
In the following (bad) example, a <code>ldap://</code> URL is used and
24+
credentials will be sent in plaintext.
25+
</p>
26+
<sample src="LdapAuthUseLdap.java"/>
27+
28+
<p>
29+
In the following (good) example, a <code>ldaps://</code> URL is used so
30+
credentials will be encrypted with SSL.
31+
</p>
32+
<sample src="LdapAuthUseLdaps.java"/>
33+
34+
<p>
35+
In the following (good) example, a <code>ldap://</code> URL is used, but
36+
SASL authentication is enabled so that the credentials will be encrypted.
37+
</p>
38+
<sample src="LdapEnableSasl.java"/>
39+
</example>
40+
41+
<references>
42+
<li>
43+
Oracle:
44+
<a href="https://docs.oracle.com/javase/jndi/tutorial/ldap/misc/url.html">LDAP and LDAPS URLs</a>
45+
</li>
46+
<li>
47+
Oracle:
48+
<a href="https://docs.oracle.com/javase/tutorial/jndi/ldap/simple.html">Simple authentication</a>
49+
</li>
50+
</references>
51+
</qhelp>

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/**
2+
* @name Insecure LDAP authentication
3+
* @description LDAP authentication with credentials sent in cleartext makes sensitive information vulnerable to remote attackers
4+
* @kind path-problem
5+
* @problem.severity warning
6+
* @security-severity 8.8
7+
* @precision medium
8+
* @id java/insecure-ldap-auth
9+
* @tags security
10+
* experimental
11+
* external/cwe/cwe-522
12+
* external/cwe/cwe-319
13+
*/
14+
15+
import java
16+
import semmle.code.java.security.InsecureLdapAuthQuery
17+
import InsecureLdapUrlFlow::PathGraph
18+
19+
from InsecureLdapUrlFlow::PathNode source, InsecureLdapUrlFlow::PathNode sink
20+
where
21+
InsecureLdapUrlFlow::flowPath(source, sink) and
22+
BasicAuthFlow::flowTo(sink.getNode()) and
23+
not RequiresSslFlow::flowTo(sink.getNode())
24+
select sink.getNode(), source, sink, "Insecure LDAP authentication from $@.", source.getNode(),
25+
"LDAP connection string"

‎java/ql/src/Security/CWE/CWE-522/LdapAuthUseLdap.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
String ldapUrl = "ldap://ad.your-server.com:389";
2+
Hashtable<String, String> environment = new Hashtable<String, String>();
3+
environment.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
4+
environment.put(Context.PROVIDER_URL, ldapUrl);
5+
environment.put(Context.REFERRAL, "follow");
6+
environment.put(Context.SECURITY_AUTHENTICATION, "simple");
7+
environment.put(Context.SECURITY_PRINCIPAL, ldapUserName);
8+
environment.put(Context.SECURITY_CREDENTIALS, password);
9+
DirContext dirContext = new InitialDirContext(environment);

‎java/ql/src/Security/CWE/CWE-522/LdapAuthUseLdaps.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
String ldapUrl = "ldaps://ad.your-server.com:636";
2+
Hashtable<String, String> environment = new Hashtable<String, String>();
3+
environment.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
4+
environment.put(Context.PROVIDER_URL, ldapUrl);
5+
environment.put(Context.REFERRAL, "follow");
6+
environment.put(Context.SECURITY_AUTHENTICATION, "simple");
7+
environment.put(Context.SECURITY_PRINCIPAL, ldapUserName);
8+
environment.put(Context.SECURITY_CREDENTIALS, password);
9+
DirContext dirContext = new InitialDirContext(environment);

‎java/ql/src/Security/CWE/CWE-522/LdapEnableSasl.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
String ldapUrl = "ldap://ad.your-server.com:389";
2+
Hashtable<String, String> environment = new Hashtable<String, String>();
3+
environment.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
4+
environment.put(Context.PROVIDER_URL, ldapUrl);
5+
environment.put(Context.REFERRAL, "follow");
6+
environment.put(Context.SECURITY_AUTHENTICATION, "DIGEST-MD5 GSSAPI");
7+
environment.put(Context.SECURITY_PRINCIPAL, ldapUserName);
8+
environment.put(Context.SECURITY_CREDENTIALS, password);
9+
DirContext dirContext = new InitialDirContext(environment);

‎java/ql/src/change-notes/2023-03-08-insecure-ldap-authentication.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* The query `java/insecure-ldap-auth` has been promoted from experimental to the main query pack. This query detects transmission of cleartext credentials in LDAP authentication. Insecure LDAP authentication causes sensitive information to be vulnerable to remote attackers. This query was originally [submitted as an experimental query by @luchua-bc](https://github.com/github/codeql/pull/4854)

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

Lines changed: 0 additions & 24 deletions
This file was deleted.

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

Lines changed: 0 additions & 27 deletions
This file was deleted.

0 commit comments

Comments
 (0)