From 8715d29a440020522753874b8edd08fcad3715f7 Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Thu, 18 Mar 2021 17:35:05 +0100
Subject: [PATCH 01/29] Upload LDAP Improper authentication query, qhelp and
 tests

---
 .../Security/CWE-287/LDAPImproperAuth.qhelp   | 31 +++++++
 .../src/Security/CWE-287/LDAPImproperAuth.ql  | 85 +++++++++++++++++++
 .../src/Security/CWE-287/tests/3_auth_bad.py  | 30 +++++++
 .../src/Security/CWE-287/tests/3_auth_good.py | 20 +++++
 .../ql/src/Security/CWE-287/tests/auth_bad.py | 52 ++++++++++++
 .../src/Security/CWE-287/tests/auth_good.py   | 52 ++++++++++++
 6 files changed, 270 insertions(+)
 create mode 100644 python/ql/src/Security/CWE-287/LDAPImproperAuth.qhelp
 create mode 100644 python/ql/src/Security/CWE-287/LDAPImproperAuth.ql
 create mode 100644 python/ql/src/Security/CWE-287/tests/3_auth_bad.py
 create mode 100644 python/ql/src/Security/CWE-287/tests/3_auth_good.py
 create mode 100644 python/ql/src/Security/CWE-287/tests/auth_bad.py
 create mode 100644 python/ql/src/Security/CWE-287/tests/auth_good.py

diff --git a/python/ql/src/Security/CWE-287/LDAPImproperAuth.qhelp b/python/ql/src/Security/CWE-287/LDAPImproperAuth.qhelp
new file mode 100644
index 000000000000..7554ae5c50bc
--- /dev/null
+++ b/python/ql/src/Security/CWE-287/LDAPImproperAuth.qhelp
@@ -0,0 +1,31 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+
+<qhelp>
+
+  <overview>
+    <p>If an LDAP connection doesn't carry any kind of authentication, gets produced an access to information 
+    in the LDAP directory.</p>
+
+    <p>Simple authentication in LDAP can be used with three different mechanisms:<p>
+
+    <li>Anonymous Authentication Mechanism by performing a bind request with a username and password value of zero length.</li>
+    <li>Unauthenticated Authentication Mechanism by performing a bind request with a password value of zero length.</li>
+    <li>Name/Password Authentication Mechanism by performing a bind request with a password value of non-zero length.</li>
+
+  </overview>
+
+  <recommendation>
+    <p>Every LDAP authentication should be done by using a password taken from a safe place.
+  <recommendation>
+
+  <references>
+    <li>
+      SonarSource
+      <a href="https://rules.sonarsource.com/python/type/Vulnerability/RSPEC-4433">RSPEC-4433</a>
+    </li>
+    <li>
+      CWE-
+      <a href="https://cwe.mitre.org/data/definitions/287.html">287</a>
+  </references>
+  
+</qhelp>
\ No newline at end of file
diff --git a/python/ql/src/Security/CWE-287/LDAPImproperAuth.ql b/python/ql/src/Security/CWE-287/LDAPImproperAuth.ql
new file mode 100644
index 000000000000..912ba3856160
--- /dev/null
+++ b/python/ql/src/Security/CWE-287/LDAPImproperAuth.ql
@@ -0,0 +1,85 @@
+/**
+ * @name Python LDAP Improper Authentication
+ * @description Check if a user-controlled query carry no authentication
+ * @kind path-problem
+ * @problem.severity warning
+ * @id python/ldap-improper-auth
+ * @tags experimental
+ *       security
+ *       external/cwe/cwe-287
+ */
+
+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
+
+// LDAP2
+class BindSink extends DataFlow::Node {
+  BindSink() {
+    exists(SsaVariable bindVar, CallNode bindCall, CallNode searchCall |
+      // get variable initializing the connection
+      bindVar.getDefinition().getImmediateDominator() = Value::named("ldap.initialize").getACall() and
+      // get a call using that variable
+      bindVar.getAUse().getImmediateDominator() = bindCall and
+      // restrict call to any bind method
+      bindCall.getNode().getFunc().(Attribute).getName().matches("%bind%") and
+      (
+        // check second argument (password)
+        bindCall.getArg(1).getNode() instanceof None or
+        count(bindCall.getAnArg()) = 1
+      ) and
+      // get another call using that variable
+      bindVar.getAUse().getNode() = searchCall.getNode().getFunc().(Attribute).getObject() and
+      // restrict call to any search method
+      searchCall.getNode().getFunc().(Attribute).getName().matches("%search%") and
+      // set the third argument as sink
+      this.asExpr() = searchCall.getArg(2).getNode()
+    )
+  }
+}
+
+// LDAP3
+class ConnectionSink extends DataFlow::Node {
+  ConnectionSink() {
+    exists(SsaVariable connectionVar, CallNode connectionCall, CallNode searchCall |
+      // get call initializing the connection
+      connectionCall = Value::named("ldap3.Connection").getACall() and
+      (
+        // check password argument
+        not exists(connectionCall.getArgByName("password")) or
+        connectionCall.getArgByName("password").pointsTo(Value::named("None"))
+      ) and
+      // get the variable initializing the connection
+      connectionVar.getDefinition().getImmediateDominator() = connectionCall and
+      // get a call using that variable
+      connectionVar.getAUse().getNode() = searchCall.getNode().getFunc().(Attribute).getObject() and
+      // restrict call to any search method
+      searchCall.getNode().getFunc().(Attribute).getName().matches("%search%") and
+      // set the second argument as sink
+      this.asExpr() = searchCall.getArg(1).getNode()
+    )
+  }
+}
+
+class LDAPImproperAuthSink extends DataFlow::Node {
+  LDAPImproperAuthSink() {
+    this instanceof BindSink or
+    this instanceof ConnectionSink
+  }
+}
+
+class LDAPImproperAuthConfig extends TaintTracking::Configuration {
+  LDAPImproperAuthConfig() { this = "LDAPImproperAuthConfig" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof LDAPImproperAuthSink }
+}
+
+from LDAPImproperAuthConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "$@ LDAP query executes $@.", sink.getNode(), "This",
+  source.getNode(), "a user-provided value without authentication."
diff --git a/python/ql/src/Security/CWE-287/tests/3_auth_bad.py b/python/ql/src/Security/CWE-287/tests/3_auth_bad.py
new file mode 100644
index 000000000000..5726ade823cf
--- /dev/null
+++ b/python/ql/src/Security/CWE-287/tests/3_auth_bad.py
@@ -0,0 +1,30 @@
+from ldap3 import Server, Connection, ALL
+from flask import request, Flask
+
+app = Flask(__name__)
+
+
+@app.route("/passwordNone")
+def passwordNone():
+    dn = request.args['dc']
+    search_filter = request.args['search']
+
+    srv = Server('servername', get_info=ALL)
+    conn = Connection(srv, user='user_dn', password=None)
+    status, result, response, _ = conn.search(dn, search_filter)
+    return result
+
+
+@app.route("/notPassword")
+def notPassword():
+    dn = request.args['dc']
+    search_filter = request.args['search']
+
+    srv = Server('servername', get_info=ALL)
+    conn = Connection(srv, user='user_dn')
+    status, result, response, _ = conn.search(dn, search_filter)
+    return result
+
+
+# if __name__ == "__main__":
+#     app.run(debug=True)
diff --git a/python/ql/src/Security/CWE-287/tests/3_auth_good.py b/python/ql/src/Security/CWE-287/tests/3_auth_good.py
new file mode 100644
index 000000000000..9c45ad58e349
--- /dev/null
+++ b/python/ql/src/Security/CWE-287/tests/3_auth_good.py
@@ -0,0 +1,20 @@
+from ldap3 import Server, Connection, ALL
+from flask import request, Flask
+import os
+
+app = Flask(__name__)
+
+
+@app.route("/passwordFromEnv")
+def passwordFromEnv():
+    dn = request.args['dc']
+    search_filter = request.args['search']
+
+    srv = Server('servername', get_info=ALL)
+    conn = Connection(srv, user='user_dn',
+                      password=os.environ.get('LDAP_PASSWORD'))
+    status, result, response, _ = conn.search(dn, search_filter)
+    return result
+
+# if __name__ == "__main__":
+#     app.run(debug=True)
diff --git a/python/ql/src/Security/CWE-287/tests/auth_bad.py b/python/ql/src/Security/CWE-287/tests/auth_bad.py
new file mode 100644
index 000000000000..36323e71ee3d
--- /dev/null
+++ b/python/ql/src/Security/CWE-287/tests/auth_bad.py
@@ -0,0 +1,52 @@
+from flask import request, Flask
+import ldap
+
+app = Flask(__name__)
+
+
+@app.route("/simple_bind")
+def simple_bind():
+    dn = request.args['dc']
+    search_filter = request.args['search']
+
+    ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
+    ldap_connection.simple_bind('cn=root')
+    user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
+    return user[0]
+
+
+@app.route("/simple_bind_s")
+def simple_bind_s():
+    dn = request.args['dc']
+    search_filter = request.args['search']
+
+    ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
+    ldap_connection.simple_bind_s('cn=root')
+    user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
+    return user[0]
+
+
+@app.route("/bind_s")
+def bind_s():
+    dn = request.args['dc']
+    search_filter = request.args['search']
+
+    ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
+    ldap_connection.bind_s('cn=root', None)
+    user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
+    return user[0]
+
+
+@app.route("/bind")
+def bind():
+    dn = request.args['dc']
+    search_filter = request.args['search']
+
+    ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
+    ldap_connection.bind('cn=root', None)
+    user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
+    return user[0]
+
+
+# if __name__ == "__main__":
+#     app.run(debug=True)
diff --git a/python/ql/src/Security/CWE-287/tests/auth_good.py b/python/ql/src/Security/CWE-287/tests/auth_good.py
new file mode 100644
index 000000000000..884bcb7af264
--- /dev/null
+++ b/python/ql/src/Security/CWE-287/tests/auth_good.py
@@ -0,0 +1,52 @@
+from flask import request, Flask
+import ldap
+import os
+
+app = Flask(__name__)
+
+
+@app.route("/simple_bind")
+def simple_bind():
+    dn = request.args['dc']
+    search_filter = request.args['search']
+
+    ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
+    ldap_connection.simple_bind('cn=root', os.environ.get('LDAP_PASSWORD'))
+    user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
+    return user[0]
+
+
+@app.route("/simple_bind_s")
+def simple_bind_s():
+    dn = request.args['dc']
+    search_filter = request.args['search']
+
+    ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
+    ldap_connection.simple_bind_s('cn=root', os.environ.get('LDAP_PASSWORD'))
+    user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
+    return user[0]
+
+
+@app.route("/bind_s")
+def bind_s():
+    dn = request.args['dc']
+    search_filter = request.args['search']
+
+    ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
+    ldap_connection.bind_s('cn=root', os.environ.get('LDAP_PASSWORD'))
+    user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
+    return user[0]
+
+
+@app.route("/bind")
+def bind():
+    dn = request.args['dc']
+    search_filter = request.args['search']
+
+    ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
+    ldap_connection.bind('cn=root', os.environ.get('LDAP_PASSWORD'))
+    user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
+    return user[0]
+
+# if __name__ == "__main__":
+#     app.run(debug=True)

From 809bf2377e35c0f67e47af536bc7be3ba993299f Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Thu, 18 Mar 2021 20:21:00 +0100
Subject: [PATCH 02/29] Move to experimental folder

---
 .../{ => experimental}/Security/CWE-287/LDAPImproperAuth.qhelp    | 0
 .../src/{ => experimental}/Security/CWE-287/LDAPImproperAuth.ql   | 0
 .../src/{ => experimental}/Security/CWE-287/tests/3_auth_bad.py   | 0
 .../src/{ => experimental}/Security/CWE-287/tests/3_auth_good.py  | 0
 .../ql/src/{ => experimental}/Security/CWE-287/tests/auth_bad.py  | 0
 .../ql/src/{ => experimental}/Security/CWE-287/tests/auth_good.py | 0
 6 files changed, 0 insertions(+), 0 deletions(-)
 rename python/ql/src/{ => experimental}/Security/CWE-287/LDAPImproperAuth.qhelp (100%)
 rename python/ql/src/{ => experimental}/Security/CWE-287/LDAPImproperAuth.ql (100%)
 rename python/ql/src/{ => experimental}/Security/CWE-287/tests/3_auth_bad.py (100%)
 rename python/ql/src/{ => experimental}/Security/CWE-287/tests/3_auth_good.py (100%)
 rename python/ql/src/{ => experimental}/Security/CWE-287/tests/auth_bad.py (100%)
 rename python/ql/src/{ => experimental}/Security/CWE-287/tests/auth_good.py (100%)

diff --git a/python/ql/src/Security/CWE-287/LDAPImproperAuth.qhelp b/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.qhelp
similarity index 100%
rename from python/ql/src/Security/CWE-287/LDAPImproperAuth.qhelp
rename to python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.qhelp
diff --git a/python/ql/src/Security/CWE-287/LDAPImproperAuth.ql b/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.ql
similarity index 100%
rename from python/ql/src/Security/CWE-287/LDAPImproperAuth.ql
rename to python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.ql
diff --git a/python/ql/src/Security/CWE-287/tests/3_auth_bad.py b/python/ql/src/experimental/Security/CWE-287/tests/3_auth_bad.py
similarity index 100%
rename from python/ql/src/Security/CWE-287/tests/3_auth_bad.py
rename to python/ql/src/experimental/Security/CWE-287/tests/3_auth_bad.py
diff --git a/python/ql/src/Security/CWE-287/tests/3_auth_good.py b/python/ql/src/experimental/Security/CWE-287/tests/3_auth_good.py
similarity index 100%
rename from python/ql/src/Security/CWE-287/tests/3_auth_good.py
rename to python/ql/src/experimental/Security/CWE-287/tests/3_auth_good.py
diff --git a/python/ql/src/Security/CWE-287/tests/auth_bad.py b/python/ql/src/experimental/Security/CWE-287/tests/auth_bad.py
similarity index 100%
rename from python/ql/src/Security/CWE-287/tests/auth_bad.py
rename to python/ql/src/experimental/Security/CWE-287/tests/auth_bad.py
diff --git a/python/ql/src/Security/CWE-287/tests/auth_good.py b/python/ql/src/experimental/Security/CWE-287/tests/auth_good.py
similarity index 100%
rename from python/ql/src/Security/CWE-287/tests/auth_good.py
rename to python/ql/src/experimental/Security/CWE-287/tests/auth_good.py

From 2f874c5c0b5e637cae37df1f717437cf9a2fa3f0 Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Thu, 18 Mar 2021 20:38:46 +0100
Subject: [PATCH 03/29] Precision warn and Remove CWE (broken) reference

---
 .../src/experimental/Security/CWE-287/LDAPImproperAuth.qhelp   | 3 ---
 .../ql/src/experimental/Security/CWE-287/LDAPImproperAuth.ql   | 1 +
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.qhelp b/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.qhelp
index 7554ae5c50bc..64fb94c1c9bf 100644
--- a/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.qhelp
+++ b/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.qhelp
@@ -23,9 +23,6 @@
       SonarSource
       <a href="https://rules.sonarsource.com/python/type/Vulnerability/RSPEC-4433">RSPEC-4433</a>
     </li>
-    <li>
-      CWE-
-      <a href="https://cwe.mitre.org/data/definitions/287.html">287</a>
   </references>
   
 </qhelp>
\ No newline at end of file
diff --git a/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.ql b/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.ql
index 912ba3856160..19a899accd0b 100644
--- a/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.ql
+++ b/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.ql
@@ -9,6 +9,7 @@
  *       external/cwe/cwe-287
  */
 
+// Determine precision above
 import python
 import semmle.python.dataflow.new.RemoteFlowSources
 import semmle.python.dataflow.new.DataFlow

From bfd4280d353026ceae7aa0e427516d94708bd3f3 Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Tue, 6 Apr 2021 15:51:37 +0200
Subject: [PATCH 04/29] Fix imports and begin refactor

---
 python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.ql | 1 -
 .../experimental/semmle/python/security/LDAPImproperAuth.qll    | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)
 create mode 100644 python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll

diff --git a/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.ql b/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.ql
index 19a899accd0b..029dfb313a7b 100644
--- a/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.ql
+++ b/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.ql
@@ -14,7 +14,6 @@ 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
 
 // LDAP2
diff --git a/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll b/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll
new file mode 100644
index 000000000000..fdf4a7dc9327
--- /dev/null
+++ b/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll
@@ -0,0 +1,2 @@
+
+//

