Change query objective

Author: jorgectf

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

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

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 }
-    }
-  }
-}

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