Skip to content

Commit 35e620a

Browse files
authored
Merge pull request #4854 from luchua-bc/java/insecure-ldap-auth
Java: Insecure LDAP authentication
2 parents d3d56fb + 724c3e0 commit 35e620a

File tree

8 files changed

+503
-10
lines changed

8 files changed

+503
-10
lines changed

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

+2-10
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,6 @@ import semmle.code.java.frameworks.ApacheHttp
1414
import semmle.code.java.dataflow.TaintTracking
1515
import DataFlow::PathGraph
1616

17-
/**
18-
* Gets a regular expression for matching private hosts, which only matches the host portion therefore checking for port is not necessary.
19-
*/
20-
private string getPrivateHostRegex() {
21-
result =
22-
"(?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\\]?(?:[:/?#].*)?"
23-
}
24-
2517
/**
2618
* Class of Java URL constructor.
2719
*/
@@ -76,7 +68,7 @@ class HttpStringLiteral extends StringLiteral {
7668
// Match URLs with the HTTP protocol and without private IP addresses to reduce false positives.
7769
exists(string s | this.getRepresentedString() = s |
7870
s.regexpMatch("(?i)http://[\\[a-zA-Z0-9].*") and
79-
not s.substring(7, s.length()).regexpMatch(getPrivateHostRegex())
71+
not s.substring(7, s.length()) instanceof PrivateHostName
8072
)
8173
}
8274
}
@@ -101,7 +93,7 @@ predicate concatHttpString(Expr protocol, Expr host) {
10193
host.(VarAccess).getVariable().getAnAssignedValue().(CompileTimeConstantExpr).getStringValue()
10294
|
10395
hostString.length() = 0 or // Empty host is loopback address
104-
hostString.regexpMatch(getPrivateHostRegex())
96+
hostString instanceof PrivateHostName
10597
)
10698
}
10799

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
public class InsecureLdapAuth {
2+
/** LDAP authentication */
3+
public DirContext ldapAuth(String ldapUserName, String password) {
4+
{
5+
// BAD: LDAP authentication in cleartext
6+
String ldapUrl = "ldap://ad.your-server.com:389";
7+
}
8+
9+
{
10+
// GOOD: LDAPS authentication over SSL
11+
String ldapUrl = "ldaps://ad.your-server.com:636";
12+
}
13+
14+
Hashtable<String, String> environment = new Hashtable<String, String>();
15+
environment.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
16+
environment.put(Context.PROVIDER_URL, ldapUrl);
17+
environment.put(Context.REFERRAL, "follow");
18+
environment.put(Context.SECURITY_AUTHENTICATION, "simple");
19+
environment.put(Context.SECURITY_PRINCIPAL, ldapUserName);
20+
environment.put(Context.SECURITY_CREDENTIALS, password);
21+
DirContext dirContext = new InitialDirContext(environment);
22+
return dirContext;
23+
}
24+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<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>
6+
</overview>
7+
8+
<recommendation>
9+
<p>Use LDAPS to send credentials through SSL or use SASL authentication.</p>
10+
</recommendation>
11+
12+
<example>
13+
<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>
14+
<sample src="InsecureLdapAuth.java" />
15+
</example>
16+
17+
<references>
18+
<li>
19+
Oracle:
20+
<a href="https://docs.oracle.com/javase/jndi/tutorial/ldap/misc/url.html">LDAP and LDAPS URLs</a>
21+
</li>
22+
<li>
23+
Oracle:
24+
<a href="https://docs.oracle.com/javase/tutorial/jndi/ldap/simple.html">Simple authentication</a>
25+
</li>
26+
</references>
27+
</qhelp>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
1+
/**
2+
* @name Insecure LDAP authentication
3+
* @description LDAP authentication with credentials sent in cleartext.
4+
* @kind path-problem
5+
* @id java/insecure-ldap-auth
6+
* @tags security
7+
* external/cwe-522
8+
* external/cwe-319
9+
*/
10+
11+
import java
12+
import DataFlow
13+
import semmle.code.java.frameworks.Jndi
14+
import semmle.code.java.frameworks.Networking
15+
import semmle.code.java.dataflow.TaintTracking
16+
import DataFlow::PathGraph
17+
18+
/**
19+
* Insecure (non-SSL, non-private) LDAP URL string literal.
20+
*/
21+
class InsecureLdapUrlLiteral extends StringLiteral {
22+
InsecureLdapUrlLiteral() {
23+
// Match connection strings with the LDAP protocol and without private IP addresses to reduce false positives.
24+
exists(string s | this.getRepresentedString() = s |
25+
s.regexpMatch("(?i)ldap://[\\[a-zA-Z0-9].*") and
26+
not s.substring(7, s.length()) instanceof PrivateHostName
27+
)
28+
}
29+
}
30+
31+
/** The interface `javax.naming.Context`. */
32+
class TypeNamingContext extends Interface {
33+
TypeNamingContext() { this.hasQualifiedName("javax.naming", "Context") }
34+
}
35+
36+
/** The class `java.util.Hashtable`. */
37+
class TypeHashtable extends Class {
38+
TypeHashtable() { this.getSourceDeclaration().hasQualifiedName("java.util", "Hashtable") }
39+
}
40+
41+
/**
42+
* Holds if a non-private LDAP string is concatenated from both protocol and host.
43+
*/
44+
predicate concatInsecureLdapString(Expr protocol, Expr host) {
45+
protocol.(CompileTimeConstantExpr).getStringValue() = "ldap://" and
46+
not exists(string hostString |
47+
hostString = host.(CompileTimeConstantExpr).getStringValue() or
48+
hostString =
49+
host.(VarAccess).getVariable().getAnAssignedValue().(CompileTimeConstantExpr).getStringValue()
50+
|
51+
hostString.length() = 0 or // Empty host is loopback address
52+
hostString instanceof PrivateHostName
53+
)
54+
}
55+
56+
/** Gets the leftmost operand in a concatenated string */
57+
Expr getLeftmostConcatOperand(Expr expr) {
58+
if expr instanceof AddExpr
59+
then result = getLeftmostConcatOperand(expr.(AddExpr).getLeftOperand())
60+
else result = expr
61+
}
62+
63+
/**
64+
* String concatenated with `InsecureLdapUrlLiteral`.
65+
*/
66+
class InsecureLdapUrl extends Expr {
67+
InsecureLdapUrl() {
68+
this instanceof InsecureLdapUrlLiteral
69+
or
70+
concatInsecureLdapString(this.(AddExpr).getLeftOperand(),
71+
getLeftmostConcatOperand(this.(AddExpr).getRightOperand()))
72+
}
73+
}
74+
75+
/**
76+
* Holds if `ma` writes the `java.naming.provider.url` (also known as `Context.PROVIDER_URL`) key of a `Hashtable`.
77+
*/
78+
predicate isProviderUrlSetter(MethodAccess ma) {
79+
ma.getMethod().getDeclaringType().getAnAncestor() instanceof TypeHashtable and
80+
ma.getMethod().hasName(["put", "setProperty"]) and
81+
(
82+
ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "java.naming.provider.url"
83+
or
84+
exists(Field f |
85+
ma.getArgument(0) = f.getAnAccess() and
86+
f.hasName("PROVIDER_URL") and
87+
f.getDeclaringType() instanceof TypeNamingContext
88+
)
89+
)
90+
}
91+
92+
/**
93+
* Holds if `ma` sets `fieldValue` to `envValue` in some `Hashtable`.
94+
*/
95+
bindingset[fieldValue, envValue]
96+
predicate hasFieldValueEnv(MethodAccess ma, string fieldValue, string envValue) {
97+
// environment.put("java.naming.security.authentication", "simple")
98+
ma.getMethod().getDeclaringType().getAnAncestor() instanceof TypeHashtable and
99+
ma.getMethod().hasName(["put", "setProperty"]) and
100+
ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = fieldValue and
101+
ma.getArgument(1).(CompileTimeConstantExpr).getStringValue() = envValue
102+
}
103+
104+
/**
105+
* Holds if `ma` sets attribute name `fieldName` to `envValue` in some `Hashtable`.
106+
*/
107+
bindingset[fieldName, envValue]
108+
predicate hasFieldNameEnv(MethodAccess ma, string fieldName, string envValue) {
109+
// environment.put(Context.SECURITY_AUTHENTICATION, "simple")
110+
ma.getMethod().getDeclaringType().getAnAncestor() instanceof TypeHashtable and
111+
ma.getMethod().hasName(["put", "setProperty"]) and
112+
exists(Field f |
113+
ma.getArgument(0) = f.getAnAccess() and
114+
f.hasName(fieldName) and
115+
f.getDeclaringType() instanceof TypeNamingContext
116+
) and
117+
ma.getArgument(1).(CompileTimeConstantExpr).getStringValue() = envValue
118+
}
119+
120+
/**
121+
* Holds if `ma` sets `java.naming.security.authentication` (also known as `Context.SECURITY_AUTHENTICATION`) to `simple` in some `Hashtable`.
122+
*/
123+
predicate isBasicAuthEnv(MethodAccess ma) {
124+
hasFieldValueEnv(ma, "java.naming.security.authentication", "simple") or
125+
hasFieldNameEnv(ma, "SECURITY_AUTHENTICATION", "simple")
126+
}
127+
128+
/**
129+
* Holds if `ma` sets `java.naming.security.protocol` (also known as `Context.SECURITY_PROTOCOL`) to `ssl` in some `Hashtable`.
130+
*/
131+
predicate isSSLEnv(MethodAccess ma) {
132+
hasFieldValueEnv(ma, "java.naming.security.protocol", "ssl") or
133+
hasFieldNameEnv(ma, "SECURITY_PROTOCOL", "ssl")
134+
}
135+
136+
/**
137+
* A taint-tracking configuration for `ldap://` URL in LDAP authentication.
138+
*/
139+
class InsecureUrlFlowConfig extends TaintTracking::Configuration {
140+
InsecureUrlFlowConfig() { this = "InsecureLdapAuth:InsecureUrlFlowConfig" }
141+
142+
/** Source of `ldap://` connection string. */
143+
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof InsecureLdapUrl }
144+
145+
/** Sink of directory context creation. */
146+
override predicate isSink(DataFlow::Node sink) {
147+
exists(ConstructorCall cc |
148+
cc.getConstructedType().getASupertype*() instanceof TypeDirContext and
149+
sink.asExpr() = cc.getArgument(0)
150+
)
151+
}
152+
153+
/** Method call of `env.put()`. */
154+
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
155+
exists(MethodAccess ma |
156+
pred.asExpr() = ma.getArgument(1) and
157+
isProviderUrlSetter(ma) and
158+
succ.asExpr() = ma.getQualifier()
159+
)
160+
}
161+
}
162+
163+
/**
164+
* A taint-tracking configuration for `simple` basic-authentication in LDAP configuration.
165+
*/
166+
class BasicAuthFlowConfig extends DataFlow::Configuration {
167+
BasicAuthFlowConfig() { this = "InsecureLdapAuth:BasicAuthFlowConfig" }
168+
169+
/** Source of `simple` configuration. */
170+
override predicate isSource(DataFlow::Node src) {
171+
exists(MethodAccess ma |
172+
isBasicAuthEnv(ma) and ma.getQualifier() = src.(PostUpdateNode).getPreUpdateNode().asExpr()
173+
)
174+
}
175+
176+
/** Sink of directory context creation. */
177+
override predicate isSink(DataFlow::Node sink) {
178+
exists(ConstructorCall cc |
179+
cc.getConstructedType().getASupertype*() instanceof TypeDirContext and
180+
sink.asExpr() = cc.getArgument(0)
181+
)
182+
}
183+
}
184+
185+
/**
186+
* A taint-tracking configuration for `ssl` configuration in LDAP authentication.
187+
*/
188+
class SSLFlowConfig extends DataFlow::Configuration {
189+
SSLFlowConfig() { this = "InsecureLdapAuth:SSLFlowConfig" }
190+
191+
/** Source of `ssl` configuration. */
192+
override predicate isSource(DataFlow::Node src) {
193+
exists(MethodAccess ma |
194+
isSSLEnv(ma) and ma.getQualifier() = src.(PostUpdateNode).getPreUpdateNode().asExpr()
195+
)
196+
}
197+
198+
/** Sink of directory context creation. */
199+
override predicate isSink(DataFlow::Node sink) {
200+
exists(ConstructorCall cc |
201+
cc.getConstructedType().getASupertype*() instanceof TypeDirContext and
202+
sink.asExpr() = cc.getArgument(0)
203+
)
204+
}
205+
}
206+
207+
from DataFlow::PathNode source, DataFlow::PathNode sink, InsecureUrlFlowConfig config
208+
where
209+
config.hasFlowPath(source, sink) and
210+
exists(BasicAuthFlowConfig bc | bc.hasFlowTo(sink.getNode())) and
211+
not exists(SSLFlowConfig sc | sc.hasFlowTo(sink.getNode()))
212+
select sink.getNode(), source, sink, "Insecure LDAP authentication from $@.", source.getNode(),
213+
"LDAP connection string"

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

+11
Original file line numberDiff line numberDiff line change
@@ -129,3 +129,14 @@ class UrlOpenConnectionMethod extends Method {
129129
this.getName() = "openConnection"
130130
}
131131
}
132+
133+
/**
134+
* A string matching private host names of IPv4 and IPv6, which only matches the host portion therefore checking for port is not necessary.
135+
* 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]
136+
*/
137+
class PrivateHostName extends string {
138+
bindingset[this]
139+
PrivateHostName() {
140+
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\\]?(?:[:/?#].*)?")
141+
}
142+
}

0 commit comments

Comments
 (0)