From db1f54a5f34d42139552eb29f60e6dde644f2138 Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Thu, 8 Apr 2021 00:19:00 +0200
Subject: [PATCH 05/29] Polish query file

---
 .../Security/CWE-287/LDAPImproperAuth.ql      | 78 ++-----------------
 1 file changed, 7 insertions(+), 71 deletions(-)

diff --git a/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.ql b/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.ql
index 029dfb313a7b..72b801bbb3d5 100644
--- a/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.ql
+++ b/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.ql
@@ -1,9 +1,9 @@
 /**
  * @name Python LDAP Improper Authentication
- * @description Check if a user-controlled query carry no authentication
+ * @description Check if a user-controlled query carries no authentication
  * @kind path-problem
  * @problem.severity warning
- * @id python/ldap-improper-auth
+ * @id python/ldap-improper-authentication
  * @tags experimental
  *       security
  *       external/cwe/cwe-287
@@ -11,75 +11,11 @@
 
 // Determine precision above
 import python
-import semmle.python.dataflow.new.RemoteFlowSources
-import semmle.python.dataflow.new.DataFlow
-import semmle.python.dataflow.new.TaintTracking
+import experimental.semmle.python.security.LDAPImproperAuth
 import DataFlow::PathGraph
 
-// LDAP2
-class BindSink extends DataFlow::Node {
-  BindSink() {
-    exists(SsaVariable bindVar, CallNode bindCall, CallNode searchCall |
-      // get variable initializing the connection
-      bindVar.getDefinition().getImmediateDominator() = Value::named("ldap.initialize").getACall() and
-      // get a call using that variable
-      bindVar.getAUse().getImmediateDominator() = bindCall and
-      // restrict call to any bind method
-      bindCall.getNode().getFunc().(Attribute).getName().matches("%bind%") and
-      (
-        // check second argument (password)
-        bindCall.getArg(1).getNode() instanceof None or
-        count(bindCall.getAnArg()) = 1
-      ) and
-      // get another call using that variable
-      bindVar.getAUse().getNode() = searchCall.getNode().getFunc().(Attribute).getObject() and
-      // restrict call to any search method
-      searchCall.getNode().getFunc().(Attribute).getName().matches("%search%") and
-      // set the third argument as sink
-      this.asExpr() = searchCall.getArg(2).getNode()
-    )
-  }
-}
-
-// LDAP3
-class ConnectionSink extends DataFlow::Node {
-  ConnectionSink() {
-    exists(SsaVariable connectionVar, CallNode connectionCall, CallNode searchCall |
-      // get call initializing the connection
-      connectionCall = Value::named("ldap3.Connection").getACall() and
-      (
-        // check password argument
-        not exists(connectionCall.getArgByName("password")) or
-        connectionCall.getArgByName("password").pointsTo(Value::named("None"))
-      ) and
-      // get the variable initializing the connection
-      connectionVar.getDefinition().getImmediateDominator() = connectionCall and
-      // get a call using that variable
-      connectionVar.getAUse().getNode() = searchCall.getNode().getFunc().(Attribute).getObject() and
-      // restrict call to any search method
-      searchCall.getNode().getFunc().(Attribute).getName().matches("%search%") and
-      // set the second argument as sink
-      this.asExpr() = searchCall.getArg(1).getNode()
-    )
-  }
-}
-
-class LDAPImproperAuthSink extends DataFlow::Node {
-  LDAPImproperAuthSink() {
-    this instanceof BindSink or
-    this instanceof ConnectionSink
-  }
-}
-
-class LDAPImproperAuthConfig extends TaintTracking::Configuration {
-  LDAPImproperAuthConfig() { this = "LDAPImproperAuthConfig" }
-
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof LDAPImproperAuthSink }
-}
-
-from LDAPImproperAuthConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
+from LDAPImproperAuthenticationConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
 where config.hasFlowPath(source, sink)
-select sink.getNode(), source, sink, "$@ LDAP query executes $@.", sink.getNode(), "This",
-  source.getNode(), "a user-provided value without authentication."
+select sink.getNode(), source, sink,
+  "$@ LDAP query parameter contains $@ and is executed without authentication.", sink.getNode(),
+  "This", source.getNode(), "a user-provided value"

From aa7763b3d2106a70fd378c35a04613600a8b0e50 Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Thu, 8 Apr 2021 00:19:14 +0200
Subject: [PATCH 06/29] Set up Concepts

---
 .../experimental/semmle/python/Concepts.qll    | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/python/ql/src/experimental/semmle/python/Concepts.qll b/python/ql/src/experimental/semmle/python/Concepts.qll
index 904b7967ee87..6a3b60cfaa10 100644
--- a/python/ql/src/experimental/semmle/python/Concepts.qll
+++ b/python/ql/src/experimental/semmle/python/Concepts.qll
@@ -13,3 +13,21 @@ private import semmle.python.dataflow.new.DataFlow
 private import semmle.python.dataflow.new.RemoteFlowSources
 private import semmle.python.dataflow.new.TaintTracking
 private import experimental.semmle.python.Frameworks
+
+module LDAPBind {
+  abstract class Range extends DataFlow::Node {
+    abstract DataFlow::Node getPasswordNode();
+
+    abstract DataFlow::Node getQueryNode();
+  }
+}
+
+class LDAPBind extends DataFlow::Node {
+  LDAPBind::Range range;
+
+  LDAPBind() { this = range }
+
+  DataFlow::Node getPasswordNode() { result = range.getPasswordNode() }
+
+  DataFlow::Node getQueryNode() { result = range.getQueryNode() }
+}

From 8ca6e84268eba430a909f389bf2e52ac18aad772 Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Thu, 8 Apr 2021 00:19:46 +0200
Subject: [PATCH 07/29] Refactor Calls to use ApiGraphs

---
 .../semmle/python/frameworks/Stdlib.qll       | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll b/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll
index 420caf0d73bb..5860151e070c 100644
--- a/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll
+++ b/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll
@@ -9,3 +9,60 @@ private import semmle.python.dataflow.new.TaintTracking
 private import semmle.python.dataflow.new.RemoteFlowSources
 private import experimental.semmle.python.Concepts
 private import semmle.python.ApiGraphs
+
+private module LDAP {
+  private module LDAP2 {
+    private class LDAP2QueryMethods extends string {
+      LDAP2QueryMethods() {
+        this in ["search", "search_s", "search_st", "search_ext", "search_ext_s"]
+      }
+    }
+
+    class LDAP2Bind extends DataFlow::CallCfgNode, LDAPBind::Range {
+      DataFlow::Node queryNode;
+
+      LDAP2Bind() {
+        exists(
+          DataFlow::AttrRead bindMethod, DataFlow::CallCfgNode searchCall,
+          DataFlow::AttrRead searchMethod
+        |
+          this.getFunction() = bindMethod and
+          API::moduleImport("ldap").getMember("initialize").getACall() =
+            bindMethod.getObject().getALocalSource() and
+          bindMethod.getAttributeName().matches("%bind%") and
+          searchCall.getFunction() = searchMethod and
+          bindMethod.getObject().getALocalSource() = searchMethod.getObject().getALocalSource() and
+          searchMethod.getAttributeName() instanceof LDAP2QueryMethods and
+          (
+            queryNode = searchCall.getArg(2) or
+            queryNode = searchCall.getArgByName("filterstr")
+          )
+        )
+      }
+
+      override DataFlow::Node getPasswordNode() { result = this.getArg(1) }
+
+      override DataFlow::Node getQueryNode() { result = queryNode }
+    }
+  }
+
+  private module LDAP3 {
+    class LDAP3Bind extends DataFlow::CallCfgNode, LDAPBind::Range {
+      DataFlow::Node queryNode;
+
+      LDAP3Bind() {
+        exists(DataFlow::CallCfgNode searchCall, DataFlow::AttrRead searchMethod |
+          this = API::moduleImport("ldap3").getMember("Connection").getACall() and
+          searchMethod.getObject().getALocalSource() = this and
+          searchCall.getFunction() = searchMethod and
+          searchMethod.getAttributeName() = "search" and
+          queryNode = searchCall.getArg(1)
+        )
+      }
+
+      override DataFlow::Node getPasswordNode() { result = this.getArgByName("password") }
+
+      override DataFlow::Node getQueryNode() { result = queryNode }
+    }
+  }
+}

From 7e456494ef9fa1ff9d5215b58ce2ef3eaf6e24b0 Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Thu, 8 Apr 2021 00:20:04 +0200
Subject: [PATCH 08/29] Set up taint config and custom sink

---
 .../python/security/LDAPImproperAuth.qll      | 25 ++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll b/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll
index fdf4a7dc9327..f481cf75831c 100644
--- a/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll
+++ b/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll
@@ -1,2 +1,25 @@
+import python
+import experimental.semmle.python.Concepts
+import semmle.python.dataflow.new.DataFlow
+import semmle.python.dataflow.new.TaintTracking
+import semmle.python.dataflow.new.RemoteFlowSources
 
-//
+class LDAPImproperAuthSink extends DataFlow::Node {
+  LDAPImproperAuthSink() {
+    exists(LDAPBind ldapBind |
+      (
+        DataFlow::localFlow(DataFlow::exprNode(any(None noneName)), ldapBind.getPasswordNode()) or
+        not exists(ldapBind.getPasswordNode())
+      ) and
+      this = ldapBind.getQueryNode()
+    )
+  }
+}
+
+class LDAPImproperAuthenticationConfig extends TaintTracking::Configuration {
+  LDAPImproperAuthenticationConfig() { this = "LDAPImproperAuthenticationConfig" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof LDAPImproperAuthSink }
+}

From 63bd32359a8bde41c67657d5b754e1cdc7fdd640 Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Fri, 9 Apr 2021 00:48:57 +0200
Subject: [PATCH 09/29] Improve qhelp

---
 .../Security/CWE-287/ImproperLdapAuth.qhelp   | 32 +++++++++++++++++++
 .../Security/CWE-287/LDAPImproperAuth.qhelp   | 28 ----------------
 2 files changed, 32 insertions(+), 28 deletions(-)
 create mode 100644 python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.qhelp
 delete mode 100644 python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.qhelp

diff --git a/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.qhelp b/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.qhelp
new file mode 100644
index 000000000000..43f47927fbad
--- /dev/null
+++ b/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.qhelp
@@ -0,0 +1,32 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>If an LDAP query is built using string concatenation or string formatting, and it doesn't carry any kind of authentication, 
+anonymous binds causes an empty or None-set password to result in a successful authentication.</p>
+</overview>
+
+<recommendation>
+<p>Use a strong password while establishing a LDAP connection to execute a query a user controls.</p>
+</recommendation>
+
+<example>
+<p>In the following examples, the code accepts both <code>username</code> and <code>dc</code> from the user, 
+which it then uses to build a LDAP query and DN while the connection carries no authentication or binds anonymously.</p>
+
+<sample src="examples/auth_bad_2.py" />
+<sample src="examples/auth_bad_3.py" />
+
+<p>In the third and fourth examples, the authentication is established using a password from a secure source such as environment variables.</p>
+
+<sample src="examples/auth_good_2.py" />
+<sample src="examples/auth_good_3.py" />
+</example>
+
+<references>
+<li>SonarSource: <a href="https://rules.sonarsource.com/python/type/Vulnerability/RSPEC-4433">RSPEC-4433</a>.</li>
+<li>Python2: <a href="https://www.python-ldap.org/en/python-ldap-3.3.0/reference/ldap.html">LDAP Documentation</a>.</li>
+<li>Python3: <a href="https://ldap3.readthedocs.io/en/latest/">LDAP Documentation</a>.</li>
+</references>
+</qhelp>
diff --git a/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.qhelp b/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.qhelp
deleted file mode 100644
index 64fb94c1c9bf..000000000000
--- a/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.qhelp
+++ /dev/null
@@ -1,28 +0,0 @@
-<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
-
-<qhelp>
-
-  <overview>
-    <p>If an LDAP connection doesn't carry any kind of authentication, gets produced an access to information 
-    in the LDAP directory.</p>
-
-    <p>Simple authentication in LDAP can be used with three different mechanisms:<p>
-
-    <li>Anonymous Authentication Mechanism by performing a bind request with a username and password value of zero length.</li>
-    <li>Unauthenticated Authentication Mechanism by performing a bind request with a password value of zero length.</li>
-    <li>Name/Password Authentication Mechanism by performing a bind request with a password value of non-zero length.</li>
-
-  </overview>
-
-  <recommendation>
-    <p>Every LDAP authentication should be done by using a password taken from a safe place.
-  <recommendation>
-
-  <references>
-    <li>
-      SonarSource
-      <a href="https://rules.sonarsource.com/python/type/Vulnerability/RSPEC-4433">RSPEC-4433</a>
-    </li>
-  </references>
-  
-</qhelp>
\ No newline at end of file

From 20fc5db49ec613fc03835a185175c0beb2050eea Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Fri, 9 Apr 2021 00:49:30 +0200
Subject: [PATCH 10/29] Polish query file

---
 .../CWE-287/{LDAPImproperAuth.ql => ImproperLdapAuth.ql}    | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
 rename python/ql/src/experimental/Security/CWE-287/{LDAPImproperAuth.ql => ImproperLdapAuth.ql} (78%)

diff --git a/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.ql b/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.ql
similarity index 78%
rename from python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.ql
rename to python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.ql
index 72b801bbb3d5..f5e92a2ee6d2 100644
--- a/python/ql/src/experimental/Security/CWE-287/LDAPImproperAuth.ql
+++ b/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.ql
@@ -1,9 +1,9 @@
 /**
- * @name Python LDAP Improper Authentication
- * @description Check if a user-controlled query carries no authentication
+ * @name Improper LDAP Authentication
+ * @description A user-controlled query carries no authentication
  * @kind path-problem
  * @problem.severity warning
- * @id python/ldap-improper-authentication
+ * @id py/improper-ldap-auth
  * @tags experimental
  *       security
  *       external/cwe/cwe-287

From 2392be08c797f1ed49b39a73fdb39d0062bd0c9a Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Fri, 9 Apr 2021 00:50:04 +0200
Subject: [PATCH 11/29] Improve sink

---
 .../semmle/python/security/LDAPImproperAuth.qll       | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll b/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll
index f481cf75831c..dcfd49221c52 100644
--- a/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll
+++ b/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll
@@ -8,8 +8,15 @@ class LDAPImproperAuthSink extends DataFlow::Node {
   LDAPImproperAuthSink() {
     exists(LDAPBind ldapBind |
       (
-        DataFlow::localFlow(DataFlow::exprNode(any(None noneName)), ldapBind.getPasswordNode()) or
-        not exists(ldapBind.getPasswordNode())
+        (
+          DataFlow::localFlow(DataFlow::exprNode(any(None noneName)), ldapBind.getPasswordNode()) or
+          not exists(ldapBind.getPasswordNode())
+        )
+        or
+        exists(StrConst emptyString |
+          emptyString.getText() = "" and
+          DataFlow::localFlow(DataFlow::exprNode(emptyString), ldapBind.getPasswordNode())
+        )
       ) and
       this = ldapBind.getQueryNode()
     )

From 015d203fcb610988ed715b929338a54b878de252 Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Fri, 9 Apr 2021 00:50:47 +0200
Subject: [PATCH 12/29] Improve tests, move them and create qhelp examples

---
 .../Security/CWE-287/examples/auth_bad_2.py   | 12 ++++++
 .../Security/CWE-287/examples/auth_bad_3.py   | 12 ++++++
 .../Security/CWE-287/examples/auth_good_2.py  | 13 ++++++
 .../auth_good_3.py}                           |  6 ---
 .../Security/CWE-287/auth_bad_2.py}           | 42 +++++++++++++------
 .../Security/CWE-287/auth_bad_3.py}           | 27 +++++++++++-
 .../Security/CWE-287/auth_good_2.py}          | 40 ++++++++++++------
 .../Security/CWE-287/auth_good_3.py           | 24 +++++++++++
 8 files changed, 143 insertions(+), 33 deletions(-)
 create mode 100644 python/ql/src/experimental/Security/CWE-287/examples/auth_bad_2.py
 create mode 100644 python/ql/src/experimental/Security/CWE-287/examples/auth_bad_3.py
 create mode 100644 python/ql/src/experimental/Security/CWE-287/examples/auth_good_2.py
 rename python/ql/src/experimental/Security/CWE-287/{tests/3_auth_good.py => examples/auth_good_3.py} (81%)
 rename python/ql/{src/experimental/Security/CWE-287/tests/auth_bad.py => test/experimental/query-tests/Security/CWE-287/auth_bad_2.py} (59%)
 rename python/ql/{src/experimental/Security/CWE-287/tests/3_auth_bad.py => test/experimental/query-tests/Security/CWE-287/auth_bad_3.py} (51%)
 rename python/ql/{src/experimental/Security/CWE-287/tests/auth_good.py => test/experimental/query-tests/Security/CWE-287/auth_good_2.py} (62%)
 create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-287/auth_good_3.py

diff --git a/python/ql/src/experimental/Security/CWE-287/examples/auth_bad_2.py b/python/ql/src/experimental/Security/CWE-287/examples/auth_bad_2.py
new file mode 100644
index 000000000000..c893016ee343
--- /dev/null
+++ b/python/ql/src/experimental/Security/CWE-287/examples/auth_bad_2.py
@@ -0,0 +1,12 @@
+from flask import request, Flask
+import ldap
+
+
+@app.route("/bind_example")
+def bind_example():
+    dn = request.args['dc']
+    search_filter = request.args['search']
+
+    ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
+    ldap_connection.bind('cn=root', "")
+    user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
diff --git a/python/ql/src/experimental/Security/CWE-287/examples/auth_bad_3.py b/python/ql/src/experimental/Security/CWE-287/examples/auth_bad_3.py
new file mode 100644
index 000000000000..2cce5b73bd5b
--- /dev/null
+++ b/python/ql/src/experimental/Security/CWE-287/examples/auth_bad_3.py
@@ -0,0 +1,12 @@
+from ldap3 import Server, Connection, ALL
+from flask import request, Flask
+
+
+@app.route("/passwordNone")
+def passwordNone():
+    dn = request.args['dc']
+    search_filter = request.args['search']
+
+    srv = Server('servername', get_info=ALL)
+    conn = Connection(srv, user='user_dn', password=None)
+    status, result, response, _ = conn.search(dn, search_filter)
diff --git a/python/ql/src/experimental/Security/CWE-287/examples/auth_good_2.py b/python/ql/src/experimental/Security/CWE-287/examples/auth_good_2.py
new file mode 100644
index 000000000000..e5fc88f56d1c
--- /dev/null
+++ b/python/ql/src/experimental/Security/CWE-287/examples/auth_good_2.py
@@ -0,0 +1,13 @@
+from flask import request, Flask
+import ldap
+import os
+
+
+@app.route("/bind_example")
+def bind_example():
+    dn = request.args['dc']
+    search_filter = request.args['search']
+
+    ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
+    ldap_connection.bind('cn=root', os.environ.get('LDAP_PASSWORD'))
+    user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
diff --git a/python/ql/src/experimental/Security/CWE-287/tests/3_auth_good.py b/python/ql/src/experimental/Security/CWE-287/examples/auth_good_3.py
similarity index 81%
rename from python/ql/src/experimental/Security/CWE-287/tests/3_auth_good.py
rename to python/ql/src/experimental/Security/CWE-287/examples/auth_good_3.py
index 9c45ad58e349..ae554fa9f7d2 100644
--- a/python/ql/src/experimental/Security/CWE-287/tests/3_auth_good.py
+++ b/python/ql/src/experimental/Security/CWE-287/examples/auth_good_3.py
@@ -2,8 +2,6 @@
 from flask import request, Flask
 import os
 
-app = Flask(__name__)
-
 
 @app.route("/passwordFromEnv")
 def passwordFromEnv():
@@ -14,7 +12,3 @@ def passwordFromEnv():
     conn = Connection(srv, user='user_dn',
                       password=os.environ.get('LDAP_PASSWORD'))
     status, result, response, _ = conn.search(dn, search_filter)
-    return result
-
-# if __name__ == "__main__":
-#     app.run(debug=True)
diff --git a/python/ql/src/experimental/Security/CWE-287/tests/auth_bad.py b/python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_2.py
similarity index 59%
rename from python/ql/src/experimental/Security/CWE-287/tests/auth_bad.py
rename to python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_2.py
index 36323e71ee3d..cc64623015e3 100644
--- a/python/ql/src/experimental/Security/CWE-287/tests/auth_bad.py
+++ b/python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_2.py
@@ -4,48 +4,64 @@
 app = Flask(__name__)
 
 
-@app.route("/simple_bind")
-def simple_bind():
+@app.route("/simple_bind_example")
+def simple_bind_example():
+    """
+    A RemoteFlowSource is used directly as DN and search filter while the bind's password
+    is not set
+    """
+
     dn = request.args['dc']
     search_filter = request.args['search']
 
     ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
     ldap_connection.simple_bind('cn=root')
     user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
-    return user[0]
 
 
-@app.route("/simple_bind_s")
-def simple_bind_s():
+@app.route("/simple_bind_s_example")
+def simple_bind_s_example():
+    """
+    A RemoteFlowSource is used directly as DN and search filter while the bind's password
+    is not set
+    """
+
     dn = request.args['dc']
     search_filter = request.args['search']
 
     ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
     ldap_connection.simple_bind_s('cn=root')
     user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
-    return user[0]
 
 
-@app.route("/bind_s")
-def bind_s():
+@app.route("/bind_s_example")
+def bind_s_example():
+    """
+    A RemoteFlowSource is used directly as DN and search filter while the bind's password
+    is set to None
+    """
+
     dn = request.args['dc']
     search_filter = request.args['search']
 
     ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
     ldap_connection.bind_s('cn=root', None)
     user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
-    return user[0]
 
 
-@app.route("/bind")
-def bind():
+@app.route("/bind_example")
+def bind_example():
+    """
+    A RemoteFlowSource is used directly as DN and search filter while the bind's password
+    is set to None
+    """
+
     dn = request.args['dc']
     search_filter = request.args['search']
 
     ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
-    ldap_connection.bind('cn=root', None)
+    ldap_connection.bind('cn=root', "")
     user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
-    return user[0]
 
 
 # if __name__ == "__main__":
diff --git a/python/ql/src/experimental/Security/CWE-287/tests/3_auth_bad.py b/python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_3.py
similarity index 51%
rename from python/ql/src/experimental/Security/CWE-287/tests/3_auth_bad.py
rename to python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_3.py
index 5726ade823cf..b69722bf589e 100644
--- a/python/ql/src/experimental/Security/CWE-287/tests/3_auth_bad.py
+++ b/python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_3.py
@@ -6,24 +6,47 @@
 
 @app.route("/passwordNone")
 def passwordNone():
+    """
+    A RemoteFlowSource is used directly as DN and search filter while the connection's password
+    is set to None
+    """
+
     dn = request.args['dc']
     search_filter = request.args['search']
 
     srv = Server('servername', get_info=ALL)
     conn = Connection(srv, user='user_dn', password=None)
     status, result, response, _ = conn.search(dn, search_filter)
-    return result
+
+
+@app.route("/passwordEmpty")
+def passwordEmpty():
+    """
+    A RemoteFlowSource is used directly as DN and search filter while the connection's password
+    is empty
+    """
+
+    dn = request.args['dc']
+    search_filter = request.args['search']
+
+    srv = Server('servername', get_info=ALL)
+    conn = Connection(srv, user='user_dn', password="")
+    status, result, response, _ = conn.search(dn, search_filter)
 
 
 @app.route("/notPassword")
 def notPassword():
+    """
+    A RemoteFlowSource is used directly as DN and search filter while the connection's password
+    is not set
+    """
+
     dn = request.args['dc']
     search_filter = request.args['search']
 
     srv = Server('servername', get_info=ALL)
     conn = Connection(srv, user='user_dn')
     status, result, response, _ = conn.search(dn, search_filter)
-    return result
 
 
 # if __name__ == "__main__":
diff --git a/python/ql/src/experimental/Security/CWE-287/tests/auth_good.py b/python/ql/test/experimental/query-tests/Security/CWE-287/auth_good_2.py
similarity index 62%
rename from python/ql/src/experimental/Security/CWE-287/tests/auth_good.py
rename to python/ql/test/experimental/query-tests/Security/CWE-287/auth_good_2.py
index 884bcb7af264..925f80a3e9d9 100644
--- a/python/ql/src/experimental/Security/CWE-287/tests/auth_good.py
+++ b/python/ql/test/experimental/query-tests/Security/CWE-287/auth_good_2.py
@@ -5,48 +5,64 @@
 app = Flask(__name__)
 
 
-@app.route("/simple_bind")
-def simple_bind():
+@app.route("/simple_bind_example")
+def simple_bind_example():
+    """
+    A RemoteFlowSource is used directly as DN and search filter while the bind's password
+    is an environment variable
+    """
+
     dn = request.args['dc']
     search_filter = request.args['search']
 
     ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
     ldap_connection.simple_bind('cn=root', os.environ.get('LDAP_PASSWORD'))
     user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
-    return user[0]
 
 
-@app.route("/simple_bind_s")
-def simple_bind_s():
+@app.route("/simple_bind_s_example")
+def simple_bind_s_example():
+    """
+    A RemoteFlowSource is used directly as DN and search filter while the bind's password
+    is an environment variable
+    """
+
     dn = request.args['dc']
     search_filter = request.args['search']
 
     ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
     ldap_connection.simple_bind_s('cn=root', os.environ.get('LDAP_PASSWORD'))
     user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
-    return user[0]
 
 
-@app.route("/bind_s")
-def bind_s():
+@app.route("/bind_s_example")
+def bind_s_example():
+    """
+    A RemoteFlowSource is used directly as DN and search filter while the bind's password
+    is an environment variable
+    """
+
     dn = request.args['dc']
     search_filter = request.args['search']
 
     ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
     ldap_connection.bind_s('cn=root', os.environ.get('LDAP_PASSWORD'))
     user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
-    return user[0]
 
 
-@app.route("/bind")
-def bind():
+@app.route("/bind_example")
+def bind_example():
+    """
+    A RemoteFlowSource is used directly as DN and search filter while the bind's password
+    is an environment variable
+    """
+
     dn = request.args['dc']
     search_filter = request.args['search']
 
     ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
     ldap_connection.bind('cn=root', os.environ.get('LDAP_PASSWORD'))
     user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
-    return user[0]
 
 # if __name__ == "__main__":
 #     app.run(debug=True)
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-287/auth_good_3.py b/python/ql/test/experimental/query-tests/Security/CWE-287/auth_good_3.py
new file mode 100644
index 000000000000..fd9174c14286
--- /dev/null
+++ b/python/ql/test/experimental/query-tests/Security/CWE-287/auth_good_3.py
@@ -0,0 +1,24 @@
+from ldap3 import Server, Connection, ALL
+from flask import request, Flask
+import os
+
+app = Flask(__name__)
+
+
+@app.route("/passwordFromEnv")
+def passwordFromEnv():
+    """
+    A RemoteFlowSource is used directly as DN and search filter while the connection's password
+    is an environment variable
+    """
+
+    dn = request.args['dc']
+    search_filter = request.args['search']
+
+    srv = Server('servername', get_info=ALL)
+    conn = Connection(srv, user='user_dn',
+                      password=os.environ.get('LDAP_PASSWORD'))
+    status, result, response, _ = conn.search(dn, search_filter)
+
+# if __name__ == "__main__":
+#     app.run(debug=True)

From 1320eeee53cf332b5ee9d9ad21f1291b9bd011ad Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Fri, 9 Apr 2021 00:51:15 +0200
Subject: [PATCH 13/29] Add qlref

---
 .../query-tests/Security/CWE-287/ImproperLdapAuth.qlref          | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.qlref

diff --git a/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.qlref b/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.qlref
new file mode 100644
index 000000000000..9f5c6e4c43f4
--- /dev/null
+++ b/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.qlref
@@ -0,0 +1 @@
+experimental/Security/CWE-287/ImproperLdapAuth.ql

From 5787406a0d3f48329ff139af6f09231dde019573 Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Fri, 9 Apr 2021 00:51:26 +0200
Subject: [PATCH 14/29] Add .expected

---
 .../CWE-287/ImproperLdapAuth.expected         | 87 +++++++++++++++++++
 1 file changed, 87 insertions(+)
 create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.expected

diff --git a/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.expected b/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.expected
new file mode 100644
index 000000000000..1712533de774
--- /dev/null
+++ b/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.expected
@@ -0,0 +1,87 @@
+edges
+| auth_bad_2.py:14:10:14:16 | ControlFlowNode for request | auth_bad_2.py:15:21:15:27 | ControlFlowNode for request |
+| auth_bad_2.py:14:10:14:16 | ControlFlowNode for request | auth_bad_2.py:15:21:15:32 | ControlFlowNode for Attribute |
+| auth_bad_2.py:15:21:15:27 | ControlFlowNode for request | auth_bad_2.py:15:21:15:32 | ControlFlowNode for Attribute |
+| auth_bad_2.py:15:21:15:32 | ControlFlowNode for Attribute | auth_bad_2.py:15:21:15:42 | ControlFlowNode for Subscript |
+| auth_bad_2.py:15:21:15:42 | ControlFlowNode for Subscript | auth_bad_2.py:19:61:19:73 | ControlFlowNode for search_filter |
+| auth_bad_2.py:29:10:29:16 | ControlFlowNode for request | auth_bad_2.py:30:21:30:27 | ControlFlowNode for request |
+| auth_bad_2.py:29:10:29:16 | ControlFlowNode for request | auth_bad_2.py:30:21:30:32 | ControlFlowNode for Attribute |
+| auth_bad_2.py:30:21:30:27 | ControlFlowNode for request | auth_bad_2.py:30:21:30:32 | ControlFlowNode for Attribute |
+| auth_bad_2.py:30:21:30:32 | ControlFlowNode for Attribute | auth_bad_2.py:30:21:30:42 | ControlFlowNode for Subscript |
+| auth_bad_2.py:30:21:30:42 | ControlFlowNode for Subscript | auth_bad_2.py:34:61:34:73 | ControlFlowNode for search_filter |
+| auth_bad_2.py:44:10:44:16 | ControlFlowNode for request | auth_bad_2.py:45:21:45:27 | ControlFlowNode for request |
+| auth_bad_2.py:44:10:44:16 | ControlFlowNode for request | auth_bad_2.py:45:21:45:32 | ControlFlowNode for Attribute |
+| auth_bad_2.py:45:21:45:27 | ControlFlowNode for request | auth_bad_2.py:45:21:45:32 | ControlFlowNode for Attribute |
+| auth_bad_2.py:45:21:45:32 | ControlFlowNode for Attribute | auth_bad_2.py:45:21:45:42 | ControlFlowNode for Subscript |
+| auth_bad_2.py:45:21:45:42 | ControlFlowNode for Subscript | auth_bad_2.py:49:61:49:73 | ControlFlowNode for search_filter |
+| auth_bad_2.py:59:10:59:16 | ControlFlowNode for request | auth_bad_2.py:60:21:60:27 | ControlFlowNode for request |
+| auth_bad_2.py:59:10:59:16 | ControlFlowNode for request | auth_bad_2.py:60:21:60:32 | ControlFlowNode for Attribute |
+| auth_bad_2.py:60:21:60:27 | ControlFlowNode for request | auth_bad_2.py:60:21:60:32 | ControlFlowNode for Attribute |
+| auth_bad_2.py:60:21:60:32 | ControlFlowNode for Attribute | auth_bad_2.py:60:21:60:42 | ControlFlowNode for Subscript |
+| auth_bad_2.py:60:21:60:42 | ControlFlowNode for Subscript | auth_bad_2.py:64:61:64:73 | ControlFlowNode for search_filter |
+| auth_bad_3.py:14:10:14:16 | ControlFlowNode for request | auth_bad_3.py:15:21:15:27 | ControlFlowNode for request |
+| auth_bad_3.py:14:10:14:16 | ControlFlowNode for request | auth_bad_3.py:15:21:15:32 | ControlFlowNode for Attribute |
+| auth_bad_3.py:15:21:15:27 | ControlFlowNode for request | auth_bad_3.py:15:21:15:32 | ControlFlowNode for Attribute |
+| auth_bad_3.py:15:21:15:32 | ControlFlowNode for Attribute | auth_bad_3.py:15:21:15:42 | ControlFlowNode for Subscript |
+| auth_bad_3.py:15:21:15:42 | ControlFlowNode for Subscript | auth_bad_3.py:19:51:19:63 | ControlFlowNode for search_filter |
+| auth_bad_3.py:29:10:29:16 | ControlFlowNode for request | auth_bad_3.py:30:21:30:27 | ControlFlowNode for request |
+| auth_bad_3.py:29:10:29:16 | ControlFlowNode for request | auth_bad_3.py:30:21:30:32 | ControlFlowNode for Attribute |
+| auth_bad_3.py:30:21:30:27 | ControlFlowNode for request | auth_bad_3.py:30:21:30:32 | ControlFlowNode for Attribute |
+| auth_bad_3.py:30:21:30:32 | ControlFlowNode for Attribute | auth_bad_3.py:30:21:30:42 | ControlFlowNode for Subscript |
+| auth_bad_3.py:30:21:30:42 | ControlFlowNode for Subscript | auth_bad_3.py:34:51:34:63 | ControlFlowNode for search_filter |
+| auth_bad_3.py:44:10:44:16 | ControlFlowNode for request | auth_bad_3.py:45:21:45:27 | ControlFlowNode for request |
+| auth_bad_3.py:44:10:44:16 | ControlFlowNode for request | auth_bad_3.py:45:21:45:32 | ControlFlowNode for Attribute |
+| auth_bad_3.py:45:21:45:27 | ControlFlowNode for request | auth_bad_3.py:45:21:45:32 | ControlFlowNode for Attribute |
+| auth_bad_3.py:45:21:45:32 | ControlFlowNode for Attribute | auth_bad_3.py:45:21:45:42 | ControlFlowNode for Subscript |
+| auth_bad_3.py:45:21:45:42 | ControlFlowNode for Subscript | auth_bad_3.py:49:51:49:63 | ControlFlowNode for search_filter |
+nodes
+| auth_bad_2.py:14:10:14:16 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
+| auth_bad_2.py:15:21:15:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
+| auth_bad_2.py:15:21:15:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| auth_bad_2.py:15:21:15:42 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
+| auth_bad_2.py:19:61:19:73 | ControlFlowNode for search_filter | semmle.label | ControlFlowNode for search_filter |
+| auth_bad_2.py:29:10:29:16 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
+| auth_bad_2.py:30:21:30:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
+| auth_bad_2.py:30:21:30:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| auth_bad_2.py:30:21:30:42 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
+| auth_bad_2.py:34:61:34:73 | ControlFlowNode for search_filter | semmle.label | ControlFlowNode for search_filter |
+| auth_bad_2.py:44:10:44:16 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
+| auth_bad_2.py:45:21:45:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
+| auth_bad_2.py:45:21:45:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| auth_bad_2.py:45:21:45:42 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
+| auth_bad_2.py:49:61:49:73 | ControlFlowNode for search_filter | semmle.label | ControlFlowNode for search_filter |
+| auth_bad_2.py:59:10:59:16 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
+| auth_bad_2.py:60:21:60:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
+| auth_bad_2.py:60:21:60:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| auth_bad_2.py:60:21:60:42 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
+| auth_bad_2.py:64:61:64:73 | ControlFlowNode for search_filter | semmle.label | ControlFlowNode for search_filter |
+| auth_bad_3.py:14:10:14:16 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
+| auth_bad_3.py:15:21:15:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
+| auth_bad_3.py:15:21:15:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| auth_bad_3.py:15:21:15:42 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
+| auth_bad_3.py:19:51:19:63 | ControlFlowNode for search_filter | semmle.label | ControlFlowNode for search_filter |
+| auth_bad_3.py:29:10:29:16 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
+| auth_bad_3.py:30:21:30:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
+| auth_bad_3.py:30:21:30:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| auth_bad_3.py:30:21:30:42 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
+| auth_bad_3.py:34:51:34:63 | ControlFlowNode for search_filter | semmle.label | ControlFlowNode for search_filter |
+| auth_bad_3.py:44:10:44:16 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
+| auth_bad_3.py:45:21:45:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
+| auth_bad_3.py:45:21:45:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| auth_bad_3.py:45:21:45:42 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
+| auth_bad_3.py:49:51:49:63 | ControlFlowNode for search_filter | semmle.label | ControlFlowNode for search_filter |
+#select
+| auth_bad_2.py:19:61:19:73 | ControlFlowNode for search_filter | auth_bad_2.py:14:10:14:16 | ControlFlowNode for request | auth_bad_2.py:19:61:19:73 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_2.py:19:61:19:73 | ControlFlowNode for search_filter | This | auth_bad_2.py:14:10:14:16 | ControlFlowNode for request | a user-provided value |
+| auth_bad_2.py:19:61:19:73 | ControlFlowNode for search_filter | auth_bad_2.py:15:21:15:27 | ControlFlowNode for request | auth_bad_2.py:19:61:19:73 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_2.py:19:61:19:73 | ControlFlowNode for search_filter | This | auth_bad_2.py:15:21:15:27 | ControlFlowNode for request | a user-provided value |
+| auth_bad_2.py:34:61:34:73 | ControlFlowNode for search_filter | auth_bad_2.py:29:10:29:16 | ControlFlowNode for request | auth_bad_2.py:34:61:34:73 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_2.py:34:61:34:73 | ControlFlowNode for search_filter | This | auth_bad_2.py:29:10:29:16 | ControlFlowNode for request | a user-provided value |
+| auth_bad_2.py:34:61:34:73 | ControlFlowNode for search_filter | auth_bad_2.py:30:21:30:27 | ControlFlowNode for request | auth_bad_2.py:34:61:34:73 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_2.py:34:61:34:73 | ControlFlowNode for search_filter | This | auth_bad_2.py:30:21:30:27 | ControlFlowNode for request | a user-provided value |
+| auth_bad_2.py:49:61:49:73 | ControlFlowNode for search_filter | auth_bad_2.py:44:10:44:16 | ControlFlowNode for request | auth_bad_2.py:49:61:49:73 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_2.py:49:61:49:73 | ControlFlowNode for search_filter | This | auth_bad_2.py:44:10:44:16 | ControlFlowNode for request | a user-provided value |
+| auth_bad_2.py:49:61:49:73 | ControlFlowNode for search_filter | auth_bad_2.py:45:21:45:27 | ControlFlowNode for request | auth_bad_2.py:49:61:49:73 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_2.py:49:61:49:73 | ControlFlowNode for search_filter | This | auth_bad_2.py:45:21:45:27 | ControlFlowNode for request | a user-provided value |
+| auth_bad_2.py:64:61:64:73 | ControlFlowNode for search_filter | auth_bad_2.py:59:10:59:16 | ControlFlowNode for request | auth_bad_2.py:64:61:64:73 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_2.py:64:61:64:73 | ControlFlowNode for search_filter | This | auth_bad_2.py:59:10:59:16 | ControlFlowNode for request | a user-provided value |
+| auth_bad_2.py:64:61:64:73 | ControlFlowNode for search_filter | auth_bad_2.py:60:21:60:27 | ControlFlowNode for request | auth_bad_2.py:64:61:64:73 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_2.py:64:61:64:73 | ControlFlowNode for search_filter | This | auth_bad_2.py:60:21:60:27 | ControlFlowNode for request | a user-provided value |
+| auth_bad_3.py:19:51:19:63 | ControlFlowNode for search_filter | auth_bad_3.py:14:10:14:16 | ControlFlowNode for request | auth_bad_3.py:19:51:19:63 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_3.py:19:51:19:63 | ControlFlowNode for search_filter | This | auth_bad_3.py:14:10:14:16 | ControlFlowNode for request | a user-provided value |
+| auth_bad_3.py:19:51:19:63 | ControlFlowNode for search_filter | auth_bad_3.py:15:21:15:27 | ControlFlowNode for request | auth_bad_3.py:19:51:19:63 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_3.py:19:51:19:63 | ControlFlowNode for search_filter | This | auth_bad_3.py:15:21:15:27 | ControlFlowNode for request | a user-provided value |
+| auth_bad_3.py:34:51:34:63 | ControlFlowNode for search_filter | auth_bad_3.py:29:10:29:16 | ControlFlowNode for request | auth_bad_3.py:34:51:34:63 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_3.py:34:51:34:63 | ControlFlowNode for search_filter | This | auth_bad_3.py:29:10:29:16 | ControlFlowNode for request | a user-provided value |
+| auth_bad_3.py:34:51:34:63 | ControlFlowNode for search_filter | auth_bad_3.py:30:21:30:27 | ControlFlowNode for request | auth_bad_3.py:34:51:34:63 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_3.py:34:51:34:63 | ControlFlowNode for search_filter | This | auth_bad_3.py:30:21:30:27 | ControlFlowNode for request | a user-provided value |
+| auth_bad_3.py:49:51:49:63 | ControlFlowNode for search_filter | auth_bad_3.py:44:10:44:16 | ControlFlowNode for request | auth_bad_3.py:49:51:49:63 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_3.py:49:51:49:63 | ControlFlowNode for search_filter | This | auth_bad_3.py:44:10:44:16 | ControlFlowNode for request | a user-provided value |
+| auth_bad_3.py:49:51:49:63 | ControlFlowNode for search_filter | auth_bad_3.py:45:21:45:27 | ControlFlowNode for request | auth_bad_3.py:49:51:49:63 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_3.py:49:51:49:63 | ControlFlowNode for search_filter | This | auth_bad_3.py:45:21:45:27 | ControlFlowNode for request | a user-provided value |

From f140601241498e2b227d49b7637ec7dda9ea0e11 Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Fri, 9 Apr 2021 01:57:23 +0200
Subject: [PATCH 15/29] Write documentation

---
 .../experimental/semmle/python/Concepts.qll   | 19 ++++++++++++++
 .../semmle/python/frameworks/Stdlib.qll       | 26 +++++++++++++++++++
 .../python/security/LDAPImproperAuth.qll      | 10 +++++++
 3 files changed, 55 insertions(+)

diff --git a/python/ql/src/experimental/semmle/python/Concepts.qll b/python/ql/src/experimental/semmle/python/Concepts.qll
index 6a3b60cfaa10..e86afca82ffb 100644
--- a/python/ql/src/experimental/semmle/python/Concepts.qll
+++ b/python/ql/src/experimental/semmle/python/Concepts.qll
@@ -14,14 +14,33 @@ private import semmle.python.dataflow.new.RemoteFlowSources
 private import semmle.python.dataflow.new.TaintTracking
 private import experimental.semmle.python.Frameworks
 
+/** Provides classes for modeling LDAP bind-related APIs. */
 module LDAPBind {
+  /**
+   * A data-flow node that collects methods binding a LDAP connection.
+   *
+   * Extend this class to model new APIs. If you want to refine existing API models,
+   * extend `LDAPBind` instead.
+   */
   abstract class Range extends DataFlow::Node {
+    /**
+     * Gets the argument containing the binding expression.
+     */
     abstract DataFlow::Node getPasswordNode();
 
+    /**
+     * Gets the argument containing the executed query.
+     */
     abstract DataFlow::Node getQueryNode();
   }
 }
 
