Python: Add LDAP Improper Authentication query

python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.qhelp

@@ -0,0 +1,31 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<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>
+<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 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" />
+
+<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>

python/ql/src/experimental/Security/CWE-287/ImproperLdapAuth.ql

@@ -0,0 +1,31 @@
+/**
+ * @name Improper LDAP Authentication
+ * @description A user-controlled query carries no authentication
+ * @kind problem
+ * @problem.severity warning
+ * @id py/improper-ldap-auth
+ * @tags experimental
+ *       security
+ *       external/cwe/cwe-287
+ */
+
+// Determine precision above
+import python
+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)
+select ldapBind, "The following LDAP bind operation is executed without authentication"

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)

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)

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)

python/ql/src/experimental/Security/CWE-287/examples/auth_good_3.py

@@ -0,0 +1,14 @@
+from ldap3 import Server, Connection, ALL
+from flask import request, Flask
+import os
+
+
+@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)

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

@@ -146,3 +146,33 @@ class LDAPEscape extends DataFlow::Node {
    */
   DataFlow::Node getAnInput() { result = range.getAnInput() }
 }
+
+/** 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 getPassword();
+  }
+}
+
+/**
+ * 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;
+
+  LDAPBind() { this = range }
+
+  DataFlow::Node getPassword() { result = range.getPassword() }
+}

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,47 @@ 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 in [this.getArg(1), this.getArgByName("cred")]
+      }
     }
 
     /**
@@ -64,9 +93,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 +105,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 +118,40 @@ 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 in [this.getArg(2), this.getArgByName("password")]
+      }
     }
 
     /**
@@ -120,14 +160,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 +172,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) }

python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.expected

@@ -0,0 +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: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 |

python/ql/test/experimental/query-tests/Security/CWE-287/ImproperLdapAuth.qlref

@@ -0,0 +1 @@
+experimental/Security/CWE-287/ImproperLdapAuth.ql