Python: Handle both positional and keyword args for LDAP bind

Author: RasmusWL

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")]
+      }
     }
 
     /**

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() |

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)

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():
     """