Skip to content

Commit f14e3f6

Browse files
RasmusWLRasmusWL
authored
Merge pull request #5445 from jorgectf/jorgectf/python/ldapinsecureauth
Python: Add LDAP Insecure Authentication query
2 parents 93daaf5 + ef6e502 commit f14e3f6

File tree

15 files changed

+707
-0
lines changed

15 files changed

+707
-0
lines changed

‎python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.qhelp

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>Failing to ensure the utilization of SSL in an LDAP connection can cause the entire communication
8+
to be sent in cleartext making it easier for an attacker to intercept it.</p>
9+
</overview>
10+
11+
<recommendation>
12+
<p>Always set <code>use_SSL</code> to <code>True</code>, call <code>start_tls_s()</code> or set a proper option flag (<code>ldap.OPT_X_TLS_XXXXXX</code>).</p>
13+
</recommendation>
14+
15+
<example>
16+
<p>This example shows both good and bad ways to deal with this issue under Python 3.</p>
17+
18+
<p>The first one sets <code>use_SSL</code> to true as a keyword argument whereas the second one fails to provide a value for it, so
19+
the default one is used (<code>False</code>).</p>
20+
<sample src="examples/LDAPInsecureAuth.py" />
21+
</example>
22+
23+
</qhelp>

‎python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.ql

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* @name Python Insecure LDAP Authentication
3+
* @description Python LDAP Insecure LDAP Authentication
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @id py/insecure-ldap-auth
7+
* @tags experimental
8+
* security
9+
* external/cwe/cwe-522
10+
* external/cwe/cwe-523
11+
*/
12+
13+
// determine precision above
14+
import python
15+
import DataFlow::PathGraph
16+
import experimental.semmle.python.security.LDAPInsecureAuth
17+
18+
from LDAPInsecureAuthConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
19+
where config.hasFlowPath(source, sink)
20+
select sink.getNode(), source, sink, "$@ is authenticated insecurely.", sink.getNode(),
21+
"This LDAP host"

‎python/ql/src/experimental/Security/CWE-522/examples/LDAPInsecureAuth.py

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
from ldap3 import Server, Connection, ALL
2+
from flask import request, Flask
3+
4+
app = Flask(__name__)
5+
6+
7+
@app.route("/good")
8+
def good():
9+
srv = Server(host, port, use_ssl=True)
10+
conn = Connection(srv, dn, password)
11+
conn.search(dn, search_filter)
12+
return conn.response
13+
14+
15+
@app.route("/bad")
16+
def bad():
17+
srv = Server(host, port)
18+
conn = Connection(srv, dn, password)
19+
conn.search(dn, search_filter)
20+
return conn.response

‎python/ql/src/experimental/semmle/python/Concepts.qll

+23
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,20 @@ module LDAPBind {
156156
* extend `LDAPBind` instead.
157157
*/
158158
abstract class Range extends DataFlow::Node {
159+
/**
160+
* Gets the argument containing the binding host.
161+
*/
162+
abstract DataFlow::Node getHost();
163+
159164
/**
160165
* Gets the argument containing the binding expression.
161166
*/
162167
abstract DataFlow::Node getPassword();
168+
169+
/**
170+
* Holds if the binding process use SSL.
171+
*/
172+
abstract predicate useSSL();
163173
}
164174
}
165175

@@ -174,7 +184,20 @@ class LDAPBind extends DataFlow::Node {
174184

175185
LDAPBind() { this = range }
176186

187+
/**
188+
* Gets the argument containing the binding host.
189+
*/
190+
DataFlow::Node getHost() { result = range.getHost() }
191+
192+
/**
193+
* Gets the argument containing the binding expression.
194+
*/
177195
DataFlow::Node getPassword() { result = range.getPassword() }
196+
197+
/**
198+
* Holds if the binding process use SSL.
199+
*/
200+
predicate useSSL() { range.useSSL() }
178201
}
179202

180203
/** Provides classes for modeling SQL sanitization libraries. */

‎python/ql/src/experimental/semmle/python/frameworks/LDAP.qll

+68
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,11 @@ private module LDAP {
8888
result.(DataFlow::AttrRead).getAttributeName() instanceof LDAP2BindMethods
8989
}
9090

