Refactor LDAP3 stuff (untested)

python/ql/src/experimental/Security/CWE-090/LDAPInjection.ql

@@ -19,7 +19,8 @@ from
   LDAPQuery castedSink
 where
   config.hasFlowPath(source, sink) and
-  castedSink = sink.getNode()
+  castedSink = sink.getNode() //and
+// if exists(castedSink.getAttrs()) then
 select sink.getNode(), source, sink, "$@ LDAP query executes $@ as a $@ probably leaking $@.",
   sink.getNode(), "This", source.getNode(), "a user-provided value", castedSink.getLDAPNode(),
-  castedSink.getLDAPPart(), castedSink.getAttrList(), "this attribute(s)"
+  castedSink.getLDAPPart(), castedSink.getAttrs(), "this attribute(s)"

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

@@ -15,12 +15,6 @@ private import semmle.python.dataflow.new.TaintTracking
 private import experimental.semmle.python.Frameworks
 private import semmle.python.ApiGraphs
 
-/**
- * To-Do
- *
- * LDAPQuery -> collect functions executing a search filter/DN
- * LDAPEscape -> collect functions escaping a search filter/DN
- */
 module LDAPQuery {
   abstract class Range extends DataFlow::Node {
     abstract DataFlow::Node getLDAPNode();

python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll

@@ -30,18 +30,14 @@ private module LDAP {
           initCall = searchMethod.getObject().getALocalSource() and
           searchMethod.getAttributeName() instanceof LDAP2QueryMethods and
           (
+            ldapNode = this.getArg(0) and
+            ldapPart = "DN"
+            or
             (
               ldapNode = this.getArg(2) or
               ldapNode = this.getArgByName("filterstr")
             ) and
             ldapPart = "search_filter"
-            or
-            ldapNode = this.getArg(0) and
-            ldapPart = "DN"
-          ) and
-          ( // what if they're not set?
-            attrs = this.getArg(3) or
-            attrs = this.getArgByName("attrlist")
           )
         )
       }
@@ -50,7 +46,9 @@ private module LDAP {
 
       override string getLDAPPart() { result = ldapPart }
 
-      override DataFlow::Node getAttrs() { result = attrs }
+      override DataFlow::Node getAttrs() {
+        result = this.getArg(3) or result = this.getArgByName("attrlist")
+      }
     }
 
     private class LDAP2EscapeDN extends DataFlow::CallCfgNode, LDAPEscape::Range {
@@ -63,5 +61,86 @@ private module LDAP {
 
       override DataFlow::Node getEscapeNode() { result = escapeNode }
     }
+
+    private class LDAP2EscapeFilter extends DataFlow::CallCfgNode, LDAPEscape::Range {
+      DataFlow::Node escapeNode;
+
+      LDAP2EscapeFilter() {
+        this =
+          API::moduleImport("ldap").getMember("filter").getMember("escape_filter_chars").getACall() and
+        escapeNode = this.getArg(0)
+      }
+
+      override DataFlow::Node getEscapeNode() { result = escapeNode }
+    }
+  }
+
+  private module LDAP3 {
+    private class LDAP3QueryMethods extends string {
+      // pending to dig into this although https://github.com/cannatag/ldap3/blob/21001d9087c0d24c399eec433a261c455b7bc97f/ldap3/core/connection.py#L760
+      LDAP3QueryMethods() { this in ["search"] }
+    }
+
+    private class LDAP3Query extends DataFlow::CallCfgNode, LDAPQuery::Range {
+      DataFlow::Node ldapNode;
+      string ldapPart;
+      DataFlow::Node attrs;
+
+      LDAP3Query() {
+        exists(DataFlow::AttrRead searchMethod, DataFlow::CallCfgNode connCall |
+          this.getFunction() = searchMethod and
+          connCall = API::moduleImport("ldap3").getMember("Connection").getACall() and
+          connCall = searchMethod.getObject().getALocalSource() and
+          searchMethod.getAttributeName() instanceof LDAP3QueryMethods and
+          (
+            ldapNode = this.getArg(0) and
+            ldapPart = "DN"
+            or
+            ldapNode = this.getArg(1) and
+            ldapPart = "search_filter"
+          )
+        )
+      }
+
+      override DataFlow::Node getLDAPNode() { result = ldapNode }
+
+      override string getLDAPPart() { result = ldapPart }
+
+      override DataFlow::Node getAttrs() {
+        result = this.getArg(3) or result = this.getArgByName("attributes")
+      }
+    }
+
+    private class LDAP3EscapeDN extends DataFlow::CallCfgNode, LDAPEscape::Range {
+      DataFlow::Node escapeNode;
+
+      LDAP3EscapeDN() {
+        this =
+          API::moduleImport("ldap3")
+              .getMember("utils")
+              .getMember("dn")
+              .getMember("escape_rdn")
+              .getACall() and
+        escapeNode = this.getArg(0)
+      }
+
+      override DataFlow::Node getEscapeNode() { result = escapeNode }
+    }
+
+    private class LDAP3EscapeFilter extends DataFlow::CallCfgNode, LDAPEscape::Range {
+      DataFlow::Node escapeNode;
+
+      LDAP3EscapeFilter() {
+        this =
+          API::moduleImport("ldap3")
+              .getMember("utils")
+              .getMember("conv")
+              .getMember("escape_filter_chars")
+              .getACall() and
+        escapeNode = this.getArg(0)
+      }
+
+      override DataFlow::Node getEscapeNode() { result = escapeNode }
+    }
   }
 }

python/ql/src/experimental/semmle/python/security/injection/LDAPInjection.qll

@@ -17,5 +17,8 @@ class LDAPInjectionFlowConfig extends TaintTracking::Configuration {
   override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
 
   override predicate isSink(DataFlow::Node sink) { sink = any(LDAPQuery lQ).getLDAPNode() }
-  // override predicate isSanitizer(DataFlow::Node sanitizer) { sanitizer instanceof RemoteFlowSource } // any(LDAPEscape ldapEsc).getEscapeNode() }
+
+  override predicate isSanitizer(DataFlow::Node sanitizer) {
+    sanitizer = any(LDAPEscape lE).getEscapeNode()
+  }
 }