+/**
+ * A data-flow node that collects methods binding a LDAP connection.
+ *
+ * Extend this class to refine existing API models. If you want to model new APIs,
+ * extend `LDAPBind::Range` instead.
+ */
 class LDAPBind extends DataFlow::Node {
   LDAPBind::Range range;
 
diff --git a/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll b/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll
index 5860151e070c..775dd7b2e3db 100644
--- a/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll
+++ b/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll
@@ -10,14 +10,32 @@ private import semmle.python.dataflow.new.RemoteFlowSources
 private import experimental.semmle.python.Concepts
 private import semmle.python.ApiGraphs
 
+/**
+ * Provides models for Python's ldap-related libraries.
+ */
 private module LDAP {
+  /**
+   * Provides models for Python's `ldap` library.
+   *
+   * See https://www.python-ldap.org/en/python-ldap-3.3.0/index.html
+   */
   private module LDAP2 {
+    /**
+     * List of `ldap` methods used to execute a query.
+     *
+     * See https://www.python-ldap.org/en/python-ldap-3.3.0/reference/ldap.html#functions
+     */
     private class LDAP2QueryMethods extends string {
       LDAP2QueryMethods() {
         this in ["search", "search_s", "search_st", "search_ext", "search_ext_s"]
       }
     }
 
+    /**
+     * A class to find `ldap` methods binding a connection.
+     *
+     * See `LDAP2QueryMethods`
+     */
     class LDAP2Bind extends DataFlow::CallCfgNode, LDAPBind::Range {
       DataFlow::Node queryNode;
 
@@ -46,7 +64,15 @@ private module LDAP {
     }
   }
 
+  /**
+   * Provides models for Python's `ldap3` library.
+   *
+   * See https://pypi.org/project/ldap3/
+   */
   private module LDAP3 {
+    /**
+     * A class to find `ldap3` methods binding a connection.
+     */
     class LDAP3Bind extends DataFlow::CallCfgNode, LDAPBind::Range {
       DataFlow::Node queryNode;
 
diff --git a/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll b/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll
index dcfd49221c52..ef54d1612ac4 100644
--- a/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll
+++ b/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll
@@ -1,9 +1,16 @@
+/**
+ * Provides a taint-tracking configuration for detecting LDAP improper authentication vulnerabilities
+ */
+
 import python
 import experimental.semmle.python.Concepts
 import semmle.python.dataflow.new.DataFlow
 import semmle.python.dataflow.new.TaintTracking
 import semmle.python.dataflow.new.RemoteFlowSources
 
+/**
+ * A class to find `LDAPBind` methods using an empty password or set as None.
+ */
 class LDAPImproperAuthSink extends DataFlow::Node {
   LDAPImproperAuthSink() {
     exists(LDAPBind ldapBind |
@@ -23,6 +30,9 @@ class LDAPImproperAuthSink extends DataFlow::Node {
   }
 }
 
+/**
+ * A taint-tracking configuration for detecting LDAP improper authentications.
+ */
 class LDAPImproperAuthenticationConfig extends TaintTracking::Configuration {
   LDAPImproperAuthenticationConfig() { this = "LDAPImproperAuthenticationConfig" }
 

From d34d2ed2b10e2580119d28fef0c10f27dfd20063 Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Thu, 17 Jun 2021 17:42:38 +0200
Subject: [PATCH 16/29] Add .qlref

---
 .../Security/CWE-287/examples/ImproperLdapAuth.qlref             | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 python/ql/src/experimental/Security/CWE-287/examples/ImproperLdapAuth.qlref

diff --git a/python/ql/src/experimental/Security/CWE-287/examples/ImproperLdapAuth.qlref b/python/ql/src/experimental/Security/CWE-287/examples/ImproperLdapAuth.qlref
new file mode 100644
index 000000000000..9f5c6e4c43f4
--- /dev/null
+++ b/python/ql/src/experimental/Security/CWE-287/examples/ImproperLdapAuth.qlref
@@ -0,0 +1 @@
+experimental/Security/CWE-287/ImproperLdapAuth.ql

From 13cfcec96812e90d1a402d981e6fa27acd28fc9a Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Thu, 17 Jun 2021 17:43:34 +0200
Subject: [PATCH 17/29] Change qhelp explanation

---
 .../experimental/Security/CWE-287/ImproperLdapAuth.qhelp   | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.qhelp b/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.qhelp
index 43f47927fbad..7f4b018bdf2c 100644
--- a/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.qhelp
+++ b/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.qhelp
@@ -3,8 +3,8 @@
   "qhelp.dtd">
 <qhelp>
 <overview>
-<p>If an LDAP query is built using string concatenation or string formatting, and it doesn't carry any kind of authentication, 
-anonymous binds causes an empty or None-set password to result in a successful authentication.</p>
+<p>If an LDAP query doesn't carry any kind of authentication, anonymous binds causes an empty or None-set password 
+to result in a successful authentication.</p>
 </overview>
 
 <recommendation>
@@ -12,8 +12,7 @@ anonymous binds causes an empty or None-set password to result in a successful a
 </recommendation>
 
 <example>
-<p>In the following examples, the code accepts both <code>username</code> and <code>dc</code> from the user, 
-which it then uses to build a LDAP query and DN while the connection carries no authentication or binds anonymously.</p>
+<p>In the following examples, the code builds a LDAP query whose execution carries no authentication or binds anonymously.</p>
 
 <sample src="examples/auth_bad_2.py" />
 <sample src="examples/auth_bad_3.py" />

From 5704ac36dbf81eea33b41f3f2380bfa61149d97b Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Thu, 17 Jun 2021 17:44:08 +0200
Subject: [PATCH 18/29] Rework LDAP framework modeling

---
 .../semmle/python/frameworks/LDAP.qll         | 124 +++++++++++-------
 1 file changed, 74 insertions(+), 50 deletions(-)

diff --git a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
index 2843a695b189..5ba727675e86 100644
--- a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
+++ b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
@@ -19,6 +19,20 @@ private module LDAP {
    * See https://www.python-ldap.org/en/python-ldap-3.3.0/index.html
    */
   private module LDAP2 {
+    /** Gets a reference to the `ldap` module. */
+    API::Node ldap() { result = API::moduleImport("ldap") }
+
+    /** Returns a `ldap` module instance */
+    API::Node ldapInitialize() { result = ldap().getMember("initialize") }
+
+    /** Gets a reference to a `ldap` operation. */
+    private DataFlow::LocalSourceNode ldapOperation(DataFlow::TypeTracker t) {
+      t.start() and
+      result.(DataFlow::AttrRead).getObject().getALocalSource() = ldapInitialize().getACall()
+      or
+      exists(DataFlow::TypeTracker t2 | result = ldapOperation(t2).track(t2, t))
+    }
+
     /**
      * List of `ldap` methods used to execute a query.
      *
@@ -30,32 +44,45 @@ private module LDAP {
       }
     }
 
+    /** Gets a reference to a `ldap` operation. */
+    private DataFlow::Node ldapOperation() {
+      ldapOperation(DataFlow::TypeTracker::end()).flowsTo(result)
+    }
+
+    /** Gets a reference to a `ldap` query. */
+    private DataFlow::Node ldapQuery() {
+      result = ldapOperation() and
+      result.(DataFlow::AttrRead).getAttributeName() instanceof LDAP2QueryMethods
+    }
+
     /**
      * A class to find `ldap` methods executing a query.
      *
      * See `LDAP2QueryMethods`
      */
     private class LDAP2Query extends DataFlow::CallCfgNode, LDAPQuery::Range {
-      DataFlow::Node ldapQuery;
-
-      LDAP2Query() {
-        exists(DataFlow::AttrRead searchMethod |
-          this.getFunction() = searchMethod and
-          API::moduleImport("ldap").getMember("initialize").getACall() =
-            searchMethod.getObject().getALocalSource() and
-          searchMethod.getAttributeName() instanceof LDAP2QueryMethods and
-          (
-            ldapQuery = this.getArg(0)
-            or
-            (
-              ldapQuery = this.getArg(2) or
-              ldapQuery = this.getArgByName("filterstr")
-            )
-          )
-        )
+      LDAP2Query() { this.getFunction() = ldapQuery() }
+
+      override DataFlow::Node getQuery() {
+        result in [this.getArg(0), [this.getArg(2), this.getArgByName("filterstr")]]
       }
+    }
 
-      override DataFlow::Node getQuery() { result = ldapQuery }
+    /** Gets a reference to a `ldap` bind. */
+    private DataFlow::Node ldapBind() {
+      result = ldapOperation() and
+      result.(DataFlow::AttrRead).getAttributeName().matches("%bind%")
+    }
+
+    /**
+     * A class to find `ldap` methods binding a connection.
+     *
+     * See `LDAP2QueryMethods`
+     */
+    private class LDAP2Bind extends DataFlow::CallCfgNode, LDAPBind::Range {
+      LDAP2Bind() { this.getFunction() = ldapBind() }
+
+      override DataFlow::Node getPassword() { result = this.getArg(1) }
     }
 
     /**
@@ -64,9 +91,7 @@ private module LDAP {
      * See https://github.com/python-ldap/python-ldap/blob/7ce471e238cdd9a4dd8d17baccd1c9e05e6f894a/Lib/ldap/dn.py#L17
      */
     private class LDAP2EscapeDNCall extends DataFlow::CallCfgNode, LDAPEscape::Range {
-      LDAP2EscapeDNCall() {
-        this = API::moduleImport("ldap").getMember("dn").getMember("escape_dn_chars").getACall()
-      }
+      LDAP2EscapeDNCall() { this = ldap().getMember("dn").getMember("escape_dn_chars").getACall() }
 
       override DataFlow::Node getAnInput() { result = this.getArg(0) }
     }
@@ -78,8 +103,7 @@ private module LDAP {
      */
     private class LDAP2EscapeFilterCall extends DataFlow::CallCfgNode, LDAPEscape::Range {
       LDAP2EscapeFilterCall() {
-        this =
-          API::moduleImport("ldap").getMember("filter").getMember("escape_filter_chars").getACall()
+        this = ldap().getMember("filter").getMember("escape_filter_chars").getACall()
       }
 
       override DataFlow::Node getAnInput() { result = this.getArg(0) }
@@ -92,26 +116,38 @@ private module LDAP {
    * See https://pypi.org/project/ldap3/
    */
   private module LDAP3 {
+    /** Gets a reference to the `ldap3` module. */
+    API::Node ldap3() { result = API::moduleImport("ldap3") }
+
+    /** Gets a reference to the `ldap3` `utils` module. */
+    API::Node ldap3Utils() { result = ldap3().getMember("utils") }
+
+    /** Returns a `ldap3` module `Server` instance */
+    API::Node ldap3Server() { result = ldap3().getMember("Server") }
+
+    /** Returns a `ldap3` module `Connection` instance */
+    API::Node ldap3Connection() { result = ldap3().getMember("Connection") }
+
     /**
      * A class to find `ldap3` methods executing a query.
      */
     private class LDAP3Query extends DataFlow::CallCfgNode, LDAPQuery::Range {
-      DataFlow::Node ldapQuery;
-
       LDAP3Query() {
-        exists(DataFlow::AttrRead searchMethod |
-          this.getFunction() = searchMethod and
-          API::moduleImport("ldap3").getMember("Connection").getACall() =
-            searchMethod.getObject().getALocalSource() and
-          searchMethod.getAttributeName() = "search" and
-          (
-            ldapQuery = this.getArg(0) or
-            ldapQuery = this.getArg(1)
-          )
-        )
+        this.getFunction().(DataFlow::AttrRead).getObject().getALocalSource() =
+          ldap3Connection().getACall() and
+        this.getFunction().(DataFlow::AttrRead).getAttributeName() = "search"
       }
 
-      override DataFlow::Node getQuery() { result = ldapQuery }
+      override DataFlow::Node getQuery() { result in [this.getArg(0), this.getArg(1)] }
+    }
+
+    /**
+     * A class to find `ldap3` methods binding a connection.
+     */
+    class LDAP3Bind extends DataFlow::CallCfgNode, LDAPBind::Range {
+      LDAP3Bind() { this = ldap3Connection().getACall() }
+
+      override DataFlow::Node getPassword() { result = this.getArgByName("password") }
     }
 
     /**
@@ -120,14 +156,7 @@ private module LDAP {
      * See https://github.com/cannatag/ldap3/blob/4d33166f0869b929f59c6e6825a1b9505eb99967/ldap3/utils/dn.py#L390
      */
     private class LDAP3EscapeDNCall extends DataFlow::CallCfgNode, LDAPEscape::Range {
-      LDAP3EscapeDNCall() {
-        this =
-          API::moduleImport("ldap3")
-              .getMember("utils")
-              .getMember("dn")
-              .getMember("escape_rdn")
-              .getACall()
-      }
+      LDAP3EscapeDNCall() { this = ldap3Utils().getMember("dn").getMember("escape_rdn").getACall() }
 
       override DataFlow::Node getAnInput() { result = this.getArg(0) }
     }
@@ -139,12 +168,7 @@ private module LDAP {
      */
     private class LDAP3EscapeFilterCall extends DataFlow::CallCfgNode, LDAPEscape::Range {
       LDAP3EscapeFilterCall() {
-        this =
-          API::moduleImport("ldap3")
-              .getMember("utils")
-              .getMember("conv")
-              .getMember("escape_filter_chars")
-              .getACall()
+        this = ldap3Utils().getMember("conv").getMember("escape_filter_chars").getACall()
       }
 
       override DataFlow::Node getAnInput() { result = this.getArg(0) }

From 9cbb7e0899ef55a6d8c0f9d1dc3ab7cc42f72e06 Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Thu, 17 Jun 2021 17:53:58 +0200
Subject: [PATCH 19/29] Change query objective

---
 .../Security/CWE-287/ImproperLdapAuth.ql      | 11 +--
 .../experimental/semmle/python/Concepts.qll   | 11 +--
 .../semmle/python/frameworks/Stdlib.qll       | 83 -------------------
 .../python/security/LDAPImproperAuth.qll      | 41 +++------
 4 files changed, 16 insertions(+), 130 deletions(-)

diff --git a/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.ql b/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.ql
index f5e92a2ee6d2..27fc46bcdb24 100644
--- a/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.ql
+++ b/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.ql
@@ -1,7 +1,7 @@
 /**
  * @name Improper LDAP Authentication
  * @description A user-controlled query carries no authentication
- * @kind path-problem
+ * @kind problem
  * @problem.severity warning
  * @id py/improper-ldap-auth
  * @tags experimental
@@ -12,10 +12,7 @@
 // Determine precision above
 import python
 import experimental.semmle.python.security.LDAPImproperAuth
-import DataFlow::PathGraph
 
-from LDAPImproperAuthenticationConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
-where config.hasFlowPath(source, sink)
-select sink.getNode(), source, sink,
-  "$@ LDAP query parameter contains $@ and is executed without authentication.", sink.getNode(),
-  "This", source.getNode(), "a user-provided value"
+from LDAPBind ldapBind
+where authenticatesImproperly(ldapBind)
+select "The following LDAP bind operation is executed without authentication", ldapBind
diff --git a/python/ql/src/experimental/semmle/python/Concepts.qll b/python/ql/src/experimental/semmle/python/Concepts.qll
index f98e6961fd47..42af8abd64af 100644
--- a/python/ql/src/experimental/semmle/python/Concepts.qll
+++ b/python/ql/src/experimental/semmle/python/Concepts.qll
@@ -159,12 +159,7 @@ module LDAPBind {
     /**
      * Gets the argument containing the binding expression.
      */
-    abstract DataFlow::Node getPasswordNode();
-
-    /**
-     * Gets the argument containing the executed query.
-     */
-    abstract DataFlow::Node getQueryNode();
+    abstract DataFlow::Node getPassword();
   }
 }
 
@@ -179,7 +174,5 @@ class LDAPBind extends DataFlow::Node {
 
   LDAPBind() { this = range }
 
-  DataFlow::Node getPasswordNode() { result = range.getPasswordNode() }
-
-  DataFlow::Node getQueryNode() { result = range.getQueryNode() }
+  DataFlow::Node getPassword() { result = range.getPassword() }
 }
diff --git a/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll b/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll
index 980b948f7e50..4f3457e0a99b 100644
--- a/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll
+++ b/python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll
@@ -98,86 +98,3 @@ private module Re {
     override DataFlow::Node getRegexNode() { result = regexNode }
   }
 }
-
-/**
- * Provides models for Python's ldap-related libraries.
- */
-private module LDAP {
-  /**
-   * Provides models for Python's `ldap` library.
-   *
-   * See https://www.python-ldap.org/en/python-ldap-3.3.0/index.html
-   */
-  private module LDAP2 {
-    /**
-     * List of `ldap` methods used to execute a query.
-     *
-     * See https://www.python-ldap.org/en/python-ldap-3.3.0/reference/ldap.html#functions
-     */
-    private class LDAP2QueryMethods extends string {
-      LDAP2QueryMethods() {
-        this in ["search", "search_s", "search_st", "search_ext", "search_ext_s"]
-      }
-    }
-
-    /**
-     * A class to find `ldap` methods binding a connection.
-     *
-     * See `LDAP2QueryMethods`
-     */
-    class LDAP2Bind extends DataFlow::CallCfgNode, LDAPBind::Range {
-      DataFlow::Node queryNode;
-
-      LDAP2Bind() {
-        exists(
-          DataFlow::AttrRead bindMethod, DataFlow::CallCfgNode searchCall,
-          DataFlow::AttrRead searchMethod
-        |
-          this.getFunction() = bindMethod and
-          API::moduleImport("ldap").getMember("initialize").getACall() =
-            bindMethod.getObject().getALocalSource() and
-          bindMethod.getAttributeName().matches("%bind%") and
-          searchCall.getFunction() = searchMethod and
-          bindMethod.getObject().getALocalSource() = searchMethod.getObject().getALocalSource() and
-          searchMethod.getAttributeName() instanceof LDAP2QueryMethods and
-          (
-            queryNode = searchCall.getArg(2) or
-            queryNode = searchCall.getArgByName("filterstr")
-          )
-        )
-      }
-
-      override DataFlow::Node getPasswordNode() { result = this.getArg(1) }
-
-      override DataFlow::Node getQueryNode() { result = queryNode }
-    }
-  }
-
-  /**
-   * Provides models for Python's `ldap3` library.
-   *
-   * See https://pypi.org/project/ldap3/
-   */
-  private module LDAP3 {
-    /**
-     * A class to find `ldap3` methods binding a connection.
-     */
-    class LDAP3Bind extends DataFlow::CallCfgNode, LDAPBind::Range {
-      DataFlow::Node queryNode;
-
-      LDAP3Bind() {
-        exists(DataFlow::CallCfgNode searchCall, DataFlow::AttrRead searchMethod |
-          this = API::moduleImport("ldap3").getMember("Connection").getACall() and
-          searchMethod.getObject().getALocalSource() = this and
-          searchCall.getFunction() = searchMethod and
-          searchMethod.getAttributeName() = "search" and
-          queryNode = searchCall.getArg(1)
-        )
-      }
-
-      override DataFlow::Node getPasswordNode() { result = this.getArgByName("password") }
-
-      override DataFlow::Node getQueryNode() { result = queryNode }
-    }
-  }
-}
diff --git a/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll b/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll
index ef54d1612ac4..5d236f0d6a4d 100644
--- a/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll
+++ b/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll
@@ -8,35 +8,14 @@ import semmle.python.dataflow.new.DataFlow
 import semmle.python.dataflow.new.TaintTracking
 import semmle.python.dataflow.new.RemoteFlowSources
 
-/**
- * A class to find `LDAPBind` methods using an empty password or set as None.
- */
-class LDAPImproperAuthSink extends DataFlow::Node {
-  LDAPImproperAuthSink() {
-    exists(LDAPBind ldapBind |
-      (
-        (
-          DataFlow::localFlow(DataFlow::exprNode(any(None noneName)), ldapBind.getPasswordNode()) or
-          not exists(ldapBind.getPasswordNode())
-        )
-        or
-        exists(StrConst emptyString |
-          emptyString.getText() = "" and
-          DataFlow::localFlow(DataFlow::exprNode(emptyString), ldapBind.getPasswordNode())
-        )
-      ) and
-      this = ldapBind.getQueryNode()
-    )
-  }
-}
-
-/**
- * A taint-tracking configuration for detecting LDAP improper authentications.
- */
-class LDAPImproperAuthenticationConfig extends TaintTracking::Configuration {
-  LDAPImproperAuthenticationConfig() { this = "LDAPImproperAuthenticationConfig" }
-
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof LDAPImproperAuthSink }
+predicate authenticatesImproperly(LDAPBind ldapBind) {
+  (
+    DataFlow::localFlow(DataFlow::exprNode(any(None noneName)), ldapBind.getPassword()) or
+    not exists(ldapBind.getPassword())
+  )
+  or
+  exists(StrConst emptyString |
+    emptyString.getText() = "" and
+    DataFlow::localFlow(DataFlow::exprNode(emptyString), ldapBind.getPassword())
+  )
 }

From 1d7ddce8db9ecec5dba4f00ffb0a6f33d223bb47 Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Thu, 17 Jun 2021 18:10:43 +0200
Subject: [PATCH 20/29] Update .expected

---
 .../CWE-287/examples/ImproperLdapAuth.qlref   |  1 -
 .../CWE-287/ImproperLdapAuth.expected         | 94 ++-----------------
 2 files changed, 7 insertions(+), 88 deletions(-)
 delete mode 100644 python/ql/src/experimental/Security/CWE-287/examples/ImproperLdapAuth.qlref

diff --git a/python/ql/src/experimental/Security/CWE-287/examples/ImproperLdapAuth.qlref b/python/ql/src/experimental/Security/CWE-287/examples/ImproperLdapAuth.qlref
deleted file mode 100644
index 9f5c6e4c43f4..000000000000
--- a/python/ql/src/experimental/Security/CWE-287/examples/ImproperLdapAuth.qlref
+++ /dev/null
@@ -1 +0,0 @@
-experimental/Security/CWE-287/ImproperLdapAuth.ql
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.expected b/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.expected
index 1712533de774..d3d868c16eee 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.expected
+++ b/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.expected
@@ -1,87 +1,7 @@
-edges
-| auth_bad_2.py:14:10:14:16 | ControlFlowNode for request | auth_bad_2.py:15:21:15:27 | ControlFlowNode for request |
-| auth_bad_2.py:14:10:14:16 | ControlFlowNode for request | auth_bad_2.py:15:21:15:32 | ControlFlowNode for Attribute |
-| auth_bad_2.py:15:21:15:27 | ControlFlowNode for request | auth_bad_2.py:15:21:15:32 | ControlFlowNode for Attribute |
-| auth_bad_2.py:15:21:15:32 | ControlFlowNode for Attribute | auth_bad_2.py:15:21:15:42 | ControlFlowNode for Subscript |
-| auth_bad_2.py:15:21:15:42 | ControlFlowNode for Subscript | auth_bad_2.py:19:61:19:73 | ControlFlowNode for search_filter |
-| auth_bad_2.py:29:10:29:16 | ControlFlowNode for request | auth_bad_2.py:30:21:30:27 | ControlFlowNode for request |
-| auth_bad_2.py:29:10:29:16 | ControlFlowNode for request | auth_bad_2.py:30:21:30:32 | ControlFlowNode for Attribute |
-| auth_bad_2.py:30:21:30:27 | ControlFlowNode for request | auth_bad_2.py:30:21:30:32 | ControlFlowNode for Attribute |
-| auth_bad_2.py:30:21:30:32 | ControlFlowNode for Attribute | auth_bad_2.py:30:21:30:42 | ControlFlowNode for Subscript |
-| auth_bad_2.py:30:21:30:42 | ControlFlowNode for Subscript | auth_bad_2.py:34:61:34:73 | ControlFlowNode for search_filter |
-| auth_bad_2.py:44:10:44:16 | ControlFlowNode for request | auth_bad_2.py:45:21:45:27 | ControlFlowNode for request |
-| auth_bad_2.py:44:10:44:16 | ControlFlowNode for request | auth_bad_2.py:45:21:45:32 | ControlFlowNode for Attribute |
-| auth_bad_2.py:45:21:45:27 | ControlFlowNode for request | auth_bad_2.py:45:21:45:32 | ControlFlowNode for Attribute |
-| auth_bad_2.py:45:21:45:32 | ControlFlowNode for Attribute | auth_bad_2.py:45:21:45:42 | ControlFlowNode for Subscript |
-| auth_bad_2.py:45:21:45:42 | ControlFlowNode for Subscript | auth_bad_2.py:49:61:49:73 | ControlFlowNode for search_filter |
-| auth_bad_2.py:59:10:59:16 | ControlFlowNode for request | auth_bad_2.py:60:21:60:27 | ControlFlowNode for request |
-| auth_bad_2.py:59:10:59:16 | ControlFlowNode for request | auth_bad_2.py:60:21:60:32 | ControlFlowNode for Attribute |
-| auth_bad_2.py:60:21:60:27 | ControlFlowNode for request | auth_bad_2.py:60:21:60:32 | ControlFlowNode for Attribute |
-| auth_bad_2.py:60:21:60:32 | ControlFlowNode for Attribute | auth_bad_2.py:60:21:60:42 | ControlFlowNode for Subscript |
-| auth_bad_2.py:60:21:60:42 | ControlFlowNode for Subscript | auth_bad_2.py:64:61:64:73 | ControlFlowNode for search_filter |
-| auth_bad_3.py:14:10:14:16 | ControlFlowNode for request | auth_bad_3.py:15:21:15:27 | ControlFlowNode for request |
-| auth_bad_3.py:14:10:14:16 | ControlFlowNode for request | auth_bad_3.py:15:21:15:32 | ControlFlowNode for Attribute |
-| auth_bad_3.py:15:21:15:27 | ControlFlowNode for request | auth_bad_3.py:15:21:15:32 | ControlFlowNode for Attribute |
-| auth_bad_3.py:15:21:15:32 | ControlFlowNode for Attribute | auth_bad_3.py:15:21:15:42 | ControlFlowNode for Subscript |
-| auth_bad_3.py:15:21:15:42 | ControlFlowNode for Subscript | auth_bad_3.py:19:51:19:63 | ControlFlowNode for search_filter |
-| auth_bad_3.py:29:10:29:16 | ControlFlowNode for request | auth_bad_3.py:30:21:30:27 | ControlFlowNode for request |
-| auth_bad_3.py:29:10:29:16 | ControlFlowNode for request | auth_bad_3.py:30:21:30:32 | ControlFlowNode for Attribute |
-| auth_bad_3.py:30:21:30:27 | ControlFlowNode for request | auth_bad_3.py:30:21:30:32 | ControlFlowNode for Attribute |
-| auth_bad_3.py:30:21:30:32 | ControlFlowNode for Attribute | auth_bad_3.py:30:21:30:42 | ControlFlowNode for Subscript |
-| auth_bad_3.py:30:21:30:42 | ControlFlowNode for Subscript | auth_bad_3.py:34:51:34:63 | ControlFlowNode for search_filter |
-| auth_bad_3.py:44:10:44:16 | ControlFlowNode for request | auth_bad_3.py:45:21:45:27 | ControlFlowNode for request |
-| auth_bad_3.py:44:10:44:16 | ControlFlowNode for request | auth_bad_3.py:45:21:45:32 | ControlFlowNode for Attribute |
-| auth_bad_3.py:45:21:45:27 | ControlFlowNode for request | auth_bad_3.py:45:21:45:32 | ControlFlowNode for Attribute |
-| auth_bad_3.py:45:21:45:32 | ControlFlowNode for Attribute | auth_bad_3.py:45:21:45:42 | ControlFlowNode for Subscript |
-| auth_bad_3.py:45:21:45:42 | ControlFlowNode for Subscript | auth_bad_3.py:49:51:49:63 | ControlFlowNode for search_filter |
-nodes
-| auth_bad_2.py:14:10:14:16 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
-| auth_bad_2.py:15:21:15:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
-| auth_bad_2.py:15:21:15:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
-| auth_bad_2.py:15:21:15:42 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
-| auth_bad_2.py:19:61:19:73 | ControlFlowNode for search_filter | semmle.label | ControlFlowNode for search_filter |
-| auth_bad_2.py:29:10:29:16 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
-| auth_bad_2.py:30:21:30:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
-| auth_bad_2.py:30:21:30:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
-| auth_bad_2.py:30:21:30:42 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
-| auth_bad_2.py:34:61:34:73 | ControlFlowNode for search_filter | semmle.label | ControlFlowNode for search_filter |
-| auth_bad_2.py:44:10:44:16 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
-| auth_bad_2.py:45:21:45:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
-| auth_bad_2.py:45:21:45:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
-| auth_bad_2.py:45:21:45:42 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
-| auth_bad_2.py:49:61:49:73 | ControlFlowNode for search_filter | semmle.label | ControlFlowNode for search_filter |
-| auth_bad_2.py:59:10:59:16 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
-| auth_bad_2.py:60:21:60:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
-| auth_bad_2.py:60:21:60:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
-| auth_bad_2.py:60:21:60:42 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
-| auth_bad_2.py:64:61:64:73 | ControlFlowNode for search_filter | semmle.label | ControlFlowNode for search_filter |
-| auth_bad_3.py:14:10:14:16 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
-| auth_bad_3.py:15:21:15:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
-| auth_bad_3.py:15:21:15:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
-| auth_bad_3.py:15:21:15:42 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
-| auth_bad_3.py:19:51:19:63 | ControlFlowNode for search_filter | semmle.label | ControlFlowNode for search_filter |
-| auth_bad_3.py:29:10:29:16 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
-| auth_bad_3.py:30:21:30:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
-| auth_bad_3.py:30:21:30:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
-| auth_bad_3.py:30:21:30:42 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
-| auth_bad_3.py:34:51:34:63 | ControlFlowNode for search_filter | semmle.label | ControlFlowNode for search_filter |
-| auth_bad_3.py:44:10:44:16 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
-| auth_bad_3.py:45:21:45:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
-| auth_bad_3.py:45:21:45:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
-| auth_bad_3.py:45:21:45:42 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
-| auth_bad_3.py:49:51:49:63 | ControlFlowNode for search_filter | semmle.label | ControlFlowNode for search_filter |
-#select
-| auth_bad_2.py:19:61:19:73 | ControlFlowNode for search_filter | auth_bad_2.py:14:10:14:16 | ControlFlowNode for request | auth_bad_2.py:19:61:19:73 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_2.py:19:61:19:73 | ControlFlowNode for search_filter | This | auth_bad_2.py:14:10:14:16 | ControlFlowNode for request | a user-provided value |
-| auth_bad_2.py:19:61:19:73 | ControlFlowNode for search_filter | auth_bad_2.py:15:21:15:27 | ControlFlowNode for request | auth_bad_2.py:19:61:19:73 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_2.py:19:61:19:73 | ControlFlowNode for search_filter | This | auth_bad_2.py:15:21:15:27 | ControlFlowNode for request | a user-provided value |
-| auth_bad_2.py:34:61:34:73 | ControlFlowNode for search_filter | auth_bad_2.py:29:10:29:16 | ControlFlowNode for request | auth_bad_2.py:34:61:34:73 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_2.py:34:61:34:73 | ControlFlowNode for search_filter | This | auth_bad_2.py:29:10:29:16 | ControlFlowNode for request | a user-provided value |
-| auth_bad_2.py:34:61:34:73 | ControlFlowNode for search_filter | auth_bad_2.py:30:21:30:27 | ControlFlowNode for request | auth_bad_2.py:34:61:34:73 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_2.py:34:61:34:73 | ControlFlowNode for search_filter | This | auth_bad_2.py:30:21:30:27 | ControlFlowNode for request | a user-provided value |
-| auth_bad_2.py:49:61:49:73 | ControlFlowNode for search_filter | auth_bad_2.py:44:10:44:16 | ControlFlowNode for request | auth_bad_2.py:49:61:49:73 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_2.py:49:61:49:73 | ControlFlowNode for search_filter | This | auth_bad_2.py:44:10:44:16 | ControlFlowNode for request | a user-provided value |
-| auth_bad_2.py:49:61:49:73 | ControlFlowNode for search_filter | auth_bad_2.py:45:21:45:27 | ControlFlowNode for request | auth_bad_2.py:49:61:49:73 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_2.py:49:61:49:73 | ControlFlowNode for search_filter | This | auth_bad_2.py:45:21:45:27 | ControlFlowNode for request | a user-provided value |
-| auth_bad_2.py:64:61:64:73 | ControlFlowNode for search_filter | auth_bad_2.py:59:10:59:16 | ControlFlowNode for request | auth_bad_2.py:64:61:64:73 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_2.py:64:61:64:73 | ControlFlowNode for search_filter | This | auth_bad_2.py:59:10:59:16 | ControlFlowNode for request | a user-provided value |
-| auth_bad_2.py:64:61:64:73 | ControlFlowNode for search_filter | auth_bad_2.py:60:21:60:27 | ControlFlowNode for request | auth_bad_2.py:64:61:64:73 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_2.py:64:61:64:73 | ControlFlowNode for search_filter | This | auth_bad_2.py:60:21:60:27 | ControlFlowNode for request | a user-provided value |
-| auth_bad_3.py:19:51:19:63 | ControlFlowNode for search_filter | auth_bad_3.py:14:10:14:16 | ControlFlowNode for request | auth_bad_3.py:19:51:19:63 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_3.py:19:51:19:63 | ControlFlowNode for search_filter | This | auth_bad_3.py:14:10:14:16 | ControlFlowNode for request | a user-provided value |
-| auth_bad_3.py:19:51:19:63 | ControlFlowNode for search_filter | auth_bad_3.py:15:21:15:27 | ControlFlowNode for request | auth_bad_3.py:19:51:19:63 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_3.py:19:51:19:63 | ControlFlowNode for search_filter | This | auth_bad_3.py:15:21:15:27 | ControlFlowNode for request | a user-provided value |
-| auth_bad_3.py:34:51:34:63 | ControlFlowNode for search_filter | auth_bad_3.py:29:10:29:16 | ControlFlowNode for request | auth_bad_3.py:34:51:34:63 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_3.py:34:51:34:63 | ControlFlowNode for search_filter | This | auth_bad_3.py:29:10:29:16 | ControlFlowNode for request | a user-provided value |
-| auth_bad_3.py:34:51:34:63 | ControlFlowNode for search_filter | auth_bad_3.py:30:21:30:27 | ControlFlowNode for request | auth_bad_3.py:34:51:34:63 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_3.py:34:51:34:63 | ControlFlowNode for search_filter | This | auth_bad_3.py:30:21:30:27 | ControlFlowNode for request | a user-provided value |
-| auth_bad_3.py:49:51:49:63 | ControlFlowNode for search_filter | auth_bad_3.py:44:10:44:16 | ControlFlowNode for request | auth_bad_3.py:49:51:49:63 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_3.py:49:51:49:63 | ControlFlowNode for search_filter | This | auth_bad_3.py:44:10:44:16 | ControlFlowNode for request | a user-provided value |
-| auth_bad_3.py:49:51:49:63 | ControlFlowNode for search_filter | auth_bad_3.py:45:21:45:27 | ControlFlowNode for request | auth_bad_3.py:49:51:49:63 | ControlFlowNode for search_filter | $@ LDAP query parameter contains $@ and is executed without authentication. | auth_bad_3.py:49:51:49:63 | ControlFlowNode for search_filter | This | auth_bad_3.py:45:21:45:27 | ControlFlowNode for request | a user-provided value |
+| The following LDAP bind operation is executed without authentication | auth_bad_2.py:18:5:18:42 | ControlFlowNode for Attribute() |
+| The following LDAP bind operation is executed without authentication | auth_bad_2.py:33:5:33:44 | ControlFlowNode for Attribute() |
+| The following LDAP bind operation is executed without authentication | auth_bad_2.py:48:5:48:43 | ControlFlowNode for Attribute() |
+| The following LDAP bind operation is executed without authentication | auth_bad_2.py:63:5:63:39 | ControlFlowNode for Attribute() |
+| The following LDAP bind operation is executed without authentication | auth_bad_3.py:18:12:18:57 | ControlFlowNode for Connection() |
+| The following LDAP bind operation is executed without authentication | auth_bad_3.py:33:12:33:55 | ControlFlowNode for Connection() |
+| The following LDAP bind operation is executed without authentication | auth_bad_3.py:48:12:48:42 | ControlFlowNode for Connection() |

From dfe16aae4c3e6748b606bd9acef4f17a24433582 Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Mon, 28 Jun 2021 10:46:13 +0200
Subject: [PATCH 21/29] Python: Handle both positional and keyword args for
 LDAP bind

---
 .../semmle/python/frameworks/LDAP.qll         |  8 ++++--
 .../CWE-287/ImproperLdapAuth.expected         | 11 +++++---
 .../Security/CWE-287/auth_bad_2.py            | 28 +++++++++++++++++++
 .../Security/CWE-287/auth_bad_3.py            | 16 ++++++++++-
 4 files changed, 56 insertions(+), 7 deletions(-)

diff --git a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
index 5ba727675e86..9531ceeecda2 100644
--- a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
+++ b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
@@ -82,7 +82,9 @@ private module LDAP {
     private class LDAP2Bind extends DataFlow::CallCfgNode, LDAPBind::Range {
       LDAP2Bind() { this.getFunction() = ldapBind() }
 
-      override DataFlow::Node getPassword() { result = this.getArg(1) }
+      override DataFlow::Node getPassword() {
+        result in [this.getArg(1), this.getArgByName("cred")]
+      }
     }
 
     /**
@@ -147,7 +149,9 @@ private module LDAP {
     class LDAP3Bind extends DataFlow::CallCfgNode, LDAPBind::Range {
       LDAP3Bind() { this = ldap3Connection().getACall() }
 
-      override DataFlow::Node getPassword() { result = this.getArgByName("password") }
+      override DataFlow::Node getPassword() {
+        result in [this.getArg(2), this.getArgByName("password")]
+      }
     }
 
     /**
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.expected b/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.expected
index d3d868c16eee..181e1baadba2 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.expected
+++ b/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.expected
@@ -1,7 +1,10 @@
 | The following LDAP bind operation is executed without authentication | auth_bad_2.py:18:5:18:42 | ControlFlowNode for Attribute() |
 | The following LDAP bind operation is executed without authentication | auth_bad_2.py:33:5:33:44 | ControlFlowNode for Attribute() |
 | The following LDAP bind operation is executed without authentication | auth_bad_2.py:48:5:48:43 | ControlFlowNode for Attribute() |
-| The following LDAP bind operation is executed without authentication | auth_bad_2.py:63:5:63:39 | ControlFlowNode for Attribute() |
-| The following LDAP bind operation is executed without authentication | auth_bad_3.py:18:12:18:57 | ControlFlowNode for Connection() |
-| The following LDAP bind operation is executed without authentication | auth_bad_3.py:33:12:33:55 | ControlFlowNode for Connection() |
-| The following LDAP bind operation is executed without authentication | auth_bad_3.py:48:12:48:42 | ControlFlowNode for Connection() |
+| The following LDAP bind operation is executed without authentication | auth_bad_2.py:62:5:62:52 | ControlFlowNode for Attribute() |
+| The following LDAP bind operation is executed without authentication | auth_bad_2.py:76:5:76:39 | ControlFlowNode for Attribute() |
+| The following LDAP bind operation is executed without authentication | auth_bad_2.py:91:5:91:48 | ControlFlowNode for Attribute() |
+| The following LDAP bind operation is executed without authentication | auth_bad_3.py:18:12:18:43 | ControlFlowNode for Connection() |
+| The following LDAP bind operation is executed without authentication | auth_bad_3.py:33:12:33:57 | ControlFlowNode for Connection() |
+| The following LDAP bind operation is executed without authentication | auth_bad_3.py:47:12:47:55 | ControlFlowNode for Connection() |
+| The following LDAP bind operation is executed without authentication | auth_bad_3.py:62:12:62:42 | ControlFlowNode for Connection() |
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_2.py b/python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_2.py
index cc64623015e3..3387c95599b9 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_2.py
+++ b/python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_2.py
@@ -48,6 +48,19 @@ def bind_s_example():
     ldap_connection.bind_s('cn=root', None)
     user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
 
+@app.route("/bind_s_example")
+def bind_s_example_kwargs():
+    """
+    A RemoteFlowSource is used directly as DN and search filter while the bind's password
+    is set to None
+    """
+
+    dn = request.args['dc']
+    search_filter = request.args['search']
+
+    ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
+    ldap_connection.bind_s(who='cn=root', cred=None)
+    user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
 
 @app.route("/bind_example")
 def bind_example():
@@ -64,5 +77,20 @@ def bind_example():
     user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
 
 
+@app.route("/bind_example")
+def bind_example():
+    """
+    A RemoteFlowSource is used directly as DN and search filter while the bind's password
+    is set to None
+    """
+
+    dn = request.args['dc']
+    search_filter = request.args['search']
+
+    ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
+    ldap_connection.bind(who='cn=root', cred="")
+    user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
+
+
 # if __name__ == "__main__":
 #     app.run(debug=True)
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_3.py b/python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_3.py
index b69722bf589e..402aff8b7e4c 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_3.py
+++ b/python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_3.py
@@ -15,10 +15,24 @@ def passwordNone():
     search_filter = request.args['search']
 
     srv = Server('servername', get_info=ALL)
-    conn = Connection(srv, user='user_dn', password=None)
+    conn = Connection(srv, 'user_dn', None)
     status, result, response, _ = conn.search(dn, search_filter)
 
 
+@app.route("/passwordNone")
+def passwordNoneKwargs():
+    """
+    A RemoteFlowSource is used directly as DN and search filter while the connection's password
+    is set to None
+    """
+
+    dn = request.args['dc']
+    search_filter = request.args['search']
+
+    srv = Server('servername', get_info=ALL)
+    conn = Connection(srv, user='user_dn', password=None)
+    status, result, response, _ = conn.search(dn, search_filter)
+
 @app.route("/passwordEmpty")
 def passwordEmpty():
     """

From b33f6a315c21b395fbddab38a76d1422330cf8bb Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Mon, 28 Jun 2021 10:51:01 +0200
Subject: [PATCH 22/29] Python: Fix select for py/improper-ldap-auth

---
 .../Security/CWE-287/ImproperLdapAuth.ql      |  2 +-
 .../CWE-287/ImproperLdapAuth.expected         | 20 +++++++++----------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.ql b/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.ql
index 27fc46bcdb24..560622943993 100644
--- a/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.ql
+++ b/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.ql
@@ -15,4 +15,4 @@ import experimental.semmle.python.security.LDAPImproperAuth
 
 from LDAPBind ldapBind
 where authenticatesImproperly(ldapBind)
-select "The following LDAP bind operation is executed without authentication", ldapBind
+select ldapBind, "The following LDAP bind operation is executed without authentication"
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.expected b/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.expected
index 181e1baadba2..7b27b7fcdd5b 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.expected
+++ b/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.expected
@@ -1,10 +1,10 @@
-| The following LDAP bind operation is executed without authentication | auth_bad_2.py:18:5:18:42 | ControlFlowNode for Attribute() |
-| The following LDAP bind operation is executed without authentication | auth_bad_2.py:33:5:33:44 | ControlFlowNode for Attribute() |
-| The following LDAP bind operation is executed without authentication | auth_bad_2.py:48:5:48:43 | ControlFlowNode for Attribute() |
-| The following LDAP bind operation is executed without authentication | auth_bad_2.py:62:5:62:52 | ControlFlowNode for Attribute() |
-| The following LDAP bind operation is executed without authentication | auth_bad_2.py:76:5:76:39 | ControlFlowNode for Attribute() |
-| The following LDAP bind operation is executed without authentication | auth_bad_2.py:91:5:91:48 | ControlFlowNode for Attribute() |
-| The following LDAP bind operation is executed without authentication | auth_bad_3.py:18:12:18:43 | ControlFlowNode for Connection() |
-| The following LDAP bind operation is executed without authentication | auth_bad_3.py:33:12:33:57 | ControlFlowNode for Connection() |
-| The following LDAP bind operation is executed without authentication | auth_bad_3.py:47:12:47:55 | ControlFlowNode for Connection() |
-| The following LDAP bind operation is executed without authentication | auth_bad_3.py:62:12:62:42 | ControlFlowNode for Connection() |
+| auth_bad_2.py:18:5:18:42 | ControlFlowNode for Attribute() | The following LDAP bind operation is executed without authentication |
+| auth_bad_2.py:33:5:33:44 | ControlFlowNode for Attribute() | The following LDAP bind operation is executed without authentication |
+| auth_bad_2.py:48:5:48:43 | ControlFlowNode for Attribute() | The following LDAP bind operation is executed without authentication |
+| auth_bad_2.py:62:5:62:52 | ControlFlowNode for Attribute() | The following LDAP bind operation is executed without authentication |
+| auth_bad_2.py:76:5:76:39 | ControlFlowNode for Attribute() | The following LDAP bind operation is executed without authentication |
+| auth_bad_2.py:91:5:91:48 | ControlFlowNode for Attribute() | The following LDAP bind operation is executed without authentication |
+| auth_bad_3.py:18:12:18:43 | ControlFlowNode for Connection() | The following LDAP bind operation is executed without authentication |
+| auth_bad_3.py:33:12:33:57 | ControlFlowNode for Connection() | The following LDAP bind operation is executed without authentication |
+| auth_bad_3.py:47:12:47:55 | ControlFlowNode for Connection() | The following LDAP bind operation is executed without authentication |
+| auth_bad_3.py:62:12:62:42 | ControlFlowNode for Connection() | The following LDAP bind operation is executed without authentication |

From 4a2c99a02194096562892e41355dcd664784998e Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Mon, 28 Jun 2021 10:51:31 +0200
Subject: [PATCH 23/29] Python: Inline `LDAPImproperAuth.qll`

Since having it inlined makes the query a bit easier to read. We
obviously need to share it if we want to share this predicate, but for
now that does not seem to be the case.
---
 .../Security/CWE-287/ImproperLdapAuth.ql      | 15 ++++++++++++-
 .../python/security/LDAPImproperAuth.qll      | 21 -------------------
 2 files changed, 14 insertions(+), 22 deletions(-)
 delete mode 100644 python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll

diff --git a/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.ql b/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.ql
index 560622943993..5c27d6417859 100644
--- a/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.ql
+++ b/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.ql
@@ -11,7 +11,20 @@
 
 // Determine precision above
 import python
-import experimental.semmle.python.security.LDAPImproperAuth
+import experimental.semmle.python.Concepts
+import semmle.python.dataflow.new.DataFlow
+
+predicate authenticatesImproperly(LDAPBind ldapBind) {
+  (
+    DataFlow::localFlow(DataFlow::exprNode(any(None noneName)), ldapBind.getPassword()) or
+    not exists(ldapBind.getPassword())
+  )
+  or
+  exists(StrConst emptyString |
+    emptyString.getText() = "" and
+    DataFlow::localFlow(DataFlow::exprNode(emptyString), ldapBind.getPassword())
+  )
+}
 
 from LDAPBind ldapBind
 where authenticatesImproperly(ldapBind)
diff --git a/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll b/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll
deleted file mode 100644
index 5d236f0d6a4d..000000000000
--- a/python/ql/src/experimental/semmle/python/security/LDAPImproperAuth.qll
+++ /dev/null
@@ -1,21 +0,0 @@
-/**
- * Provides a taint-tracking configuration for detecting LDAP improper authentication vulnerabilities
- */
-
-import python
-import experimental.semmle.python.Concepts
-import semmle.python.dataflow.new.DataFlow
-import semmle.python.dataflow.new.TaintTracking
-import semmle.python.dataflow.new.RemoteFlowSources
-
-predicate authenticatesImproperly(LDAPBind ldapBind) {
-  (
-    DataFlow::localFlow(DataFlow::exprNode(any(None noneName)), ldapBind.getPassword()) or
-    not exists(ldapBind.getPassword())
-  )
-  or
-  exists(StrConst emptyString |
-    emptyString.getText() = "" and
-    DataFlow::localFlow(DataFlow::exprNode(emptyString), ldapBind.getPassword())
-  )
-}

From 5477b2e0d51dc80d7e17bc10dc46db03d2fd837a Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Mon, 28 Jun 2021 10:54:00 +0200
Subject: [PATCH 24/29] Python: Minor refactoring cleanup

---
 python/ql/src/experimental/semmle/python/frameworks/LDAP.qll | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
index 9531ceeecda2..9d5d093e7a7e 100644
--- a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
+++ b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
@@ -64,7 +64,7 @@ private module LDAP {
       LDAP2Query() { this.getFunction() = ldapQuery() }
 
       override DataFlow::Node getQuery() {
-        result in [this.getArg(0), [this.getArg(2), this.getArgByName("filterstr")]]
+        result in [this.getArg(0), this.getArg(2), this.getArgByName("filterstr")]
       }
     }
 

From b9422518b354f537a9f3377df51d57936e0a722f Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Mon, 28 Jun 2021 14:00:00 +0200
Subject: [PATCH 25/29] Rephrase .qhelp

---
 .../src/experimental/Security/CWE-287/ImproperLdapAuth.qhelp  | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.qhelp b/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.qhelp
index 7f4b018bdf2c..6f7c2b775701 100644
--- a/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.qhelp
+++ b/python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.qhelp
@@ -8,7 +8,7 @@ to result in a successful authentication.</p>
 </overview>
 
 <recommendation>
-<p>Use a strong password while establishing a LDAP connection to execute a query a user controls.</p>
+<p>Use a non-empty password while establishing an LDAP connection.</p>
 </recommendation>
 
 <example>
@@ -17,7 +17,7 @@ to result in a successful authentication.</p>
 <sample src="examples/auth_bad_2.py" />
 <sample src="examples/auth_bad_3.py" />
 
-<p>In the third and fourth examples, the authentication is established using a password from a secure source such as environment variables.</p>
+<p>In the third and fourth examples, the authentication is established using a password.</p>
 
 <sample src="examples/auth_good_2.py" />
 <sample src="examples/auth_good_3.py" />

From 1d4d8ab6e0cd53f9785072384891e88de5dbbd70 Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Mon, 28 Jun 2021 14:16:52 +0200
Subject: [PATCH 26/29] Fix tests

---
 .../Security/CWE-287/examples/auth_bad_2.py   |  6 ++-
 .../Security/CWE-287/examples/auth_bad_3.py   |  7 +--
 .../Security/CWE-287/examples/auth_good_2.py  |  9 ++--
 .../Security/CWE-287/examples/auth_good_3.py  | 10 ++---
 .../Security/CWE-287/auth_bad_2.py            | 44 +++++++++----------
 .../Security/CWE-287/auth_bad_3.py            | 30 ++++++-------
 .../Security/CWE-287/auth_good_2.py           | 39 ++++++++--------
 .../Security/CWE-287/auth_good_3.py           | 12 ++---
 8 files changed, 76 insertions(+), 81 deletions(-)

diff --git a/python/ql/src/experimental/Security/CWE-287/examples/auth_bad_2.py b/python/ql/src/experimental/Security/CWE-287/examples/auth_bad_2.py
index c893016ee343..550305de5e71 100644
--- a/python/ql/src/experimental/Security/CWE-287/examples/auth_bad_2.py
+++ b/python/ql/src/experimental/Security/CWE-287/examples/auth_bad_2.py
@@ -1,11 +1,13 @@
 from flask import request, Flask
 import ldap
+import ldap.filter
+import ldap.dn
 
 
 @app.route("/bind_example")
 def bind_example():
-    dn = request.args['dc']
-    search_filter = request.args['search']
+    dn = "dc={}".format(ldap.dn.escape_dn_chars(request.args['dc']))
+    search_filter = "(user={})".format(ldap.filter.escape_filter_chars(request.args['search']))
 
     ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
     ldap_connection.bind('cn=root', "")
diff --git a/python/ql/src/experimental/Security/CWE-287/examples/auth_bad_3.py b/python/ql/src/experimental/Security/CWE-287/examples/auth_bad_3.py
index 2cce5b73bd5b..b28aad4155fc 100644
--- a/python/ql/src/experimental/Security/CWE-287/examples/auth_bad_3.py
+++ b/python/ql/src/experimental/Security/CWE-287/examples/auth_bad_3.py
@@ -1,11 +1,12 @@
 from ldap3 import Server, Connection, ALL
 from flask import request, Flask
-
+from ldap3.utils.dn import escape_rdn
+from ldap3.utils.conv import escape_filter_chars
 
 @app.route("/passwordNone")
 def passwordNone():
-    dn = request.args['dc']
-    search_filter = request.args['search']
+    dn = "dc={}".format(escape_rdn(request.args['dc']))
+    search_filter = "(user={})".format(escape_filter_chars(request.args['search']))
 
     srv = Server('servername', get_info=ALL)
     conn = Connection(srv, user='user_dn', password=None)
diff --git a/python/ql/src/experimental/Security/CWE-287/examples/auth_good_2.py b/python/ql/src/experimental/Security/CWE-287/examples/auth_good_2.py
index e5fc88f56d1c..c08606321711 100644
--- a/python/ql/src/experimental/Security/CWE-287/examples/auth_good_2.py
+++ b/python/ql/src/experimental/Security/CWE-287/examples/auth_good_2.py
@@ -1,13 +1,14 @@
 from flask import request, Flask
 import ldap
-import os
+import ldap.filter
+import ldap.dn
 
 
 @app.route("/bind_example")
 def bind_example():
-    dn = request.args['dc']
-    search_filter = request.args['search']
+    dn = "dc={}".format(ldap.dn.escape_dn_chars(request.args['dc']))
+    search_filter = "(user={})".format(ldap.filter.escape_filter_chars(request.args['search']))
 
     ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
-    ldap_connection.bind('cn=root', os.environ.get('LDAP_PASSWORD'))
+    ldap_connection.bind('cn=root', "SecurePa$$!")
     user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
diff --git a/python/ql/src/experimental/Security/CWE-287/examples/auth_good_3.py b/python/ql/src/experimental/Security/CWE-287/examples/auth_good_3.py
index ae554fa9f7d2..0d93943831b1 100644
--- a/python/ql/src/experimental/Security/CWE-287/examples/auth_good_3.py
+++ b/python/ql/src/experimental/Security/CWE-287/examples/auth_good_3.py
@@ -1,14 +1,14 @@
 from ldap3 import Server, Connection, ALL
 from flask import request, Flask
-import os
-
+from ldap3.utils.dn import escape_rdn
+from ldap3.utils.conv import escape_filter_chars
 
 @app.route("/passwordFromEnv")
 def passwordFromEnv():
-    dn = request.args['dc']
-    search_filter = request.args['search']
+    dn = "dc={}".format(escape_rdn(request.args['dc']))
+    search_filter = "(user={})".format(escape_filter_chars(request.args['search']))
 
     srv = Server('servername', get_info=ALL)
     conn = Connection(srv, user='user_dn',
-                      password=os.environ.get('LDAP_PASSWORD'))
+                      password="SecurePa$$!")
     status, result, response, _ = conn.search(dn, search_filter)
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_2.py b/python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_2.py
index 3387c95599b9..d37cc09cfd06 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_2.py
+++ b/python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_2.py
@@ -1,5 +1,7 @@
 from flask import request, Flask
 import ldap
+import ldap.filter
+import ldap.dn
 
 app = Flask(__name__)
 
@@ -7,12 +9,11 @@
 @app.route("/simple_bind_example")
 def simple_bind_example():
     """
-    A RemoteFlowSource is used directly as DN and search filter while the bind's password
-    is not set
+    The bind's password argument is not set
     """
 
-    dn = request.args['dc']
-    search_filter = request.args['search']
+    dn = "dc={}".format(ldap.dn.escape_dn_chars(request.args['dc']))
+    search_filter = "(user={})".format(ldap.filter.escape_filter_chars(request.args['search']))
 
     ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
     ldap_connection.simple_bind('cn=root')
@@ -22,12 +23,11 @@ def simple_bind_example():
 @app.route("/simple_bind_s_example")
 def simple_bind_s_example():
     """
-    A RemoteFlowSource is used directly as DN and search filter while the bind's password
-    is not set
+    The bind's password argument is not set
     """
 
-    dn = request.args['dc']
-    search_filter = request.args['search']
+    dn = "dc={}".format(ldap.dn.escape_dn_chars(request.args['dc']))
+    search_filter = "(user={})".format(ldap.filter.escape_filter_chars(request.args['search']))
 
     ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
     ldap_connection.simple_bind_s('cn=root')
@@ -37,12 +37,11 @@ def simple_bind_s_example():
 @app.route("/bind_s_example")
 def bind_s_example():
     """
-    A RemoteFlowSource is used directly as DN and search filter while the bind's password
-    is set to None
+    The bind's password argument is set to None
     """
 
-    dn = request.args['dc']
-    search_filter = request.args['search']
+    dn = "dc={}".format(ldap.dn.escape_dn_chars(request.args['dc']))
+    search_filter = "(user={})".format(ldap.filter.escape_filter_chars(request.args['search']))
 
     ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
     ldap_connection.bind_s('cn=root', None)
@@ -51,12 +50,11 @@ def bind_s_example():
 @app.route("/bind_s_example")
 def bind_s_example_kwargs():
     """
-    A RemoteFlowSource is used directly as DN and search filter while the bind's password
-    is set to None
+    The bind's password argument is set to None
     """
 
-    dn = request.args['dc']
-    search_filter = request.args['search']
+    dn = "dc={}".format(ldap.dn.escape_dn_chars(request.args['dc']))
+    search_filter = "(user={})".format(ldap.filter.escape_filter_chars(request.args['search']))
 
     ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
     ldap_connection.bind_s(who='cn=root', cred=None)
@@ -65,12 +63,11 @@ def bind_s_example_kwargs():
 @app.route("/bind_example")
 def bind_example():
     """
-    A RemoteFlowSource is used directly as DN and search filter while the bind's password
-    is set to None
+    The bind's password argument is an empty string
     """
 
-    dn = request.args['dc']
-    search_filter = request.args['search']
+    dn = "dc={}".format(ldap.dn.escape_dn_chars(request.args['dc']))
+    search_filter = "(user={})".format(ldap.filter.escape_filter_chars(request.args['search']))
 
     ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
     ldap_connection.bind('cn=root', "")
@@ -80,12 +77,11 @@ def bind_example():
 @app.route("/bind_example")
 def bind_example():
     """
-    A RemoteFlowSource is used directly as DN and search filter while the bind's password
-    is set to None
+    The bind's password argument is an empty string
     """
 
-    dn = request.args['dc']
-    search_filter = request.args['search']
+    dn = "dc={}".format(ldap.dn.escape_dn_chars(request.args['dc']))
+    search_filter = "(user={})".format(ldap.filter.escape_filter_chars(request.args['search']))
 
     ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
     ldap_connection.bind(who='cn=root', cred="")
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_3.py b/python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_3.py
index 402aff8b7e4c..2500b4cadb6b 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_3.py
+++ b/python/ql/test/experimental/query-tests/Security/CWE-287/auth_bad_3.py
@@ -1,5 +1,7 @@
 from ldap3 import Server, Connection, ALL
 from flask import request, Flask
+from ldap3.utils.dn import escape_rdn
+from ldap3.utils.conv import escape_filter_chars
 
 app = Flask(__name__)
 
@@ -7,12 +9,11 @@
 @app.route("/passwordNone")
 def passwordNone():
     """
-    A RemoteFlowSource is used directly as DN and search filter while the connection's password
-    is set to None
+    The bind's password argument is set to None
     """
 
-    dn = request.args['dc']
-    search_filter = request.args['search']
+    dn = "dc={}".format(escape_rdn(request.args['dc']))
+    search_filter = "(user={})".format(escape_filter_chars(request.args['search']))
 
     srv = Server('servername', get_info=ALL)
     conn = Connection(srv, 'user_dn', None)
@@ -22,12 +23,11 @@ def passwordNone():
 @app.route("/passwordNone")
 def passwordNoneKwargs():
     """
-    A RemoteFlowSource is used directly as DN and search filter while the connection's password
-    is set to None
+    The bind's password argument is set to None
     """
 
-    dn = request.args['dc']
-    search_filter = request.args['search']
+    dn = "dc={}".format(escape_rdn(request.args['dc']))
+    search_filter = "(user={})".format(escape_filter_chars(request.args['search']))
 
     srv = Server('servername', get_info=ALL)
     conn = Connection(srv, user='user_dn', password=None)
@@ -36,12 +36,11 @@ def passwordNoneKwargs():
 @app.route("/passwordEmpty")
 def passwordEmpty():
     """
-    A RemoteFlowSource is used directly as DN and search filter while the connection's password
-    is empty
+    The bind's password argument is an empty string
     """
 
-    dn = request.args['dc']
-    search_filter = request.args['search']
+    dn = "dc={}".format(escape_rdn(request.args['dc']))
+    search_filter = "(user={})".format(escape_filter_chars(request.args['search']))
 
     srv = Server('servername', get_info=ALL)
     conn = Connection(srv, user='user_dn', password="")
@@ -51,12 +50,11 @@ def passwordEmpty():
 @app.route("/notPassword")
 def notPassword():
     """
-    A RemoteFlowSource is used directly as DN and search filter while the connection's password
-    is not set
+    The bind's password argument is not set
     """
 
-    dn = request.args['dc']
-    search_filter = request.args['search']
+    dn = "dc={}".format(escape_rdn(request.args['dc']))
+    search_filter = "(user={})".format(escape_filter_chars(request.args['search']))
 
     srv = Server('servername', get_info=ALL)
     conn = Connection(srv, user='user_dn')
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-287/auth_good_2.py b/python/ql/test/experimental/query-tests/Security/CWE-287/auth_good_2.py
index 925f80a3e9d9..1c959efc613b 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-287/auth_good_2.py
+++ b/python/ql/test/experimental/query-tests/Security/CWE-287/auth_good_2.py
@@ -1,6 +1,7 @@
 from flask import request, Flask
 import ldap
-import os
+import ldap.filter
+import ldap.dn
 
 app = Flask(__name__)
 
@@ -8,60 +9,56 @@
 @app.route("/simple_bind_example")
 def simple_bind_example():
     """
-    A RemoteFlowSource is used directly as DN and search filter while the bind's password
-    is an environment variable
+    The bind's password argument is a non-empty string
     """
 
-    dn = request.args['dc']
-    search_filter = request.args['search']
+    dn = "dc={}".format(ldap.dn.escape_dn_chars(request.args['dc']))
+    search_filter = "(user={})".format(ldap.filter.escape_filter_chars(request.args['search']))
 
     ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
-    ldap_connection.simple_bind('cn=root', os.environ.get('LDAP_PASSWORD'))
+    ldap_connection.simple_bind('cn=root', "SecurePa$$!")
     user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
 
 
 @app.route("/simple_bind_s_example")
 def simple_bind_s_example():
     """
-    A RemoteFlowSource is used directly as DN and search filter while the bind's password
-    is an environment variable
+    The bind's password argument is a non-empty string
     """
 
-    dn = request.args['dc']
-    search_filter = request.args['search']
+    dn = "dc={}".format(ldap.dn.escape_dn_chars(request.args['dc']))
+    search_filter = "(user={})".format(ldap.filter.escape_filter_chars(request.args['search']))
 
     ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
-    ldap_connection.simple_bind_s('cn=root', os.environ.get('LDAP_PASSWORD'))
+    ldap_connection.simple_bind_s('cn=root', "SecurePa$$!")
     user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
 
 
 @app.route("/bind_s_example")
 def bind_s_example():
     """
-    A RemoteFlowSource is used directly as DN and search filter while the bind's password
-    is an environment variable
+    The bind's password argument is a non-empty string
     """
 
-    dn = request.args['dc']
-    search_filter = request.args['search']
+    dn = "dc={}".format(ldap.dn.escape_dn_chars(request.args['dc']))
+    search_filter = "(user={})".format(ldap.filter.escape_filter_chars(request.args['search']))
 
     ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
-    ldap_connection.bind_s('cn=root', os.environ.get('LDAP_PASSWORD'))
+    ldap_connection.bind_s('cn=root', "SecurePa$$!")
     user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
 
 
 @app.route("/bind_example")
 def bind_example():
     """
-    A RemoteFlowSource is used directly as DN and search filter while the bind's password
-    is an environment variable
+    The bind's password argument is a non-empty string
     """
 
-    dn = request.args['dc']
-    search_filter = request.args['search']
+    dn = "dc={}".format(ldap.dn.escape_dn_chars(request.args['dc']))
+    search_filter = "(user={})".format(ldap.filter.escape_filter_chars(request.args['search']))
 
     ldap_connection = ldap.initialize("ldap://127.0.0.1:1337")
-    ldap_connection.bind('cn=root', os.environ.get('LDAP_PASSWORD'))
+    ldap_connection.bind('cn=root', "SecurePa$$!")
     user = ldap_connection.search_s(dn, ldap.SCOPE_SUBTREE, search_filter)
 
 # if __name__ == "__main__":
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-287/auth_good_3.py b/python/ql/test/experimental/query-tests/Security/CWE-287/auth_good_3.py
index fd9174c14286..530900877db9 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-287/auth_good_3.py
+++ b/python/ql/test/experimental/query-tests/Security/CWE-287/auth_good_3.py
@@ -1,6 +1,7 @@
 from ldap3 import Server, Connection, ALL
 from flask import request, Flask
-import os
+from ldap3.utils.dn import escape_rdn
+from ldap3.utils.conv import escape_filter_chars
 
 app = Flask(__name__)
 
@@ -8,16 +9,15 @@
 @app.route("/passwordFromEnv")
 def passwordFromEnv():
     """
-    A RemoteFlowSource is used directly as DN and search filter while the connection's password
-    is an environment variable
+    The bind's password argument is a non-empty string
     """
 
-    dn = request.args['dc']
-    search_filter = request.args['search']
+    dn = "dc={}".format(escape_rdn(request.args['dc']))
+    search_filter = "(user={})".format(escape_filter_chars(request.args['search']))
 
     srv = Server('servername', get_info=ALL)
     conn = Connection(srv, user='user_dn',
-                      password=os.environ.get('LDAP_PASSWORD'))
+                      password="SecurePa$$!")
     status, result, response, _ = conn.search(dn, search_filter)
 
 # if __name__ == "__main__":

From 1d432af4985bcc0c2d1afc6d91ea517ff97c4223 Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Mon, 28 Jun 2021 14:18:27 +0200
Subject: [PATCH 27/29] Update `.expected`

---
 .../Security/CWE-287/ImproperLdapAuth.expected   | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.expected b/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.expected
index 7b27b7fcdd5b..766665ee1b6b 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.expected
+++ b/python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.expected
@@ -1,10 +1,10 @@
-| auth_bad_2.py:18:5:18:42 | ControlFlowNode for Attribute() | The following LDAP bind operation is executed without authentication |
+| auth_bad_2.py:19:5:19:42 | ControlFlowNode for Attribute() | The following LDAP bind operation is executed without authentication |
 | auth_bad_2.py:33:5:33:44 | ControlFlowNode for Attribute() | The following LDAP bind operation is executed without authentication |
-| auth_bad_2.py:48:5:48:43 | ControlFlowNode for Attribute() | The following LDAP bind operation is executed without authentication |
-| auth_bad_2.py:62:5:62:52 | ControlFlowNode for Attribute() | The following LDAP bind operation is executed without authentication |
-| auth_bad_2.py:76:5:76:39 | ControlFlowNode for Attribute() | The following LDAP bind operation is executed without authentication |
-| auth_bad_2.py:91:5:91:48 | ControlFlowNode for Attribute() | The following LDAP bind operation is executed without authentication |
-| auth_bad_3.py:18:12:18:43 | ControlFlowNode for Connection() | The following LDAP bind operation is executed without authentication |
+| auth_bad_2.py:47:5:47:43 | ControlFlowNode for Attribute() | The following LDAP bind operation is executed without authentication |
+| auth_bad_2.py:60:5:60:52 | ControlFlowNode for Attribute() | The following LDAP bind operation is executed without authentication |
+| auth_bad_2.py:73:5:73:39 | ControlFlowNode for Attribute() | The following LDAP bind operation is executed without authentication |
+| auth_bad_2.py:87:5:87:48 | ControlFlowNode for Attribute() | The following LDAP bind operation is executed without authentication |
+| auth_bad_3.py:19:12:19:43 | ControlFlowNode for Connection() | The following LDAP bind operation is executed without authentication |
 | auth_bad_3.py:33:12:33:57 | ControlFlowNode for Connection() | The following LDAP bind operation is executed without authentication |
-| auth_bad_3.py:47:12:47:55 | ControlFlowNode for Connection() | The following LDAP bind operation is executed without authentication |
-| auth_bad_3.py:62:12:62:42 | ControlFlowNode for Connection() | The following LDAP bind operation is executed without authentication |
+| auth_bad_3.py:46:12:46:55 | ControlFlowNode for Connection() | The following LDAP bind operation is executed without authentication |
+| auth_bad_3.py:60:12:60:42 | ControlFlowNode for Connection() | The following LDAP bind operation is executed without authentication |

From 2f9e6454a59ff853cc8cd99db8a523c1e5d340a4 Mon Sep 17 00:00:00 2001
From: jorgectf <jorgectf@protonmail.com>
Date: Tue, 29 Jun 2021 16:14:55 +0200
Subject: [PATCH 28/29] Hardcode `ldap2` binding functions

---
 .../semmle/python/frameworks/LDAP.qll          | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
index 9d5d093e7a7e..9d115959f19b 100644
--- a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
+++ b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
@@ -68,16 +68,30 @@ private module LDAP {
       }
     }
 
+    /**
+     * List of `ldap` methods used for binding.
+     *
+     * See https://www.python-ldap.org/en/python-ldap-3.3.0/reference/ldap.html#functions
+     */
+    private class LDAP2BindMethods extends string {
+      LDAP2BindMethods() {
+        this in [
+            "bind", "bind_s", "simple_bind", "simple_bind_s", "sasl_interactive_bind_s",
+            "sasl_non_interactive_bind_s", "sasl_external_bind_s", "sasl_gssapi_bind_s"
+          ]
+      }
+    }
+
     /** Gets a reference to a `ldap` bind. */
     private DataFlow::Node ldapBind() {
       result = ldapOperation() and
-      result.(DataFlow::AttrRead).getAttributeName().matches("%bind%")
+      result.(DataFlow::AttrRead).getAttributeName() instanceof LDAP2BindMethods
     }
 
     /**
      * A class to find `ldap` methods binding a connection.
      *
-     * See `LDAP2QueryMethods`
+     * See `LDAP2BindMethods`
      */
     private class LDAP2Bind extends DataFlow::CallCfgNode, LDAPBind::Range {
       LDAP2Bind() { this.getFunction() = ldapBind() }

From 42a997cbcbacd7b3169df09de94c18daabee7b3b Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Thu, 22 Jul 2021 15:59:13 +0200
Subject: [PATCH 29/29] Python: Fix deprecation warning

---
 python/ql/src/experimental/semmle/python/frameworks/LDAP.qll | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
index 9d115959f19b..83b1accafc1c 100644
--- a/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
+++ b/python/ql/src/experimental/semmle/python/frameworks/LDAP.qll
@@ -26,7 +26,7 @@ private module LDAP {
     API::Node ldapInitialize() { result = ldap().getMember("initialize") }
 
     /** Gets a reference to a `ldap` operation. */
-    private DataFlow::LocalSourceNode ldapOperation(DataFlow::TypeTracker t) {
+    private DataFlow::TypeTrackingNode ldapOperation(DataFlow::TypeTracker t) {
       t.start() and
       result.(DataFlow::AttrRead).getObject().getALocalSource() = ldapInitialize().getACall()
       or