91+
/**List of SSL-demanding options */
92+
private class LDAPSSLOptions extends DataFlow::Node {
93+
LDAPSSLOptions() { this = ldap().getMember("OPT_X_TLS_" + ["DEMAND", "HARD"]).getAUse() }
94+
}
95+
9196
/**
9297
* A class to find `ldap` methods binding a connection.
9398
*
@@ -99,6 +104,44 @@ private module LDAP {
99104
override DataFlow::Node getPassword() {
100105
result in [this.getArg(1), this.getArgByName("cred")]
101106
}
107+
108+
override DataFlow::Node getHost() {
109+
exists(DataFlow::CallCfgNode initialize |
110+
this.getFunction().(DataFlow::AttrRead).getObject().getALocalSource() = initialize and
111+
initialize = ldapInitialize().getACall() and
112+
result = initialize.getArg(0)
113+
)
114+
}
115+
116+
override predicate useSSL() {
117+
// use initialize to correlate `this` and so avoid FP in several instances
118+
exists(DataFlow::CallCfgNode initialize |
119+
// ldap.set_option(ldap.OPT_X_TLS_%s)
120+
ldap().getMember("set_option").getACall().getArg(_) instanceof LDAPSSLOptions
121+
or
122+
this.getFunction().(DataFlow::AttrRead).getObject().getALocalSource() = initialize and
123+
initialize = ldapInitialize().getACall() and
124+
(
125+
// ldap_connection.start_tls_s()
126+
// see https://www.python-ldap.org/en/python-ldap-3.3.0/reference/ldap.html#ldap.LDAPObject.start_tls_s
127+
exists(DataFlow::MethodCallNode startTLS |
128+
startTLS.getObject().getALocalSource() = initialize and
129+
startTLS.getMethodName() = "start_tls_s"
130+
)
131+
or
132+
// ldap_connection.set_option(ldap.OPT_X_TLS_%s, True)
133+
exists(DataFlow::CallCfgNode setOption |
134+
setOption.getFunction().(DataFlow::AttrRead).getObject().getALocalSource() =
135+
initialize and
136+
setOption.getFunction().(DataFlow::AttrRead).getAttributeName() = "set_option" and
137+
setOption.getArg(0) instanceof LDAPSSLOptions and
138+
not DataFlow::exprNode(any(False falseExpr))
139+
.(DataFlow::LocalSourceNode)
140+
.flowsTo(setOption.getArg(1))
141+
)
142+
)
143+
)
144+
}
102145
}
103146

104147
/**
@@ -166,6 +209,31 @@ private module LDAP {
166209
override DataFlow::Node getPassword() {
167210
result in [this.getArg(2), this.getArgByName("password")]
168211
}
212+
213+
override DataFlow::Node getHost() {
214+
exists(DataFlow::CallCfgNode serverCall |
215+
serverCall = ldap3Server().getACall() and
216+
this.getArg(0).getALocalSource() = serverCall and
217+
result = serverCall.getArg(0)
218+
)
219+
}
220+
221+
override predicate useSSL() {
222+
exists(DataFlow::CallCfgNode serverCall |
223+
serverCall = ldap3Server().getACall() and
224+
this.getArg(0).getALocalSource() = serverCall and
225+
DataFlow::exprNode(any(True trueExpr))
226+
.(DataFlow::LocalSourceNode)
227+
.flowsTo([serverCall.getArg(2), serverCall.getArgByName("use_ssl")])
228+
)
229+
or
230+
// ldap_connection.start_tls_s()
231+
// see https://www.python-ldap.org/en/python-ldap-3.3.0/reference/ldap.html#ldap.LDAPObject.start_tls_s
232+
exists(DataFlow::MethodCallNode startTLS |
233+
startTLS.getMethodName() = "start_tls_s" and
234+
startTLS.getObject().getALocalSource() = this
235+
)
236+
}
169237
}
170238

171239
/**

‎python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll

+106
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
/**
2+
* Provides a taint-tracking configuration for detecting LDAP injection vulnerabilities
3+
*/
4+
5+
import python
6+
import semmle.python.dataflow.new.DataFlow
7+
import semmle.python.dataflow.new.TaintTracking
8+
import semmle.python.dataflow.new.RemoteFlowSources
9+
import experimental.semmle.python.Concepts
10+
11+
string getFullHostRegex() { result = "(?i)ldap://.+" }
12+
13+
string getSchemaRegex() { result = "(?i)ldap(://)?" }
14+
15+
string getPrivateHostRegex() {
16+
result =
17+
"(?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\\]?(?:[:/?#].*)?"
18+
}
19+
20+
// "ldap://somethingon.theinternet.com"
21+
class LDAPFullHost extends StrConst {
22+
LDAPFullHost() {
23+
exists(string s |
24+
s = this.getText() and
25+
s.regexpMatch(getFullHostRegex()) and
26+
// check what comes after the `ldap://` prefix
27+
not s.substring(7, s.length()).regexpMatch(getPrivateHostRegex())
28+
)
29+
}
30+
}
31+
32+
class LDAPSchema extends StrConst {
33+
LDAPSchema() { this.getText().regexpMatch(getSchemaRegex()) }
34+
}
35+
36+
class LDAPPrivateHost extends StrConst {
37+
LDAPPrivateHost() { this.getText().regexpMatch(getPrivateHostRegex()) }
38+
}
39+
40+
predicate concatAndCompareAgainstFullHostRegex(LDAPSchema schema, StrConst host) {
41+
not host instanceof LDAPPrivateHost and
42+
(schema.getText() + host.getText()).regexpMatch(getFullHostRegex())
43+
}
44+
45+
// "ldap://" + "somethingon.theinternet.com"
46+
class LDAPBothStrings extends BinaryExpr {
47+
LDAPBothStrings() { concatAndCompareAgainstFullHostRegex(this.getLeft(), this.getRight()) }
48+
}
49+
50+
// schema + host
51+
class LDAPBothVar extends BinaryExpr {
52+
LDAPBothVar() {
53+
exists(SsaVariable schemaVar, SsaVariable hostVar |
54+
this.getLeft() = schemaVar.getVariable().getALoad() and // getAUse is incompatible with Expr
55+
this.getRight() = hostVar.getVariable().getALoad() and
56+
concatAndCompareAgainstFullHostRegex(schemaVar
57+
.getDefinition()
58+
.getImmediateDominator()
59+
.getNode(), hostVar.getDefinition().getImmediateDominator().getNode())
60+
)
61+
}
62+
}
63+
64+
// schema + "somethingon.theinternet.com"
65+
class LDAPVarString extends BinaryExpr {
66+
LDAPVarString() {
67+
exists(SsaVariable schemaVar |
68+
this.getLeft() = schemaVar.getVariable().getALoad() and
69+
concatAndCompareAgainstFullHostRegex(schemaVar
70+
.getDefinition()
71+
.getImmediateDominator()
72+
.getNode(), this.getRight())
73+
)
74+
}
75+
}
76+
77+
// "ldap://" + host
78+
class LDAPStringVar extends BinaryExpr {
79+
LDAPStringVar() {
80+
exists(SsaVariable hostVar |
81+
this.getRight() = hostVar.getVariable().getALoad() and
82+
concatAndCompareAgainstFullHostRegex(this.getLeft(),
83+
hostVar.getDefinition().getImmediateDominator().getNode())
84+
)
85+
}
86+
}
87+
88+
/**
89+
* A taint-tracking configuration for detecting LDAP insecure authentications.
90+
*/
91+
class LDAPInsecureAuthConfig extends TaintTracking::Configuration {
92+
LDAPInsecureAuthConfig() { this = "LDAPInsecureAuthConfig" }
93+
94+
override predicate isSource(DataFlow::Node source) {
95+
source instanceof RemoteFlowSource or
96+
source.asExpr() instanceof LDAPFullHost or
97+
source.asExpr() instanceof LDAPBothStrings or
98+
source.asExpr() instanceof LDAPBothVar or
99+
source.asExpr() instanceof LDAPVarString or
100+
source.asExpr() instanceof LDAPStringVar
101+
}
102+
103+
override predicate isSink(DataFlow::Node sink) {
104+
exists(LDAPBind ldapBind | not ldapBind.useSSL() and sink = ldapBind.getHost())
105+
}
106+
}

