From 7de9214c992f068ccfcdc25bceac48896c14862e Mon Sep 17 00:00:00 2001
From: jorgectf
Date: Thu, 18 Mar 2021 17:41:34 +0100
Subject: [PATCH 01/18] Upload LDAP Insecure authentication query and tests
---
.../src/Security/CWE-090/LDAPInsecureAuth.ql | 191 ++++++++++++++++++
.../src/Security/CWE-090/tests/2_private.py | 66 ++++++
.../ql/src/Security/CWE-090/tests/2_remote.py | 66 ++++++
.../src/Security/CWE-090/tests/3_private.py | 105 ++++++++++
.../ql/src/Security/CWE-090/tests/3_remote.py | 146 +++++++++++++
5 files changed, 574 insertions(+)
create mode 100644 python/ql/src/Security/CWE-090/LDAPInsecureAuth.ql
create mode 100644 python/ql/src/Security/CWE-090/tests/2_private.py
create mode 100644 python/ql/src/Security/CWE-090/tests/2_remote.py
create mode 100644 python/ql/src/Security/CWE-090/tests/3_private.py
create mode 100644 python/ql/src/Security/CWE-090/tests/3_remote.py
diff --git a/python/ql/src/Security/CWE-090/LDAPInsecureAuth.ql b/python/ql/src/Security/CWE-090/LDAPInsecureAuth.ql
new file mode 100644
index 000000000000..b156cc2b036a
--- /dev/null
+++ b/python/ql/src/Security/CWE-090/LDAPInsecureAuth.ql
@@ -0,0 +1,191 @@
+/**
+ * @name Python Insecure LDAP Authentication
+ * @description Python LDAP Insecure LDAP Authentication
+ * @kind path-problem
+ * @problem.severity error
+ * @id python/insecure-ldap-auth
+ * @tags experimental
+ * security
+ * external/cwe/cwe-090
+ */
+
+import python
+import semmle.python.dataflow.new.RemoteFlowSources
+import semmle.python.dataflow.new.DataFlow
+import semmle.python.dataflow.new.TaintTracking
+import semmle.python.dataflow.new.internal.TaintTrackingPublic
+import DataFlow::PathGraph
+
+class FalseArg extends ControlFlowNode {
+ FalseArg() { this.getNode().(Expr).(BooleanLiteral) instanceof False }
+}
+
+// From luchua-bc's Insecure LDAP authentication in Java (to reduce false positives)
+string getFullHostRegex() { result = "(?i)ldap://[\\[a-zA-Z0-9].*" }
+
+string getSchemaRegex() { result = "(?i)ldap(://)?" }
+
+string getPrivateHostRegex() {
+ result =
+ "(?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\\]?(?:[:/?#].*)?"
+}
+
+// "ldap://somethingon.theinternet.com"
+class LDAPFullHost extends StrConst {
+ LDAPFullHost() {
+ exists(string s |
+ s = this.getText() and
+ s.regexpMatch(getFullHostRegex()) and
+ not s.substring(7, s.length()).regexpMatch(getPrivateHostRegex()) // No need to check for ldaps, would be SSL by default.
+ )
+ }
+}
+
+class LDAPSchema extends StrConst {
+ LDAPSchema() { this.getText().regexpMatch(getSchemaRegex()) }
+}
+
+class LDAPPrivateHost extends StrConst {
+ LDAPPrivateHost() { this.getText().regexpMatch(getPrivateHostRegex()) }
+}
+
+predicate concatAndCompareAgainstFullHostRegex(Expr schema, Expr host) {
+ schema instanceof LDAPSchema and
+ not host instanceof LDAPPrivateHost and
+ exists(string full_host |
+ full_host = schema.(StrConst).getText() + host.(StrConst).getText() and
+ full_host.regexpMatch(getFullHostRegex())
+ )
+}
+
+// "ldap://" + "somethingon.theinternet.com"
+class LDAPBothStrings extends BinaryExpr {
+ LDAPBothStrings() { concatAndCompareAgainstFullHostRegex(this.getLeft(), this.getRight()) }
+}
+
+// schema + host
+class LDAPBothVar extends BinaryExpr {
+ LDAPBothVar() {
+ exists(SsaVariable schemaVar, SsaVariable hostVar |
+ this.getLeft() = schemaVar.getVariable().getALoad() and // getAUse is incompatible with Expr
+ this.getRight() = hostVar.getVariable().getALoad() and
+ concatAndCompareAgainstFullHostRegex(schemaVar
+ .getDefinition()
+ .getImmediateDominator()
+ .getNode(), hostVar.getDefinition().getImmediateDominator().getNode())
+ )
+ }
+}
+
+// schema + "somethingon.theinternet.com"
+class LDAPVarString extends BinaryExpr {
+ LDAPVarString() {
+ exists(SsaVariable schemaVar |
+ this.getLeft() = schemaVar.getVariable().getALoad() and
+ concatAndCompareAgainstFullHostRegex(schemaVar
+ .getDefinition()
+ .getImmediateDominator()
+ .getNode(), this.getRight())
+ )
+ }
+}
+
+// "ldap://" + host
+class LDAPStringVar extends BinaryExpr {
+ LDAPStringVar() {
+ exists(SsaVariable hostVar |
+ this.getRight() = hostVar.getVariable().getALoad() and
+ concatAndCompareAgainstFullHostRegex(this.getLeft(),
+ hostVar.getDefinition().getImmediateDominator().getNode())
+ )
+ }
+}
+
+class LDAPInsecureAuthSource extends DataFlow::Node {
+ LDAPInsecureAuthSource() {
+ this instanceof RemoteFlowSource or
+ this.asExpr() instanceof LDAPBothStrings or
+ this.asExpr() instanceof LDAPBothVar or
+ this.asExpr() instanceof LDAPVarString or
+ this.asExpr() instanceof LDAPStringVar
+ }
+}
+
+class SafeLDAPOptions extends ControlFlowNode {
+ SafeLDAPOptions() {
+ this = Value::named("ldap.OPT_X_TLS_ALLOW").getAReference() or
+ this = Value::named("ldap.OPT_X_TLS_TRY").getAReference() or
+ this = Value::named("ldap.OPT_X_TLS_DEMAND").getAReference() or
+ this = Value::named("ldap.OPT_X_TLS_HARD").getAReference()
+ }
+}
+
+// LDAP3
+class LDAPInsecureAuthSink extends DataFlow::Node {
+ LDAPInsecureAuthSink() {
+ exists(SsaVariable connVar, CallNode connCall, SsaVariable srvVar, CallNode srvCall |
+ // set connCall as a Call to ldap3.Connection
+ connCall = Value::named("ldap3.Connection").getACall() and
+ // get variable whose definition is a call to ldap3.Connection to correlate ldap3.Server and Connection.start_tls()
+ connVar.getDefinition().getImmediateDominator() = connCall and
+ // get connCall's first argument variable definition
+ srvVar.getAUse() = connCall.getArg(0) and
+ /*
+ * // restrict srvVar definition to a ldap3.Server Call
+ * srvCall = Value::named("ldap3.Server").getACall() and
+ * srvVar.getDefinition().getImmediateDominator() = srvCall
+ * // redundant? ldap3.Connection's first argument *must* be ldap3.Server
+ */
+
+ // set srvCall as srvVar definition's call
+ srvVar.getDefinition().getImmediateDominator() = srvCall and
+ // set ldap3.Server's 1st argument as sink
+ this.asExpr() = srvCall.getArg(0).getNode() and
+ (
+ // check ldap3.Server call's 3rd argument (positional) is null and there's no use_ssl
+ count(srvCall.getAnArg()) < 3 and
+ count(srvCall.getArgByName("use_ssl")) = 0
+ or
+ // check ldap3.Server call's 3rd argument is False
+ srvCall.getAnArg() instanceof FalseArg
+ or
+ // check ldap3.Server argByName "use_ssl" is False
+ srvCall.getArgByName("use_ssl") instanceof FalseArg
+ ) and
+ /*
+ * Avoid flow through any function (Server()) whose variable declaring it (srv) is the first
+ * argument in any function (Connection()) whose variable declaring it also calls .start_tls
+ */
+
+ /*
+ * host = schema + "somethingon.theinternet.com"
+ * srv = Server(host, port = 1337)
+ * conn = Connection(srv, "dn", "password")
+ * conn.start_tls() !
+ */
+
+ not connVar
+ .getAUse()
+ .getImmediateDominator()
+ .(CallNode)
+ .getNode()
+ .getFunc()
+ .(Attribute)
+ .getName()
+ .matches("start_tls")
+ )
+ }
+}
+
+class LDAPInsecureAuthConfig extends TaintTracking::Configuration {
+ LDAPInsecureAuthConfig() { this = "LDAPInsecureAuthConfig" }
+
+ override predicate isSource(DataFlow::Node source) { source instanceof LDAPInsecureAuthSource }
+
+ override predicate isSink(DataFlow::Node sink) { sink instanceof LDAPInsecureAuthSink }
+}
+
+from LDAPInsecureAuthConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "$@ from $@ is authenticated insecurely.", source.getNode(),
+ "The host", sink.getNode(), "this LDAP query"
diff --git a/python/ql/src/Security/CWE-090/tests/2_private.py b/python/ql/src/Security/CWE-090/tests/2_private.py
new file mode 100644
index 000000000000..4e152cd93c2e
--- /dev/null
+++ b/python/ql/src/Security/CWE-090/tests/2_private.py
@@ -0,0 +1,66 @@
+import ldap
+from flask import request, Flask
+
+app = Flask(__name__)
+
+schema = "ldap://"
+remote_host = "127.0.0.1"
+full_host = "ldap://127.0.0.1"
+
+
+# GOOD
+
+# SSL through ldap variable option
+# host from variable + variable
+
+@app.route("/one")
+def one():
+ ldap_connection_1 = ldap.initialize(schema + remote_host)
+ ldap_connection_1.set_option(ldap.OPT_X_TLS_DEMAND, True)
+ ldap_connection_1.simple_bind_s('', '')
+ user = ldap_connection_1.search_s(
+ "dn", ldap.SCOPE_SUBTREE, "search_filter")
+
+ return user
+
+
+# SSL through ldap start_tls_s
+# hardcoded host
+
+@app.route("/two")
+def two():
+ ldap_connection_2 = ldap.initialize("ldap://127.0.0.1")
+ ldap_connection_2.start_tls_s()
+ ldap_connection_2.simple_bind_s('', '')
+ user = ldap_connection_2.search_s(
+ "dn", ldap.SCOPE_SUBTREE, "search_filter")
+
+ return user
+
+
+# BAD (not a sink because it's private)
+
+@app.route("/one_bad")
+def one_bad():
+ ldap_connection_3 = ldap.initialize(schema + remote_host)
+ ldap_connection_3.set_option(ldap.OPT_X_TLS_DEMAND, False)
+ ldap_connection_3.simple_bind_s('', '')
+ user = ldap_connection_3.search_s(
+ "dn", ldap.SCOPE_SUBTREE, "search_filter")
+
+ return user
+
+
+@app.route("/one_bad_2")
+def one_bad_2():
+ ldap_connection_4 = ldap.initialize(schema + remote_host)
+ ldap_connection_4.set_option(ldap.OPT_X_TLS_NEVER)
+ ldap_connection_4.simple_bind_s('', '')
+ user = ldap_connection_4.search_s(
+ "dn", ldap.SCOPE_SUBTREE, "search_filter")
+
+ return user
+
+
+# if __name__ == "__main__":
+# app.run(debug=True)
diff --git a/python/ql/src/Security/CWE-090/tests/2_remote.py b/python/ql/src/Security/CWE-090/tests/2_remote.py
new file mode 100644
index 000000000000..6f82f1dbfd17
--- /dev/null
+++ b/python/ql/src/Security/CWE-090/tests/2_remote.py
@@ -0,0 +1,66 @@
+import ldap
+from flask import request, Flask
+
+app = Flask(__name__)
+
+schema = "ldap://"
+remote_host = "somethingon.theinternet.com"
+full_host = "ldap://somethingon.theinternet.com"
+
+
+# GOOD
+
+# SSL through ldap variable option
+# host from variable + variable
+
+@app.route("/one")
+def one():
+ ldap_connection_5 = ldap.initialize(schema + remote_host)
+ ldap_connection_5.set_option(ldap.OPT_X_TLS_DEMAND, True)
+ ldap_connection_5.simple_bind_s('', '')
+ user = ldap_connection_5.search_s(
+ "dn", ldap.SCOPE_SUBTREE, "search_filter")
+
+ return user
+
+
+# SSL through ldap start_tls_s
+# hardcoded host
+
+@app.route("/two")
+def two():
+ ldap_connection_6 = ldap.initialize("ldap://somethingon.theinternet.com")
+ ldap_connection_6.start_tls_s()
+ ldap_connection_6.simple_bind_s('', '')
+ user = ldap_connection_6.search_s(
+ "dn", ldap.SCOPE_SUBTREE, "search_filter")
+
+ return user
+
+
+# BAD
+
+@app.route("/one_bad")
+def one_bad():
+ ldap_connection_7 = ldap.initialize(schema + remote_host)
+ ldap_connection_7.set_option(ldap.OPT_X_TLS_DEMAND, False)
+ ldap_connection_7.simple_bind_s('', '')
+ user = ldap_connection_7.search_s(
+ "dn", ldap.SCOPE_SUBTREE, "search_filter")
+
+ return user
+
+
+@app.route("/one_bad_2")
+def one_bad_2():
+ ldap_connection_8 = ldap.initialize(schema + remote_host)
+ ldap_connection_8.set_option(ldap.OPT_X_TLS_NEVER)
+ ldap_connection_8.simple_bind_s('', '')
+ user = ldap_connection_8.search_s(
+ "dn", ldap.SCOPE_SUBTREE, "search_filter")
+
+ return user
+
+
+# if __name__ == "__main__":
+# app.run(debug=True)
diff --git a/python/ql/src/Security/CWE-090/tests/3_private.py b/python/ql/src/Security/CWE-090/tests/3_private.py
new file mode 100644
index 000000000000..7c4ede3f0e61
--- /dev/null
+++ b/python/ql/src/Security/CWE-090/tests/3_private.py
@@ -0,0 +1,105 @@
+from ldap3 import Server, Connection, ALL
+from flask import request, Flask
+
+app = Flask(__name__)
+
+schema = "ldap://"
+partial_host = "127.0.0.1"
+full_host = "ldap://127.0.0.1"
+
+
+# hardcoded host
+
+@app.route("/one")
+def one():
+ srv = Server("ldap://127.0.0.1", port=1337)
+ conn = Connection(srv, "dn", "password")
+ conn.search("dn", "search_filter")
+ return conn.response
+
+
+# host from variable
+
+@app.route("/two")
+def two():
+ srv = Server(full_host, port=1337)
+ conn = Connection(srv, "dn", "password")
+ conn.search("dn", "search_filter")
+ return conn.response
+
+
+# schema from string + variable
+
+@app.route("/three")
+def three():
+ host = "ldap://" + partial_host
+
+ srv = Server(host, port=1337)
+ conn = Connection(srv, "dn", "password")
+ conn.search("dn", "search_filter")
+ return conn.response
+
+
+# schema from variable + variable
+
+@app.route("/four")
+def four():
+ host = schema + partial_host
+
+ srv = Server(host, port=1337)
+ conn = Connection(srv, "dn", "password")
+ conn.search("dn", "search_filter")
+ return conn.response
+
+
+# schema from string + string
+
+@app.route("/five")
+def five():
+ host = "ldap://" + "127.0.0.1"
+
+ srv = Server(host, port=1337)
+ conn = Connection(srv, "dn", "password")
+ conn.search("dn", "search_filter")
+ return conn.response
+
+
+# schema from variable + hardcoded host
+
+@app.route("/six")
+def six():
+ host = schema + "127.0.0.1"
+
+ srv = Server(host, port=1337)
+ conn = Connection(srv, "dn", "password")
+ conn.search("dn", "search_filter")
+ return conn.response
+
+
+# use_ssl = True (positional argument)
+# host from string + variable
+
+@app.route("/four")
+def four():
+ host = "ldap://" + partial_host
+
+ srv = Server(host, port=1337, True)
+ conn = Connection(srv, "dn", "password")
+ conn.search("dn", "search_filter")
+ return conn.response
+
+
+# use_ssl = True (argument by name)
+# host from variable + variable
+
+@app.route("/five")
+def five():
+ host = schema + partial_host
+
+ srv = Server(host, port=1337, use_ssl=True)
+ conn = Connection(srv, "dn", "password")
+ conn.search("dn", "search_filter")
+
+
+# if __name__ == "__main__":
+# app.run(debug=True)
diff --git a/python/ql/src/Security/CWE-090/tests/3_remote.py b/python/ql/src/Security/CWE-090/tests/3_remote.py
new file mode 100644
index 000000000000..366fce2b186e
--- /dev/null
+++ b/python/ql/src/Security/CWE-090/tests/3_remote.py
@@ -0,0 +1,146 @@
+from ldap3 import Server, Connection, ALL
+from flask import request, Flask
+
+app = Flask(__name__)
+
+schema = "ldap://"
+remote_host = "somethingon.theinternet.com"
+full_host = "ldap://somethingon.theinternet.com"
+
+
+# use_ssl = True (positional argument)
+# hardcoded host
+
+@app.route("/one")
+def one():
+ srv = Server("ldap://somethingon.theinternet.com", port=1337, True)
+ conn = Connection(srv, "dn", "password")
+ conn.search("dn", "search_filter")
+ return conn.response
+
+
+# use_ssl = True (argument by name)
+# host from variable
+
+@app.route("/two")
+def two():
+ srv = Server(full_host, port=1337, use_ssl=True)
+ conn = Connection(srv, "dn", "password")
+ conn.search("dn", "search_filter")
+ return conn.response
+
+
+# use_ssl = True (argument by name)
+# host from RFS
+
+@app.route("/three")
+def three():
+ srv = Server(request.args['host'], port=1337, use_ssl=True)
+ conn = Connection(srv, "dn", "password")
+ conn.search("dn", "search_filter")
+ return conn.response
+
+
+# use_ssl = True (positional argument)
+# host from string + variable
+
+@app.route("/four")
+def four():
+ host = "ldap://" + remote_host
+
+ srv = Server(host, port=1337, True)
+ conn = Connection(srv, "dn", "password")
+ conn.search("dn", "search_filter")
+ return conn.response
+
+
+# use_ssl = True (argument by name)
+# host from variable + variable
+
+@app.route("/five")
+def five():
+ host = schema + remote_host
+
+ srv = Server(host, port=1337, use_ssl=True)
+ conn = Connection(srv, "dn", "password")
+ conn.search("dn", "search_filter")
+ return conn.response
+
+
+# use_ssl = True (argument by name)
+# host from string + RFS
+
+@app.route("/six")
+def six():
+ host = "ldap://" + request.args['host']
+
+ srv = Server(host, port=1337, use_ssl=True)
+ conn = Connection(srv, "dn", "password")
+ conn.search("dn", "search_filter")
+ return conn.response
+
+
+# use_ssl = True (positional argument)
+# host from variable + RFS
+
+@app.route("/seven")
+def seven():
+ host = schema + request.args['host']
+
+ srv = Server(host, port=1337, True)
+ conn = Connection(srv, "dn", "password")
+ conn.search("dn", "search_filter")
+ return conn.response
+
+
+# SSL through special method
+# host from variable + hardcoded host
+
+@app.route("/eight")
+def eight():
+ host = schema + "somethingon.theinternet.com"
+ srv = Server(host, port=1337)
+ conn = Connection(srv, "dn", "password")
+ conn.start_tls() # !
+ conn.search("dn", "search_filter")
+ return conn.response
+
+
+# No SSL (to test sink)
+# host from variable + hardcoded host
+
+@app.route("/nine")
+def nine():
+ host = schema + "somethingon.theinternet.com"
+ srv = Server(host, port=1337, False)
+ conn = Connection(srv, "dn", "password")
+ conn.search("dn", "search_filter")
+ return conn.response
+
+
+# No SSL (to test sink)
+# host from variable + variable
+
+@app.route("/ten")
+def ten():
+ host = schema + remote_host
+ srv = Server(host, port=1337, use_ssl=False)
+ conn = Connection(srv, "dn", "password")
+ conn.search("dn", "search_filter")
+ return conn.response
+
+
+# No SSL (to test sink)
+# host from variable + RFS
+
+@app.route("/eleven")
+def eleven():
+ host = schema + request.args['host']
+ srv = Server(host, port=1337)
+ conn = Connection(srv, "dn", "password")
+ conn.search("dn", "search_filter")
+ return conn.response
+
+
+# if __name__ == "__main__":
+# app.run(debug=True)
From 3ce0a9c8c08ed8091335267143fd2f25703fb425 Mon Sep 17 00:00:00 2001
From: jorgectf
Date: Thu, 18 Mar 2021 20:20:04 +0100
Subject: [PATCH 02/18] Move to experimental folder
---
.../src/{ => experimental}/Security/CWE-090/LDAPInsecureAuth.ql | 0
.../ql/src/{ => experimental}/Security/CWE-090/tests/2_private.py | 0
.../ql/src/{ => experimental}/Security/CWE-090/tests/2_remote.py | 0
.../ql/src/{ => experimental}/Security/CWE-090/tests/3_private.py | 0
.../ql/src/{ => experimental}/Security/CWE-090/tests/3_remote.py | 0
5 files changed, 0 insertions(+), 0 deletions(-)
rename python/ql/src/{ => experimental}/Security/CWE-090/LDAPInsecureAuth.ql (100%)
rename python/ql/src/{ => experimental}/Security/CWE-090/tests/2_private.py (100%)
rename python/ql/src/{ => experimental}/Security/CWE-090/tests/2_remote.py (100%)
rename python/ql/src/{ => experimental}/Security/CWE-090/tests/3_private.py (100%)
rename python/ql/src/{ => experimental}/Security/CWE-090/tests/3_remote.py (100%)
diff --git a/python/ql/src/Security/CWE-090/LDAPInsecureAuth.ql b/python/ql/src/experimental/Security/CWE-090/LDAPInsecureAuth.ql
similarity index 100%
rename from python/ql/src/Security/CWE-090/LDAPInsecureAuth.ql
rename to python/ql/src/experimental/Security/CWE-090/LDAPInsecureAuth.ql
diff --git a/python/ql/src/Security/CWE-090/tests/2_private.py b/python/ql/src/experimental/Security/CWE-090/tests/2_private.py
similarity index 100%
rename from python/ql/src/Security/CWE-090/tests/2_private.py
rename to python/ql/src/experimental/Security/CWE-090/tests/2_private.py
diff --git a/python/ql/src/Security/CWE-090/tests/2_remote.py b/python/ql/src/experimental/Security/CWE-090/tests/2_remote.py
similarity index 100%
rename from python/ql/src/Security/CWE-090/tests/2_remote.py
rename to python/ql/src/experimental/Security/CWE-090/tests/2_remote.py
diff --git a/python/ql/src/Security/CWE-090/tests/3_private.py b/python/ql/src/experimental/Security/CWE-090/tests/3_private.py
similarity index 100%
rename from python/ql/src/Security/CWE-090/tests/3_private.py
rename to python/ql/src/experimental/Security/CWE-090/tests/3_private.py
diff --git a/python/ql/src/Security/CWE-090/tests/3_remote.py b/python/ql/src/experimental/Security/CWE-090/tests/3_remote.py
similarity index 100%
rename from python/ql/src/Security/CWE-090/tests/3_remote.py
rename to python/ql/src/experimental/Security/CWE-090/tests/3_remote.py
From 957b3e1e85fd3c478b81ffbef7c4e6bed5e7d235 Mon Sep 17 00:00:00 2001
From: jorgectf
Date: Thu, 18 Mar 2021 20:39:53 +0100
Subject: [PATCH 03/18] Precision warn
---
python/ql/src/experimental/Security/CWE-090/LDAPInsecureAuth.ql | 1 +
1 file changed, 1 insertion(+)
diff --git a/python/ql/src/experimental/Security/CWE-090/LDAPInsecureAuth.ql b/python/ql/src/experimental/Security/CWE-090/LDAPInsecureAuth.ql
index b156cc2b036a..ba31bb8a2875 100644
--- a/python/ql/src/experimental/Security/CWE-090/LDAPInsecureAuth.ql
+++ b/python/ql/src/experimental/Security/CWE-090/LDAPInsecureAuth.ql
@@ -9,6 +9,7 @@
* external/cwe/cwe-090
*/
+// Determine precision above
import python
import semmle.python.dataflow.new.RemoteFlowSources
import semmle.python.dataflow.new.DataFlow
From a34d6d390ecb8387b3c68bf79cdf9ac3eb7b4a25 Mon Sep 17 00:00:00 2001
From: jorgectf
Date: Thu, 22 Jul 2021 18:34:57 +0200
Subject: [PATCH 04/18] Port to ApiGraphs and finish the query
---
.../Security/CWE-090/LDAPInsecureAuth.ql | 192 ------------------
.../Security/CWE-522/LDAPInsecureAuth.ql | 20 ++
.../experimental/semmle/python/Concepts.qll | 23 +++
.../semmle/python/frameworks/LDAP.qll | 54 +++++
.../python/security/LDAPInsecureAuth.qll | 108 ++++++++++
.../CWE-522/LDAPInsecureAuth.expected | 39 ++++
.../Security/CWE-522/LDAPInsecureAuth.qlref | 1 +
.../Security/CWE-522/ldap2_private.py} | 0
.../Security/CWE-522/ldap2_remote.py} | 0
.../Security/CWE-522/ldap3_private.py} | 2 +-
.../Security/CWE-522/ldap3_remote.py} | 0
11 files changed, 246 insertions(+), 193 deletions(-)
delete mode 100644 python/ql/src/experimental/Security/CWE-090/LDAPInsecureAuth.ql
create mode 100644 python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.ql
create mode 100644 python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll
create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.expected
create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.qlref
rename python/ql/{src/experimental/Security/CWE-090/tests/2_private.py => test/experimental/query-tests/Security/CWE-522/ldap2_private.py} (100%)
rename python/ql/{src/experimental/Security/CWE-090/tests/2_remote.py => test/experimental/query-tests/Security/CWE-522/ldap2_remote.py} (100%)
rename python/ql/{src/experimental/Security/CWE-090/tests/3_private.py => test/experimental/query-tests/Security/CWE-522/ldap3_private.py} (98%)
rename python/ql/{src/experimental/Security/CWE-090/tests/3_remote.py => test/experimental/query-tests/Security/CWE-522/ldap3_remote.py} (100%)
diff --git a/python/ql/src/experimental/Security/CWE-090/LDAPInsecureAuth.ql b/python/ql/src/experimental/Security/CWE-090/LDAPInsecureAuth.ql
deleted file mode 100644
index ba31bb8a2875..000000000000
--- a/python/ql/src/experimental/Security/CWE-090/LDAPInsecureAuth.ql
+++ /dev/null
@@ -1,192 +0,0 @@
-/**
- * @name Python Insecure LDAP Authentication
- * @description Python LDAP Insecure LDAP Authentication
- * @kind path-problem
- * @problem.severity error
- * @id python/insecure-ldap-auth
- * @tags experimental
- * security
- * external/cwe/cwe-090
- */
-
-// Determine precision above
-import python
-import semmle.python.dataflow.new.RemoteFlowSources
-import semmle.python.dataflow.new.DataFlow
-import semmle.python.dataflow.new.TaintTracking
-import semmle.python.dataflow.new.internal.TaintTrackingPublic
-import DataFlow::PathGraph
-
-class FalseArg extends ControlFlowNode {
- FalseArg() { this.getNode().(Expr).(BooleanLiteral) instanceof False }
-}
-
-// From luchua-bc's Insecure LDAP authentication in Java (to reduce false positives)
-string getFullHostRegex() { result = "(?i)ldap://[\\[a-zA-Z0-9].*" }
-
-string getSchemaRegex() { result = "(?i)ldap(://)?" }
-
-string getPrivateHostRegex() {
- result =
- "(?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\\]?(?:[:/?#].*)?"
-}
-
-// "ldap://somethingon.theinternet.com"
-class LDAPFullHost extends StrConst {
- LDAPFullHost() {
- exists(string s |
- s = this.getText() and
- s.regexpMatch(getFullHostRegex()) and
- not s.substring(7, s.length()).regexpMatch(getPrivateHostRegex()) // No need to check for ldaps, would be SSL by default.
- )
- }
-}
-
-class LDAPSchema extends StrConst {
- LDAPSchema() { this.getText().regexpMatch(getSchemaRegex()) }
-}
-
-class LDAPPrivateHost extends StrConst {
- LDAPPrivateHost() { this.getText().regexpMatch(getPrivateHostRegex()) }
-}
-
-predicate concatAndCompareAgainstFullHostRegex(Expr schema, Expr host) {
- schema instanceof LDAPSchema and
- not host instanceof LDAPPrivateHost and
- exists(string full_host |
- full_host = schema.(StrConst).getText() + host.(StrConst).getText() and
- full_host.regexpMatch(getFullHostRegex())
- )
-}
-
-// "ldap://" + "somethingon.theinternet.com"
-class LDAPBothStrings extends BinaryExpr {
- LDAPBothStrings() { concatAndCompareAgainstFullHostRegex(this.getLeft(), this.getRight()) }
-}
-
-// schema + host
-class LDAPBothVar extends BinaryExpr {
- LDAPBothVar() {
- exists(SsaVariable schemaVar, SsaVariable hostVar |
- this.getLeft() = schemaVar.getVariable().getALoad() and // getAUse is incompatible with Expr
- this.getRight() = hostVar.getVariable().getALoad() and
- concatAndCompareAgainstFullHostRegex(schemaVar
- .getDefinition()
- .getImmediateDominator()
- .getNode(), hostVar.getDefinition().getImmediateDominator().getNode())
- )
- }
-}
-
-// schema + "somethingon.theinternet.com"
-class LDAPVarString extends BinaryExpr {
- LDAPVarString() {
- exists(SsaVariable schemaVar |
- this.getLeft() = schemaVar.getVariable().getALoad() and
- concatAndCompareAgainstFullHostRegex(schemaVar
- .getDefinition()
- .getImmediateDominator()
- .getNode(), this.getRight())
- )
- }
-}
-
-// "ldap://" + host
-class LDAPStringVar extends BinaryExpr {
- LDAPStringVar() {
- exists(SsaVariable hostVar |
- this.getRight() = hostVar.getVariable().getALoad() and
- concatAndCompareAgainstFullHostRegex(this.getLeft(),
- hostVar.getDefinition().getImmediateDominator().getNode())
- )
- }
-}
-
-class LDAPInsecureAuthSource extends DataFlow::Node {
- LDAPInsecureAuthSource() {
- this instanceof RemoteFlowSource or
- this.asExpr() instanceof LDAPBothStrings or
- this.asExpr() instanceof LDAPBothVar or
- this.asExpr() instanceof LDAPVarString or
- this.asExpr() instanceof LDAPStringVar
- }
-}
-
-class SafeLDAPOptions extends ControlFlowNode {
- SafeLDAPOptions() {
- this = Value::named("ldap.OPT_X_TLS_ALLOW").getAReference() or
- this = Value::named("ldap.OPT_X_TLS_TRY").getAReference() or
- this = Value::named("ldap.OPT_X_TLS_DEMAND").getAReference() or
- this = Value::named("ldap.OPT_X_TLS_HARD").getAReference()
- }
-}
-
-// LDAP3
-class LDAPInsecureAuthSink extends DataFlow::Node {
- LDAPInsecureAuthSink() {
- exists(SsaVariable connVar, CallNode connCall, SsaVariable srvVar, CallNode srvCall |
- // set connCall as a Call to ldap3.Connection
- connCall = Value::named("ldap3.Connection").getACall() and
- // get variable whose definition is a call to ldap3.Connection to correlate ldap3.Server and Connection.start_tls()
- connVar.getDefinition().getImmediateDominator() = connCall and
- // get connCall's first argument variable definition
- srvVar.getAUse() = connCall.getArg(0) and
- /*
- * // restrict srvVar definition to a ldap3.Server Call
- * srvCall = Value::named("ldap3.Server").getACall() and
- * srvVar.getDefinition().getImmediateDominator() = srvCall
- * // redundant? ldap3.Connection's first argument *must* be ldap3.Server
- */
-
- // set srvCall as srvVar definition's call
- srvVar.getDefinition().getImmediateDominator() = srvCall and
- // set ldap3.Server's 1st argument as sink
- this.asExpr() = srvCall.getArg(0).getNode() and
- (
- // check ldap3.Server call's 3rd argument (positional) is null and there's no use_ssl
- count(srvCall.getAnArg()) < 3 and
- count(srvCall.getArgByName("use_ssl")) = 0
- or
- // check ldap3.Server call's 3rd argument is False
- srvCall.getAnArg() instanceof FalseArg
- or
- // check ldap3.Server argByName "use_ssl" is False
- srvCall.getArgByName("use_ssl") instanceof FalseArg
- ) and
- /*
- * Avoid flow through any function (Server()) whose variable declaring it (srv) is the first
- * argument in any function (Connection()) whose variable declaring it also calls .start_tls
- */
-
- /*
- * host = schema + "somethingon.theinternet.com"
- * srv = Server(host, port = 1337)
- * conn = Connection(srv, "dn", "password")
- * conn.start_tls() !
- */
-
- not connVar
- .getAUse()
- .getImmediateDominator()
- .(CallNode)
- .getNode()
- .getFunc()
- .(Attribute)
- .getName()
- .matches("start_tls")
- )
- }
-}
-
-class LDAPInsecureAuthConfig extends TaintTracking::Configuration {
- LDAPInsecureAuthConfig() { this = "LDAPInsecureAuthConfig" }
-
- override predicate isSource(DataFlow::Node source) { source instanceof LDAPInsecureAuthSource }
-
- override predicate isSink(DataFlow::Node sink) { sink instanceof LDAPInsecureAuthSink }
-}
-
-from LDAPInsecureAuthConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
-where config.hasFlowPath(source, sink)
-select sink.getNode(), source, sink, "$@ from $@ is authenticated insecurely.", source.getNode(),
- "The host", sink.getNode(), "this LDAP query"
diff --git a/python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.ql b/python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.ql
new file mode 100644
index 000000000000..9899eae932f8
--- /dev/null
+++ b/python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.ql
@@ -0,0 +1,20 @@
+/**
+ * @name Python Insecure LDAP Authentication
+ * @description Python LDAP Insecure LDAP Authentication
+ * @kind path-problem
+ * @problem.severity error
+ * @id python/insecure-ldap-auth
+ * @tags experimental
+ * security
+ * external/cwe/cwe-522
+ */
+
+// determine precision above
+import python
+import DataFlow::PathGraph
+import experimental.semmle.python.security.LDAPInsecureAuth
+
+from LDAPInsecureAuthConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "$@ is authenticated insecurely.", sink.getNode(),
+ "This LDAP host"
diff --git a/python/ql/src/experimental/semmle/python/Concepts.qll b/python/ql/src/experimental/semmle/python/Concepts.qll
index 42af8abd64af..b848cb15381f 100644
--- a/python/ql/src/experimental/semmle/python/Concepts.qll
+++ b/python/ql/src/experimental/semmle/python/Concepts.qll
@@ -156,10 +156,20 @@ module LDAPBind {
* extend `LDAPBind` instead.
*/
abstract class Range extends DataFlow::Node {
+ /**
+ * Gets the argument containing the binding host.
+ */
+ abstract DataFlow::Node getHost();
+
/**
* Gets the argument containing the binding expression.
*/
abstract DataFlow::Node getPassword();
+
+ /**
+ * Checks if the binding process use SSL.
+ */
+ abstract predicate useSSL();
}
}
@@ -174,5 +184,18 @@ class LDAPBind extends DataFlow::Node {
LDAPBind() { this = range }
+ /**
+ * Gets the argument containing the binding host.
+ */
+ DataFlow::Node getHost() { result = range.getHost() }
+
+ /**
+ * Gets the argument containing the binding expression.
+ */
DataFlow::Node getPassword() { result = range.getPassword() }
+
+ /**
+ * Checks if the binding process use SSL.
+ */
+ predicate useSSL() { range.useSSL() }
}
diff --git a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
index 9d115959f19b..ed2ba3a5817e 100644
--- a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
+++ b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
@@ -99,6 +99,42 @@ private module LDAP {
override DataFlow::Node getPassword() {
result in [this.getArg(1), this.getArgByName("cred")]
}
+
+ override DataFlow::Node getHost() {
+ exists(DataFlow::CallCfgNode initialize |
+ this.getFunction().(DataFlow::AttrRead).getObject().getALocalSource() = initialize and
+ initialize = ldapInitialize().getACall() and
+ result = initialize.getArg(0)
+ )
+ }
+
+ override predicate useSSL() {
+ // use initialize to correlate `this` and so avoid FP in several instances
+ exists(DataFlow::CallCfgNode initialize |
+ this.getFunction().(DataFlow::AttrRead).getObject().getALocalSource() = initialize and
+ initialize = ldapInitialize().getACall() and
+ (
+ // ldap_connection.start_tls_s()
+ exists(DataFlow::AttrRead startTLS |
+ startTLS.getObject().getALocalSource() = initialize and
+ startTLS.getAttributeName().matches("%start_tls%")
+ )
+ or
+ // ldap_connection.set_option(ldap.OPT_X_TLS_%s, True)
+ // ldap_connection.set_option(ldap.OPT_X_TLS_%s)
+ exists(DataFlow::CallCfgNode setOption |
+ setOption.getFunction().(DataFlow::AttrRead).getObject().getALocalSource() =
+ initialize and
+ setOption.getFunction().(DataFlow::AttrRead).getAttributeName() = "set_option" and
+ setOption.getArg(0) =
+ ldap().getMember("OPT_X_TLS_" + ["ALLOW", "TRY", "DEMAND", "HARD"]).getAUse() and
+ not DataFlow::exprNode(any(False falseExpr))
+ .(DataFlow::LocalSourceNode)
+ .flowsTo(setOption.getArg(1))
+ )
+ )
+ )
+ }
}
/**
@@ -166,6 +202,24 @@ private module LDAP {
override DataFlow::Node getPassword() {
result in [this.getArg(2), this.getArgByName("password")]
}
+
+ override DataFlow::Node getHost() {
+ exists(DataFlow::CallCfgNode serverCall |
+ serverCall = ldap3Server().getACall() and
+ this.getArg(0).getALocalSource() = serverCall and
+ result = serverCall.getArg(0)
+ )
+ }
+
+ override predicate useSSL() {
+ exists(DataFlow::CallCfgNode serverCall |
+ serverCall = ldap3Server().getACall() and
+ this.getArg(0).getALocalSource() = serverCall and
+ DataFlow::exprNode(any(True trueExpr))
+ .(DataFlow::LocalSourceNode)
+ .flowsTo([serverCall.getArg(2), serverCall.getArgByName("use_ssl")])
+ )
+ }
}
/**
diff --git a/python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll b/python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll
new file mode 100644
index 000000000000..53b3745cc62a
--- /dev/null
+++ b/python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll
@@ -0,0 +1,108 @@
+/**
+ * Provides a taint-tracking configuration for detecting LDAP injection vulnerabilities
+ */
+
+import python
+import semmle.python.dataflow.new.DataFlow
+import semmle.python.dataflow.new.TaintTracking
+import semmle.python.dataflow.new.RemoteFlowSources
+import experimental.semmle.python.Concepts
+
+string getFullHostRegex() { result = "(?i)ldap://[\\[a-zA-Z0-9].*" }
+
+string getSchemaRegex() { result = "(?i)ldap(://)?" }
+
+string getPrivateHostRegex() {
+ result =
+ "(?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\\]?(?:[:/?#].*)?"
+}
+
+// "ldap://somethingon.theinternet.com"
+class LDAPFullHost extends StrConst {
+ LDAPFullHost() {
+ exists(string s |
+ s = this.getText() and
+ s.regexpMatch(getFullHostRegex()) and
+ not s.substring(7, s.length()).regexpMatch(getPrivateHostRegex()) // No need to check for ldaps, it would be SSL by default.
+ )
+ }
+}
+
+class LDAPSchema extends StrConst {
+ LDAPSchema() { this.getText().regexpMatch(getSchemaRegex()) }
+}
+
+class LDAPPrivateHost extends StrConst {
+ LDAPPrivateHost() { this.getText().regexpMatch(getPrivateHostRegex()) }
+}
+
+predicate concatAndCompareAgainstFullHostRegex(Expr schema, Expr host) {
+ schema instanceof LDAPSchema and
+ not host instanceof LDAPPrivateHost and
+ exists(string full_host |
+ full_host = schema.(StrConst).getText() + host.(StrConst).getText() and
+ full_host.regexpMatch(getFullHostRegex())
+ )
+}
+
+// "ldap://" + "somethingon.theinternet.com"
+class LDAPBothStrings extends BinaryExpr {
+ LDAPBothStrings() { concatAndCompareAgainstFullHostRegex(this.getLeft(), this.getRight()) }
+}
+
+// schema + host
+class LDAPBothVar extends BinaryExpr {
+ LDAPBothVar() {
+ exists(SsaVariable schemaVar, SsaVariable hostVar |
+ this.getLeft() = schemaVar.getVariable().getALoad() and // getAUse is incompatible with Expr
+ this.getRight() = hostVar.getVariable().getALoad() and
+ concatAndCompareAgainstFullHostRegex(schemaVar
+ .getDefinition()
+ .getImmediateDominator()
+ .getNode(), hostVar.getDefinition().getImmediateDominator().getNode())
+ )
+ }
+}
+
+// schema + "somethingon.theinternet.com"
+class LDAPVarString extends BinaryExpr {
+ LDAPVarString() {
+ exists(SsaVariable schemaVar |
+ this.getLeft() = schemaVar.getVariable().getALoad() and
+ concatAndCompareAgainstFullHostRegex(schemaVar
+ .getDefinition()
+ .getImmediateDominator()
+ .getNode(), this.getRight())
+ )
+ }
+}
+
+// "ldap://" + host
+class LDAPStringVar extends BinaryExpr {
+ LDAPStringVar() {
+ exists(SsaVariable hostVar |
+ this.getRight() = hostVar.getVariable().getALoad() and
+ concatAndCompareAgainstFullHostRegex(this.getLeft(),
+ hostVar.getDefinition().getImmediateDominator().getNode())
+ )
+ }
+}
+
+/**
+ * A taint-tracking configuration for detecting LDAP injections.
+ */
+class LDAPInsecureAuthConfig extends TaintTracking::Configuration {
+ LDAPInsecureAuthConfig() { this = "LDAPInsecureAuthConfig" }
+
+ override predicate isSource(DataFlow::Node source) {
+ source instanceof RemoteFlowSource or
+ source.asExpr() instanceof LDAPBothStrings or
+ source.asExpr() instanceof LDAPBothVar or
+ source.asExpr() instanceof LDAPVarString or
+ source.asExpr() instanceof LDAPStringVar
+ }
+
+ override predicate isSink(DataFlow::Node sink) {
+ exists(LDAPBind ldapBind | not ldapBind.useSSL() and sink = ldapBind.getHost())
+ }
+}
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.expected b/python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.expected
new file mode 100644
index 000000000000..fdcb414b9814
--- /dev/null
+++ b/python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.expected
@@ -0,0 +1,39 @@
+edges
+| ldap3_remote.py:49:12:49:34 | ControlFlowNode for BinaryExpr | ldap3_remote.py:51:18:51:21 | ControlFlowNode for host |
+| ldap3_remote.py:88:21:88:27 | ControlFlowNode for request | ldap3_remote.py:88:21:88:32 | ControlFlowNode for Attribute |
+| ldap3_remote.py:88:21:88:32 | ControlFlowNode for Attribute | ldap3_remote.py:88:21:88:40 | ControlFlowNode for Subscript |
+| ldap3_remote.py:88:21:88:40 | ControlFlowNode for Subscript | ldap3_remote.py:90:18:90:21 | ControlFlowNode for host |
+| ldap3_remote.py:101:12:101:49 | ControlFlowNode for BinaryExpr | ldap3_remote.py:102:18:102:21 | ControlFlowNode for host |
+| ldap3_remote.py:114:12:114:49 | ControlFlowNode for BinaryExpr | ldap3_remote.py:115:18:115:21 | ControlFlowNode for host |
+| ldap3_remote.py:126:12:126:31 | ControlFlowNode for BinaryExpr | ldap3_remote.py:127:18:127:21 | ControlFlowNode for host |
+| ldap3_remote.py:138:21:138:27 | ControlFlowNode for request | ldap3_remote.py:138:21:138:32 | ControlFlowNode for Attribute |
+| ldap3_remote.py:138:21:138:32 | ControlFlowNode for Attribute | ldap3_remote.py:138:21:138:40 | ControlFlowNode for Subscript |
+| ldap3_remote.py:138:21:138:40 | ControlFlowNode for Subscript | ldap3_remote.py:139:18:139:21 | ControlFlowNode for host |
+nodes
+| ldap2_remote.py:45:41:45:60 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
+| ldap2_remote.py:56:41:56:60 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
+| ldap3_remote.py:49:12:49:34 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
+| ldap3_remote.py:51:18:51:21 | ControlFlowNode for host | semmle.label | ControlFlowNode for host |
+| ldap3_remote.py:88:21:88:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
+| ldap3_remote.py:88:21:88:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| ldap3_remote.py:88:21:88:40 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
+| ldap3_remote.py:90:18:90:21 | ControlFlowNode for host | semmle.label | ControlFlowNode for host |
+| ldap3_remote.py:101:12:101:49 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
+| ldap3_remote.py:102:18:102:21 | ControlFlowNode for host | semmle.label | ControlFlowNode for host |
+| ldap3_remote.py:114:12:114:49 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
+| ldap3_remote.py:115:18:115:21 | ControlFlowNode for host | semmle.label | ControlFlowNode for host |
+| ldap3_remote.py:126:12:126:31 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
+| ldap3_remote.py:127:18:127:21 | ControlFlowNode for host | semmle.label | ControlFlowNode for host |
+| ldap3_remote.py:138:21:138:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
+| ldap3_remote.py:138:21:138:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| ldap3_remote.py:138:21:138:40 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
+| ldap3_remote.py:139:18:139:21 | ControlFlowNode for host | semmle.label | ControlFlowNode for host |
+#select
+| 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 |
+| 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 |
+| ldap3_remote.py:51:18:51:21 | ControlFlowNode for host | ldap3_remote.py:49:12:49:34 | ControlFlowNode for BinaryExpr | ldap3_remote.py:51:18:51:21 | ControlFlowNode for host | $@ is authenticated insecurely. | ldap3_remote.py:51:18:51:21 | ControlFlowNode for host | This LDAP host |
+| ldap3_remote.py:90:18:90:21 | ControlFlowNode for host | ldap3_remote.py:88:21:88:27 | ControlFlowNode for request | ldap3_remote.py:90:18:90:21 | ControlFlowNode for host | $@ is authenticated insecurely. | ldap3_remote.py:90:18:90:21 | ControlFlowNode for host | This LDAP host |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.qlref b/python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.qlref
new file mode 100644
index 000000000000..8bb2c1e9b52a
--- /dev/null
+++ b/python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.qlref
@@ -0,0 +1 @@
+experimental/Security/CWE-522/LDAPInsecureAuth.ql
\ No newline at end of file
diff --git a/python/ql/src/experimental/Security/CWE-090/tests/2_private.py b/python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_private.py
similarity index 100%
rename from python/ql/src/experimental/Security/CWE-090/tests/2_private.py
rename to python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_private.py
diff --git a/python/ql/src/experimental/Security/CWE-090/tests/2_remote.py b/python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_remote.py
similarity index 100%
rename from python/ql/src/experimental/Security/CWE-090/tests/2_remote.py
rename to python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_remote.py
diff --git a/python/ql/src/experimental/Security/CWE-090/tests/3_private.py b/python/ql/test/experimental/query-tests/Security/CWE-522/ldap3_private.py
similarity index 98%
rename from python/ql/src/experimental/Security/CWE-090/tests/3_private.py
rename to python/ql/test/experimental/query-tests/Security/CWE-522/ldap3_private.py
index 7c4ede3f0e61..796ecbd01b02 100644
--- a/python/ql/src/experimental/Security/CWE-090/tests/3_private.py
+++ b/python/ql/test/experimental/query-tests/Security/CWE-522/ldap3_private.py
@@ -83,7 +83,7 @@ def six():
def four():
host = "ldap://" + partial_host
- srv = Server(host, port=1337, True)
+ srv = Server(host, 1337, True)
conn = Connection(srv, "dn", "password")
conn.search("dn", "search_filter")
return conn.response
diff --git a/python/ql/src/experimental/Security/CWE-090/tests/3_remote.py b/python/ql/test/experimental/query-tests/Security/CWE-522/ldap3_remote.py
similarity index 100%
rename from python/ql/src/experimental/Security/CWE-090/tests/3_remote.py
rename to python/ql/test/experimental/query-tests/Security/CWE-522/ldap3_remote.py
From b03e75e3d1920135dbd396ee53acfe340a1e13bd Mon Sep 17 00:00:00 2001
From: jorgectf
Date: Thu, 22 Jul 2021 18:42:41 +0200
Subject: [PATCH 05/18] Extend `ldap3`'s `start_tls` and fix tests
---
.../ql/src/experimental/semmle/python/frameworks/LDAP.qll | 5 +++++
.../query-tests/Security/CWE-522/ldap3_remote.py | 6 +++---
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
index ed2ba3a5817e..b36897deea73 100644
--- a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
+++ b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
@@ -219,6 +219,11 @@ private module LDAP {
.(DataFlow::LocalSourceNode)
.flowsTo([serverCall.getArg(2), serverCall.getArgByName("use_ssl")])
)
+ or
+ exists(DataFlow::AttrRead startTLS |
+ startTLS.getAttributeName().matches("%start_tls%") and
+ startTLS.getObject().getALocalSource() = this
+ )
}
}
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-522/ldap3_remote.py b/python/ql/test/experimental/query-tests/Security/CWE-522/ldap3_remote.py
index 366fce2b186e..09dadcfb0e0f 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-522/ldap3_remote.py
+++ b/python/ql/test/experimental/query-tests/Security/CWE-522/ldap3_remote.py
@@ -48,7 +48,7 @@ def three():
def four():
host = "ldap://" + remote_host
- srv = Server(host, port=1337, True)
+ srv = Server(host, 1337, True)
conn = Connection(srv, "dn", "password")
conn.search("dn", "search_filter")
return conn.response
@@ -87,7 +87,7 @@ def six():
def seven():
host = schema + request.args['host']
- srv = Server(host, port=1337, True)
+ srv = Server(host, 1337, True)
conn = Connection(srv, "dn", "password")
conn.search("dn", "search_filter")
return conn.response
@@ -112,7 +112,7 @@ def eight():
@app.route("/nine")
def nine():
host = schema + "somethingon.theinternet.com"
- srv = Server(host, port=1337, False)
+ srv = Server(host, 1337, False)
conn = Connection(srv, "dn", "password")
conn.search("dn", "search_filter")
return conn.response
From d458464e6b7a0563a3343fdc68b92ac118d7b95c Mon Sep 17 00:00:00 2001
From: Jorge <46056498+jorgectf@users.noreply.github.com>
Date: Thu, 26 Aug 2021 12:20:09 +0200
Subject: [PATCH 06/18] Apply suggestions from code review
Co-authored-by: Rasmus Wriedt Larsen
---
.../ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.ql | 1 +
python/ql/src/experimental/semmle/python/Concepts.qll | 4 ++--
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.ql b/python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.ql
index 9899eae932f8..5daaae5ff39f 100644
--- a/python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.ql
+++ b/python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.ql
@@ -7,6 +7,7 @@
* @tags experimental
* security
* external/cwe/cwe-522
+ * external/cwe/cwe-523
*/
// determine precision above
diff --git a/python/ql/src/experimental/semmle/python/Concepts.qll b/python/ql/src/experimental/semmle/python/Concepts.qll
index 3daacf539817..bdabebb804d2 100644
--- a/python/ql/src/experimental/semmle/python/Concepts.qll
+++ b/python/ql/src/experimental/semmle/python/Concepts.qll
@@ -167,7 +167,7 @@ module LDAPBind {
abstract DataFlow::Node getPassword();
/**
- * Checks if the binding process use SSL.
+ * Holds if the binding process use SSL.
*/
abstract predicate useSSL();
}
@@ -195,7 +195,7 @@ class LDAPBind extends DataFlow::Node {
DataFlow::Node getPassword() { result = range.getPassword() }
/**
- * Checks if the binding process use SSL.
+ * Holds if the binding process use SSL.
*/
predicate useSSL() { range.useSSL() }
}
From 786edb72df5537b5a77df9d54057e47563f237e2 Mon Sep 17 00:00:00 2001
From: jorgectf
Date: Thu, 26 Aug 2021 12:36:34 +0200
Subject: [PATCH 07/18] Update `.expected`
---
.../Security/CWE-522/LDAPInsecureAuth.expected | 16 ----------------
1 file changed, 16 deletions(-)
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.expected b/python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.expected
index fdcb414b9814..13b5ba7b7b7b 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.expected
+++ b/python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.expected
@@ -1,9 +1,4 @@
edges
-| ldap3_remote.py:49:12:49:34 | ControlFlowNode for BinaryExpr | ldap3_remote.py:51:18:51:21 | ControlFlowNode for host |
-| ldap3_remote.py:88:21:88:27 | ControlFlowNode for request | ldap3_remote.py:88:21:88:32 | ControlFlowNode for Attribute |
-| ldap3_remote.py:88:21:88:32 | ControlFlowNode for Attribute | ldap3_remote.py:88:21:88:40 | ControlFlowNode for Subscript |
-| ldap3_remote.py:88:21:88:40 | ControlFlowNode for Subscript | ldap3_remote.py:90:18:90:21 | ControlFlowNode for host |
-| ldap3_remote.py:101:12:101:49 | ControlFlowNode for BinaryExpr | ldap3_remote.py:102:18:102:21 | ControlFlowNode for host |
| ldap3_remote.py:114:12:114:49 | ControlFlowNode for BinaryExpr | ldap3_remote.py:115:18:115:21 | ControlFlowNode for host |
| ldap3_remote.py:126:12:126:31 | ControlFlowNode for BinaryExpr | ldap3_remote.py:127:18:127:21 | ControlFlowNode for host |
| ldap3_remote.py:138:21:138:27 | ControlFlowNode for request | ldap3_remote.py:138:21:138:32 | ControlFlowNode for Attribute |
@@ -12,14 +7,6 @@ edges
nodes
| ldap2_remote.py:45:41:45:60 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
| ldap2_remote.py:56:41:56:60 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
-| ldap3_remote.py:49:12:49:34 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
-| ldap3_remote.py:51:18:51:21 | ControlFlowNode for host | semmle.label | ControlFlowNode for host |
-| ldap3_remote.py:88:21:88:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
-| ldap3_remote.py:88:21:88:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
-| ldap3_remote.py:88:21:88:40 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
-| ldap3_remote.py:90:18:90:21 | ControlFlowNode for host | semmle.label | ControlFlowNode for host |
-| ldap3_remote.py:101:12:101:49 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
-| ldap3_remote.py:102:18:102:21 | ControlFlowNode for host | semmle.label | ControlFlowNode for host |
| ldap3_remote.py:114:12:114:49 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
| ldap3_remote.py:115:18:115:21 | ControlFlowNode for host | semmle.label | ControlFlowNode for host |
| ldap3_remote.py:126:12:126:31 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
@@ -31,9 +18,6 @@ nodes
#select
| 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 |
| 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 |
-| ldap3_remote.py:51:18:51:21 | ControlFlowNode for host | ldap3_remote.py:49:12:49:34 | ControlFlowNode for BinaryExpr | ldap3_remote.py:51:18:51:21 | ControlFlowNode for host | $@ is authenticated insecurely. | ldap3_remote.py:51:18:51:21 | ControlFlowNode for host | This LDAP host |
-| ldap3_remote.py:90:18:90:21 | ControlFlowNode for host | ldap3_remote.py:88:21:88:27 | ControlFlowNode for request | ldap3_remote.py:90:18:90:21 | ControlFlowNode for host | $@ is authenticated insecurely. | ldap3_remote.py:90:18:90:21 | ControlFlowNode for host | This LDAP host |
-| 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 |
| 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 |
| 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 |
| 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 |
From 64b305cf7a7c984737c90777dcdf72a7cec7d80c Mon Sep 17 00:00:00 2001
From: jorgectf
Date: Thu, 26 Aug 2021 23:29:45 +0200
Subject: [PATCH 08/18] Add `.qhelp` along with its example
---
.../Security/CWE-522/LDAPInsecureAuth.qhelp | 23 +++++++++++++++++++
.../CWE-522/examples/LDAPInsecureAuth.py | 20 ++++++++++++++++
.../Security/CWE-522/ldap3_remote.py | 2 +-
3 files changed, 44 insertions(+), 1 deletion(-)
create mode 100644 python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.qhelp
create mode 100644 python/ql/src/experimental/Security/CWE-522/examples/LDAPInsecureAuth.py
diff --git a/python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.qhelp b/python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.qhelp
new file mode 100644
index 000000000000..cd568432d730
--- /dev/null
+++ b/python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.qhelp
@@ -0,0 +1,23 @@
+
+
+
+
+Failing to ensure the utilization of SSL in an LDAP connection can cause the entire communication
+to be sent in cleartext making it easier for an attacker to intercept it.
+
+
+
+Always set use_SSL
to True
, call start_tls_s()
or set a proper option flag (ldap.OPT_X_TLS_XXXXXX
).
+
+
+
+This example shows both good and bad ways to deal with this issue under Python 3.
+
+The first one sets use_SSL
to true as a keyword argument whereas the second one fails to provide a value for it, so
+the default one is used (False
).
+
+
+
+
diff --git a/python/ql/src/experimental/Security/CWE-522/examples/LDAPInsecureAuth.py b/python/ql/src/experimental/Security/CWE-522/examples/LDAPInsecureAuth.py
new file mode 100644
index 000000000000..051ca07b0dcf
--- /dev/null
+++ b/python/ql/src/experimental/Security/CWE-522/examples/LDAPInsecureAuth.py
@@ -0,0 +1,20 @@
+from ldap3 import Server, Connection, ALL
+from flask import request, Flask
+
+app = Flask(__name__)
+
+
+@app.route("/good")
+def good():
+ srv = Server(host, port, use_ssl=True)
+ conn = Connection(srv, dn, password)
+ conn.search(dn, search_filter)
+ return conn.response
+
+
+@app.route("/bad")
+def bad():
+ srv = Server(host, port)
+ conn = Connection(srv, dn, password)
+ conn.search(dn, search_filter)
+ return conn.response
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-522/ldap3_remote.py b/python/ql/test/experimental/query-tests/Security/CWE-522/ldap3_remote.py
index 09dadcfb0e0f..dc0ef9cf9773 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-522/ldap3_remote.py
+++ b/python/ql/test/experimental/query-tests/Security/CWE-522/ldap3_remote.py
@@ -101,7 +101,7 @@ def eight():
host = schema + "somethingon.theinternet.com"
srv = Server(host, port=1337)
conn = Connection(srv, "dn", "password")
- conn.start_tls() # !
+ conn.start_tls()
conn.search("dn", "search_filter")
return conn.response
From 1bc16fb31e6c2e02414e63c5a65a83194f79042a Mon Sep 17 00:00:00 2001
From: Jorge <46056498+jorgectf@users.noreply.github.com>
Date: Tue, 7 Sep 2021 18:37:33 +0200
Subject: [PATCH 09/18] Apply suggestions from code review
Co-authored-by: Rasmus Wriedt Larsen
---
.../experimental/Security/CWE-522/LDAPInsecureAuth.qhelp | 2 +-
.../src/experimental/Security/CWE-522/LDAPInsecureAuth.ql | 2 +-
.../ql/src/experimental/semmle/python/frameworks/LDAP.qll | 3 ++-
.../semmle/python/security/LDAPInsecureAuth.qll | 6 ++++--
4 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.qhelp b/python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.qhelp
index cd568432d730..9033697fd599 100644
--- a/python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.qhelp
+++ b/python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.qhelp
@@ -17,7 +17,7 @@ to be sent in cleartext making it easier for an attacker to intercept it.
The first one sets use_SSL
to true as a keyword argument whereas the second one fails to provide a value for it, so
the default one is used (False
).
-
+
diff --git a/python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.ql b/python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.ql
index 5daaae5ff39f..9f99527a3e3e 100644
--- a/python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.ql
+++ b/python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.ql
@@ -3,7 +3,7 @@
* @description Python LDAP Insecure LDAP Authentication
* @kind path-problem
* @problem.severity error
- * @id python/insecure-ldap-auth
+ * @id py/insecure-ldap-auth
* @tags experimental
* security
* external/cwe/cwe-522
diff --git a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
index d821fdc6425f..1aa30350fb87 100644
--- a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
+++ b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
@@ -115,9 +115,10 @@ private module LDAP {
initialize = ldapInitialize().getACall() and
(
// ldap_connection.start_tls_s()
+ // see https://www.python-ldap.org/en/python-ldap-3.3.0/reference/ldap.html#ldap.LDAPObject.start_tls_s
exists(DataFlow::AttrRead startTLS |
startTLS.getObject().getALocalSource() = initialize and
- startTLS.getAttributeName().matches("%start_tls%")
+ startTLS.getAttributeName() = "start_tls_s"
)
or
// ldap_connection.set_option(ldap.OPT_X_TLS_%s, True)
diff --git a/python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll b/python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll
index 53b3745cc62a..aada8b96ce1e 100644
--- a/python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll
+++ b/python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll
@@ -23,7 +23,8 @@ class LDAPFullHost extends StrConst {
exists(string s |
s = this.getText() and
s.regexpMatch(getFullHostRegex()) and
- not s.substring(7, s.length()).regexpMatch(getPrivateHostRegex()) // No need to check for ldaps, it would be SSL by default.
+ // check what comes after the `ldap://` prefix
+ not s.substring(7, s.length()).regexpMatch(getPrivateHostRegex())
)
}
}
@@ -36,7 +37,7 @@ class LDAPPrivateHost extends StrConst {
LDAPPrivateHost() { this.getText().regexpMatch(getPrivateHostRegex()) }
}
-predicate concatAndCompareAgainstFullHostRegex(Expr schema, Expr host) {
+predicate concatAndCompareAgainstFullHostRegex(LDAPSchema schema, StrConst host) {
schema instanceof LDAPSchema and
not host instanceof LDAPPrivateHost and
exists(string full_host |
@@ -96,6 +97,7 @@ class LDAPInsecureAuthConfig extends TaintTracking::Configuration {
override predicate isSource(DataFlow::Node source) {
source instanceof RemoteFlowSource or
+ source.asExpr() instanceof LDAPFullHost or
source.asExpr() instanceof LDAPBothStrings or
source.asExpr() instanceof LDAPBothVar or
source.asExpr() instanceof LDAPVarString or
From ee98c0c587267fd433c3045e66bc3944cb378a29 Mon Sep 17 00:00:00 2001
From: jorgectf
Date: Tue, 7 Sep 2021 19:00:14 +0200
Subject: [PATCH 10/18] Add `start_tls_s()` comment and use
`DataFlow::MethodCallNode` instead
---
.../src/experimental/semmle/python/frameworks/LDAP.qll | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
index 1aa30350fb87..adfb2ad7a2dd 100644
--- a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
+++ b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
@@ -116,9 +116,9 @@ private module LDAP {
(
// ldap_connection.start_tls_s()
// see https://www.python-ldap.org/en/python-ldap-3.3.0/reference/ldap.html#ldap.LDAPObject.start_tls_s
- exists(DataFlow::AttrRead startTLS |
+ exists(DataFlow::MethodCallNode startTLS |
startTLS.getObject().getALocalSource() = initialize and
- startTLS.getAttributeName() = "start_tls_s"
+ startTLS.getMethodName() = "start_tls_s"
)
or
// ldap_connection.set_option(ldap.OPT_X_TLS_%s, True)
@@ -221,8 +221,10 @@ private module LDAP {
.flowsTo([serverCall.getArg(2), serverCall.getArgByName("use_ssl")])
)
or
- exists(DataFlow::AttrRead startTLS |
- startTLS.getAttributeName().matches("%start_tls%") and
+ // ldap_connection.start_tls_s()
+ // see https://www.python-ldap.org/en/python-ldap-3.3.0/reference/ldap.html#ldap.LDAPObject.start_tls_s
+ exists(DataFlow::MethodCallNode startTLS |
+ startTLS.getMethodName() = "start_tls_s" and
startTLS.getObject().getALocalSource() = this
)
}
From b802d7903ab9bcdbc25d3eed82a33d171991395d Mon Sep 17 00:00:00 2001
From: jorgectf
Date: Tue, 7 Sep 2021 19:01:46 +0200
Subject: [PATCH 11/18] Fix `OPT_X_TLS_` mandatory options
---
python/ql/src/experimental/semmle/python/frameworks/LDAP.qll | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
index adfb2ad7a2dd..a9137d3156f9 100644
--- a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
+++ b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
@@ -122,13 +122,11 @@ private module LDAP {
)
or
// ldap_connection.set_option(ldap.OPT_X_TLS_%s, True)
- // ldap_connection.set_option(ldap.OPT_X_TLS_%s)
exists(DataFlow::CallCfgNode setOption |
setOption.getFunction().(DataFlow::AttrRead).getObject().getALocalSource() =
initialize and
setOption.getFunction().(DataFlow::AttrRead).getAttributeName() = "set_option" and
- setOption.getArg(0) =
- ldap().getMember("OPT_X_TLS_" + ["ALLOW", "TRY", "DEMAND", "HARD"]).getAUse() and
+ setOption.getArg(0) = ldap().getMember("OPT_X_TLS_" + ["DEMAND", "HARD"]).getAUse() and
not DataFlow::exprNode(any(False falseExpr))
.(DataFlow::LocalSourceNode)
.flowsTo(setOption.getArg(1))
From 800801177db14724a32d65bafbe669af981c066a Mon Sep 17 00:00:00 2001
From: jorgectf
Date: Tue, 7 Sep 2021 19:02:32 +0200
Subject: [PATCH 12/18] Fix taint tracking comment
---
.../experimental/semmle/python/security/LDAPInsecureAuth.qll | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll b/python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll
index aada8b96ce1e..efeb16470394 100644
--- a/python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll
+++ b/python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll
@@ -90,7 +90,7 @@ class LDAPStringVar extends BinaryExpr {
}
/**
- * A taint-tracking configuration for detecting LDAP injections.
+ * A taint-tracking configuration for detecting LDAP insecure authentications.
*/
class LDAPInsecureAuthConfig extends TaintTracking::Configuration {
LDAPInsecureAuthConfig() { this = "LDAPInsecureAuthConfig" }
From 4e261c61aed06ddde7641a19241400dfe632026c Mon Sep 17 00:00:00 2001
From: jorgectf
Date: Tue, 7 Sep 2021 19:05:03 +0200
Subject: [PATCH 13/18] Optimize `concatAndCompareAgainstFullHostRegex`
---
.../semmle/python/security/LDAPInsecureAuth.qll | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll b/python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll
index efeb16470394..72a6baffb65c 100644
--- a/python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll
+++ b/python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll
@@ -38,12 +38,8 @@ class LDAPPrivateHost extends StrConst {
}
predicate concatAndCompareAgainstFullHostRegex(LDAPSchema schema, StrConst host) {
- schema instanceof LDAPSchema and
not host instanceof LDAPPrivateHost and
- exists(string full_host |
- full_host = schema.(StrConst).getText() + host.(StrConst).getText() and
- full_host.regexpMatch(getFullHostRegex())
- )
+ (schema.getText() + host.getText()).regexpMatch(getFullHostRegex())
}
// "ldap://" + "somethingon.theinternet.com"
From 54012eba239469fa49f59454ee0a4f52e4a403cf Mon Sep 17 00:00:00 2001
From: jorgectf
Date: Sun, 12 Sep 2021 20:13:08 +0200
Subject: [PATCH 14/18] Optimize `getFullHostRegex`
---
.../experimental/semmle/python/security/LDAPInsecureAuth.qll | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll b/python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll
index 72a6baffb65c..442a21de30f4 100644
--- a/python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll
+++ b/python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll
@@ -8,7 +8,7 @@ import semmle.python.dataflow.new.TaintTracking
import semmle.python.dataflow.new.RemoteFlowSources
import experimental.semmle.python.Concepts
-string getFullHostRegex() { result = "(?i)ldap://[\\[a-zA-Z0-9].*" }
+string getFullHostRegex() { result = "(?i)ldap://.+" }
string getSchemaRegex() { result = "(?i)ldap(://)?" }
From 18b05bc56eedc4fbb894aabaa224a265fe1863f9 Mon Sep 17 00:00:00 2001
From: jorgectf
Date: Sun, 12 Sep 2021 20:35:57 +0200
Subject: [PATCH 15/18] Fix tests and add global option
---
.../semmle/python/frameworks/LDAP.qll | 9 ++++++-
.../Security/CWE-522/ldap2_global.py | 24 +++++++++++++++++++
.../Security/CWE-522/ldap2_private.py | 2 +-
.../Security/CWE-522/ldap2_remote.py | 2 +-
.../Security/CWE-522/ldap3_remote.py | 2 +-
5 files changed, 35 insertions(+), 4 deletions(-)
create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_global.py
diff --git a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
index a9137d3156f9..e84ccaa87f47 100644
--- a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
+++ b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
@@ -88,6 +88,11 @@ private module LDAP {
result.(DataFlow::AttrRead).getAttributeName() instanceof LDAP2BindMethods
}
+ /**List of SSL-demanding options */
+ private class LDAPSSLOptions extends DataFlow::Node {
+ LDAPSSLOptions() { this = ldap().getMember("OPT_X_TLS_" + ["DEMAND", "HARD"]).getAUse() }
+ }
+
/**
* A class to find `ldap` methods binding a connection.
*
@@ -111,6 +116,8 @@ private module LDAP {
override predicate useSSL() {
// use initialize to correlate `this` and so avoid FP in several instances
exists(DataFlow::CallCfgNode initialize |
+ ldap().getMember("set_option").getACall().getArg(_) instanceof LDAPSSLOptions
+ or
this.getFunction().(DataFlow::AttrRead).getObject().getALocalSource() = initialize and
initialize = ldapInitialize().getACall() and
(
@@ -126,7 +133,7 @@ private module LDAP {
setOption.getFunction().(DataFlow::AttrRead).getObject().getALocalSource() =
initialize and
setOption.getFunction().(DataFlow::AttrRead).getAttributeName() = "set_option" and
- setOption.getArg(0) = ldap().getMember("OPT_X_TLS_" + ["DEMAND", "HARD"]).getAUse() and
+ setOption.getArg(0) instanceof LDAPSSLOptions and
not DataFlow::exprNode(any(False falseExpr))
.(DataFlow::LocalSourceNode)
.flowsTo(setOption.getArg(1))
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_global.py b/python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_global.py
new file mode 100644
index 000000000000..8d0d1b6b2f39
--- /dev/null
+++ b/python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_global.py
@@ -0,0 +1,24 @@
+import ldap
+from flask import request, Flask
+
+app = Flask(__name__)
+
+# GOOD
+
+# SSL through ldap global variable option
+
+ldap.set_option(ldap.OPT_X_TLS_DEMAND)
+
+
+@app.route("/one")
+def one():
+ ldap_connection_5 = ldap.initialize("ldap://somethingon.theinternet.com")
+ ldap_connection_5.simple_bind_s('', '')
+ user = ldap_connection_5.search_s(
+ "dn", ldap.SCOPE_SUBTREE, "search_filter")
+
+ return user
+
+
+# if __name__ == "__main__":
+# app.run(debug=True)
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_private.py b/python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_private.py
index 4e152cd93c2e..834f1d7bd343 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_private.py
+++ b/python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_private.py
@@ -54,7 +54,7 @@ def one_bad():
@app.route("/one_bad_2")
def one_bad_2():
ldap_connection_4 = ldap.initialize(schema + remote_host)
- ldap_connection_4.set_option(ldap.OPT_X_TLS_NEVER)
+ ldap_connection_4.set_option(ldap.OPT_X_TLS_NEVER, True)
ldap_connection_4.simple_bind_s('', '')
user = ldap_connection_4.search_s(
"dn", ldap.SCOPE_SUBTREE, "search_filter")
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_remote.py b/python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_remote.py
index 6f82f1dbfd17..3119ca2d28a7 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_remote.py
+++ b/python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_remote.py
@@ -54,7 +54,7 @@ def one_bad():
@app.route("/one_bad_2")
def one_bad_2():
ldap_connection_8 = ldap.initialize(schema + remote_host)
- ldap_connection_8.set_option(ldap.OPT_X_TLS_NEVER)
+ ldap_connection_8.set_option(ldap.OPT_X_TLS_NEVER, True)
ldap_connection_8.simple_bind_s('', '')
user = ldap_connection_8.search_s(
"dn", ldap.SCOPE_SUBTREE, "search_filter")
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-522/ldap3_remote.py b/python/ql/test/experimental/query-tests/Security/CWE-522/ldap3_remote.py
index dc0ef9cf9773..269e03e41fef 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-522/ldap3_remote.py
+++ b/python/ql/test/experimental/query-tests/Security/CWE-522/ldap3_remote.py
@@ -13,7 +13,7 @@
@app.route("/one")
def one():
- srv = Server("ldap://somethingon.theinternet.com", port=1337, True)
+ srv = Server("ldap://somethingon.theinternet.com", 1337, True)
conn = Connection(srv, "dn", "password")
conn.search("dn", "search_filter")
return conn.response
From 353c0a9ee7da704521ef85082f1ea7afe41ffe4d Mon Sep 17 00:00:00 2001
From: jorgectf
Date: Sun, 12 Sep 2021 20:44:04 +0200
Subject: [PATCH 16/18] Add missing comment
---
python/ql/src/experimental/semmle/python/frameworks/LDAP.qll | 1 +
1 file changed, 1 insertion(+)
diff --git a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
index e84ccaa87f47..9286129cf6ee 100644
--- a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
+++ b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
@@ -116,6 +116,7 @@ private module LDAP {
override predicate useSSL() {
// use initialize to correlate `this` and so avoid FP in several instances
exists(DataFlow::CallCfgNode initialize |
+ // ldap.set_option(ldap.OPT_X_TLS_%s)
ldap().getMember("set_option").getACall().getArg(_) instanceof LDAPSSLOptions
or
this.getFunction().(DataFlow::AttrRead).getObject().getALocalSource() = initialize and
From b505662ef9878763a9512f719b00f7d2dc699b65 Mon Sep 17 00:00:00 2001
From: jorgectf
Date: Tue, 14 Sep 2021 10:20:50 +0200
Subject: [PATCH 17/18] Fix global test and update `.expected`
---
.../query-tests/Security/CWE-522/LDAPInsecureAuth.expected | 7 +++++++
.../query-tests/Security/CWE-522/ldap2_global.py | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.expected b/python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.expected
index 13b5ba7b7b7b..592ad8b99f5c 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.expected
+++ b/python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.expected
@@ -1,12 +1,16 @@
edges
+| ldap3_remote.py:101:12:101:49 | ControlFlowNode for BinaryExpr | ldap3_remote.py:102:18:102:21 | ControlFlowNode for host |
| ldap3_remote.py:114:12:114:49 | ControlFlowNode for BinaryExpr | ldap3_remote.py:115:18:115:21 | ControlFlowNode for host |
| ldap3_remote.py:126:12:126:31 | ControlFlowNode for BinaryExpr | ldap3_remote.py:127:18:127:21 | ControlFlowNode for host |
| ldap3_remote.py:138:21:138:27 | ControlFlowNode for request | ldap3_remote.py:138:21:138:32 | ControlFlowNode for Attribute |
| ldap3_remote.py:138:21:138:32 | ControlFlowNode for Attribute | ldap3_remote.py:138:21:138:40 | ControlFlowNode for Subscript |
| ldap3_remote.py:138:21:138:40 | ControlFlowNode for Subscript | ldap3_remote.py:139:18:139:21 | ControlFlowNode for host |
nodes
+| ldap2_global.py:15:41:15:76 | ControlFlowNode for Str | semmle.label | ControlFlowNode for Str |
| ldap2_remote.py:45:41:45:60 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
| ldap2_remote.py:56:41:56:60 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
+| ldap3_remote.py:101:12:101:49 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
+| ldap3_remote.py:102:18:102:21 | ControlFlowNode for host | semmle.label | ControlFlowNode for host |
| ldap3_remote.py:114:12:114:49 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
| ldap3_remote.py:115:18:115:21 | ControlFlowNode for host | semmle.label | ControlFlowNode for host |
| ldap3_remote.py:126:12:126:31 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
@@ -15,9 +19,12 @@ nodes
| ldap3_remote.py:138:21:138:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
| ldap3_remote.py:138:21:138:40 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
| ldap3_remote.py:139:18:139:21 | ControlFlowNode for host | semmle.label | ControlFlowNode for host |
+subpaths
#select
+| ldap2_global.py:15:41:15:76 | ControlFlowNode for Str | ldap2_global.py:15:41:15:76 | ControlFlowNode for Str | ldap2_global.py:15:41:15:76 | ControlFlowNode for Str | $@ is authenticated insecurely. | ldap2_global.py:15:41:15:76 | ControlFlowNode for Str | This LDAP host |
| 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 |
| 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 |
+| 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 |
| 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 |
| 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 |
| 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 |
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_global.py b/python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_global.py
index 8d0d1b6b2f39..44d528698fab 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_global.py
+++ b/python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_global.py
@@ -7,7 +7,7 @@
# SSL through ldap global variable option
-ldap.set_option(ldap.OPT_X_TLS_DEMAND)
+ldap.set_option(ldap.OPT_X_TLS_NEVER)
@app.route("/one")
From ef6e502ff068adb41c90886d874481292a4d6b55 Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen
Date: Thu, 23 Sep 2021 10:18:18 +0200
Subject: [PATCH 18/18] Python: Make LDAP global options test better
Before it didn't really showcase that we know it can make connections
secure.
---
.../CWE-522-global-option/LDAPInsecureAuth.expected | 4 ++++
.../Security/CWE-522-global-option/LDAPInsecureAuth.qlref | 1 +
.../{CWE-522 => CWE-522-global-option}/ldap2_global.py | 7 ++++++-
.../query-tests/Security/CWE-522/LDAPInsecureAuth.expected | 2 --
4 files changed, 11 insertions(+), 3 deletions(-)
create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-522-global-option/LDAPInsecureAuth.expected
create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-522-global-option/LDAPInsecureAuth.qlref
rename python/ql/test/experimental/query-tests/Security/{CWE-522 => CWE-522-global-option}/ldap2_global.py (58%)
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-522-global-option/LDAPInsecureAuth.expected b/python/ql/test/experimental/query-tests/Security/CWE-522-global-option/LDAPInsecureAuth.expected
new file mode 100644
index 000000000000..e217064d1dfc
--- /dev/null
+++ b/python/ql/test/experimental/query-tests/Security/CWE-522-global-option/LDAPInsecureAuth.expected
@@ -0,0 +1,4 @@
+edges
+nodes
+subpaths
+#select
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-522-global-option/LDAPInsecureAuth.qlref b/python/ql/test/experimental/query-tests/Security/CWE-522-global-option/LDAPInsecureAuth.qlref
new file mode 100644
index 000000000000..8bb2c1e9b52a
--- /dev/null
+++ b/python/ql/test/experimental/query-tests/Security/CWE-522-global-option/LDAPInsecureAuth.qlref
@@ -0,0 +1 @@
+experimental/Security/CWE-522/LDAPInsecureAuth.ql
\ No newline at end of file
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_global.py b/python/ql/test/experimental/query-tests/Security/CWE-522-global-option/ldap2_global.py
similarity index 58%
rename from python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_global.py
rename to python/ql/test/experimental/query-tests/Security/CWE-522-global-option/ldap2_global.py
index 44d528698fab..59723cf6ddac 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-522/ldap2_global.py
+++ b/python/ql/test/experimental/query-tests/Security/CWE-522-global-option/ldap2_global.py
@@ -1,3 +1,6 @@
+# since global options are considered to affect all files in a repo, we need to keep
+# this test in its' own directory (so it doesn't interfere with other tests).
+
import ldap
from flask import request, Flask
@@ -7,11 +10,13 @@
# SSL through ldap global variable option
-ldap.set_option(ldap.OPT_X_TLS_NEVER)
+ldap.set_option(ldap.OPT_X_TLS_DEMAND, True)
@app.route("/one")
def one():
+ # The following connection would have been insecure if the global option above was
+ # not set
ldap_connection_5 = ldap.initialize("ldap://somethingon.theinternet.com")
ldap_connection_5.simple_bind_s('', '')
user = ldap_connection_5.search_s(
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.expected b/python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.expected
index 592ad8b99f5c..24784f039e75 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.expected
+++ b/python/ql/test/experimental/query-tests/Security/CWE-522/LDAPInsecureAuth.expected
@@ -6,7 +6,6 @@ edges
| ldap3_remote.py:138:21:138:32 | ControlFlowNode for Attribute | ldap3_remote.py:138:21:138:40 | ControlFlowNode for Subscript |
| ldap3_remote.py:138:21:138:40 | ControlFlowNode for Subscript | ldap3_remote.py:139:18:139:21 | ControlFlowNode for host |
nodes
-| ldap2_global.py:15:41:15:76 | ControlFlowNode for Str | semmle.label | ControlFlowNode for Str |
| ldap2_remote.py:45:41:45:60 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
| ldap2_remote.py:56:41:56:60 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
| ldap3_remote.py:101:12:101:49 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
@@ -21,7 +20,6 @@ nodes
| ldap3_remote.py:139:18:139:21 | ControlFlowNode for host | semmle.label | ControlFlowNode for host |
subpaths
#select
-| ldap2_global.py:15:41:15:76 | ControlFlowNode for Str | ldap2_global.py:15:41:15:76 | ControlFlowNode for Str | ldap2_global.py:15:41:15:76 | ControlFlowNode for Str | $@ is authenticated insecurely. | ldap2_global.py:15:41:15:76 | ControlFlowNode for Str | This LDAP host |
| 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 |
| 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 |
| 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 |