‎python/ql/test/experimental/query-tests/Security/CWE-522-global-option/LDAPInsecureAuth.expected

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
edges
2+
nodes
3+
subpaths
4+
#select

‎python/ql/test/experimental/query-tests/Security/CWE-522-global-option/LDAPInsecureAuth.qlref

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE-522/LDAPInsecureAuth.ql

‎python/ql/test/experimental/query-tests/Security/CWE-522-global-option/ldap2_global.py

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
# since global options are considered to affect all files in a repo, we need to keep
2+
# this test in its' own directory (so it doesn't interfere with other tests).
3+
4+
import ldap
5+
from flask import request, Flask
6+
7+
app = Flask(__name__)
8+
9+
# GOOD
10+
11+
# SSL through ldap global variable option
12+
13+
ldap.set_option(ldap.OPT_X_TLS_DEMAND, True)
14+
15+
16+
@app.route("/one")
17+
def one():
18+
# The following connection would have been insecure if the global option above was
19+
# not set
20+
ldap_connection_5 = ldap.initialize("ldap://somethingon.theinternet.com")
21+
ldap_connection_5.simple_bind_s('', '')
22+
user = ldap_connection_5.search_s(
23+
"dn", ldap.SCOPE_SUBTREE, "search_filter")
24+
25+
return user
26+
27+
28+
# if __name__ == "__main__":
29+
# app.run(debug=True)

‎python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.expected

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
edges
2+
| ldap3_remote.py:101:12:101:49 | ControlFlowNode for BinaryExpr | ldap3_remote.py:102:18:102:21 | ControlFlowNode for host |
3+
| ldap3_remote.py:114:12:114:49 | ControlFlowNode for BinaryExpr | ldap3_remote.py:115:18:115:21 | ControlFlowNode for host |
4+
| ldap3_remote.py:126:12:126:31 | ControlFlowNode for BinaryExpr | ldap3_remote.py:127:18:127:21 | ControlFlowNode for host |
5+
| ldap3_remote.py:138:21:138:27 | ControlFlowNode for request | ldap3_remote.py:138:21:138:32 | ControlFlowNode for Attribute |
6+
| ldap3_remote.py:138:21:138:32 | ControlFlowNode for Attribute | ldap3_remote.py:138:21:138:40 | ControlFlowNode for Subscript |
7+
| ldap3_remote.py:138:21:138:40 | ControlFlowNode for Subscript | ldap3_remote.py:139:18:139:21 | ControlFlowNode for host |
8+
nodes
9+
| ldap2_remote.py:45:41:45:60 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
10+
| ldap2_remote.py:56:41:56:60 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
11+
| ldap3_remote.py:101:12:101:49 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
12+
| ldap3_remote.py:102:18:102:21 | ControlFlowNode for host | semmle.label | ControlFlowNode for host |
13+
| ldap3_remote.py:114:12:114:49 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
14+
| ldap3_remote.py:115:18:115:21 | ControlFlowNode for host | semmle.label | ControlFlowNode for host |
15+
| ldap3_remote.py:126:12:126:31 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
16+
| ldap3_remote.py:127:18:127:21 | ControlFlowNode for host | semmle.label | ControlFlowNode for host |
17+
| ldap3_remote.py:138:21:138:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
18+
| ldap3_remote.py:138:21:138:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
19+
| ldap3_remote.py:138:21:138:40 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
20+
| ldap3_remote.py:139:18:139:21 | ControlFlowNode for host | semmle.label | ControlFlowNode for host |
21+
subpaths
22+
#select
23+
| ldap2_remote.py:45:41:45:60 | ControlFlowNode for BinaryExpr | ldap2_remote.py:45:41:45:60 | ControlFlowNode for BinaryExpr | ldap2_remote.py:45:41:45:60 | ControlFlowNode for BinaryExpr | $@ is authenticated insecurely. | ldap2_remote.py:45:41:45:60 | ControlFlowNode for BinaryExpr | This LDAP host |
24+
| ldap2_remote.py:56:41:56:60 | ControlFlowNode for BinaryExpr | ldap2_remote.py:56:41:56:60 | ControlFlowNode for BinaryExpr | ldap2_remote.py:56:41:56:60 | ControlFlowNode for BinaryExpr | $@ is authenticated insecurely. | ldap2_remote.py:56:41:56:60 | ControlFlowNode for BinaryExpr | This LDAP host |
25+
| ldap3_remote.py:102:18:102:21 | ControlFlowNode for host | ldap3_remote.py:101:12:101:49 | ControlFlowNode for BinaryExpr | ldap3_remote.py:102:18:102:21 | ControlFlowNode for host | $@ is authenticated insecurely. | ldap3_remote.py:102:18:102:21 | ControlFlowNode for host | This LDAP host |
26+
| ldap3_remote.py:115:18:115:21 | ControlFlowNode for host | ldap3_remote.py:114:12:114:49 | ControlFlowNode for BinaryExpr | ldap3_remote.py:115:18:115:21 | ControlFlowNode for host | $@ is authenticated insecurely. | ldap3_remote.py:115:18:115:21 | ControlFlowNode for host | This LDAP host |
27+
| ldap3_remote.py:127:18:127:21 | ControlFlowNode for host | ldap3_remote.py:126:12:126:31 | ControlFlowNode for BinaryExpr | ldap3_remote.py:127:18:127:21 | ControlFlowNode for host | $@ is authenticated insecurely. | ldap3_remote.py:127:18:127:21 | ControlFlowNode for host | This LDAP host |
28+
| ldap3_remote.py:139:18:139:21 | ControlFlowNode for host | ldap3_remote.py:138:21:138:27 | ControlFlowNode for request | ldap3_remote.py:139:18:139:21 | ControlFlowNode for host | $@ is authenticated insecurely. | ldap3_remote.py:139:18:139:21 | ControlFlowNode for host | This LDAP host |

‎python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.qlref

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE-522/LDAPInsecureAuth.ql

0 commit comments

Comments
 (0)