python: rewrite to separate configurations

source nodes get duplicated, so perhaps flow states
are actually better for performance?

python/ql/lib/semmle/python/security/dataflow/LdapInjection.qll

@@ -8,80 +8,42 @@ import semmle.python.dataflow.new.DataFlow
 import semmle.python.dataflow.new.TaintTracking
 import semmle.python.dataflow.new.RemoteFlowSources
 
-/**
- * A taint-tracking configuration for detecting LDAP injections.
- */
-class LDAPInjectionFlowConfig extends TaintTracking::Configuration {
-  LDAPInjectionFlowConfig() { this = "LDAPInjectionFlowConfig" }
+module LdapInjection {
+  import LdapInjectionCustomizations::LdapInjection
 
-  override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) {
-    source instanceof RemoteFlowSource and
-    state instanceof Unsafe
+  class DnConfiguration extends TaintTracking::Configuration {
+    DnConfiguration() { this = "LdapDnInjection" }
+
+    override predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+    override predicate isSink(DataFlow::Node sink) { sink instanceof DnSink }
+
+    override predicate isSanitizer(DataFlow::Node node) { node instanceof DnSanitizer }
+
+    override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+      guard instanceof DnSanitizerGuard
+    }
   }
 
-  override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) {
-    sink = any(LdapExecution ldap).getBaseDn() and
-    (state instanceof Unsafe or state instanceof UnsafeForDn)
+  class FilterConfiguration extends TaintTracking::Configuration {
+    FilterConfiguration() { this = "LdapFilterInjection" }
+
+    override predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+    override predicate isSink(DataFlow::Node sink) { sink instanceof FilterSink }
+
+    override predicate isSanitizer(DataFlow::Node node) { node instanceof FilterSanitizer }
+
+    override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+      guard instanceof FilterSanitizerGuard
+    }
+  }
+
+  import DataFlow::PathGraph
+
+  predicate ldapInjection(DataFlow::PathNode source, DataFlow::PathNode sink) {
+    any(DnConfiguration dnConfig).hasFlowPath(source, sink)
     or
-    sink = any(LdapExecution ldap).getFilter() and
-    (state instanceof Unsafe or state instanceof UnsafeForFilter)
-  }
-
-  override predicate isBarrier(DataFlow::Node node, DataFlow::FlowState state) {
-    // All additional flow steps signify a state change.
-    //    (n, `state`) --> (`node`, s)
-    // Thus, if a node in `state` transitions to `node` and a new state,
-    // then `state` should be blocked at `node`.
-    isAdditionalFlowStep(_, state, node, _)
-  }
-
-  override predicate isAdditionalFlowStep(
-    DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
-    DataFlow::FlowState state2
-  ) {
-    exists(LdapDnEscaping ldapDnEsc |
-      node1 = ldapDnEsc.getAnInput() and
-      node2 = ldapDnEsc.getOutput()
-    |
-      state1 instanceof Unsafe and
-      state2 instanceof UnsafeForFilter
-      or
-      state1 instanceof UnsafeForDn and
-      state2 instanceof Safe
-    )
-    or
-    exists(LdapFilterEscaping ldapFilterEsc |
-      node1 = ldapFilterEsc.getAnInput() and
-      node2 = ldapFilterEsc.getOutput()
-    |
-      state1 instanceof Unsafe and
-      state2 instanceof UnsafeForDn
-      or
-      state1 instanceof UnsafeForFilter and
-      state2 instanceof Safe
-    )
+    any(FilterConfiguration filterConfig).hasFlowPath(source, sink)
   }
 }
-
-/** A flow satte signifying generally unsafe input. */
-class Unsafe extends DataFlow::FlowState {
-  Unsafe() { this = "Unsafe" }
-}
-
-/** A flow state signifying input that is only unsafe for DNs. */
-class UnsafeForDn extends DataFlow::FlowState {
-  UnsafeForDn() { this = "UnsafeForDn" }
-}
-
-/** A flow state signifying input that is only unsafe for filter strings. */
-class UnsafeForFilter extends DataFlow::FlowState {
-  UnsafeForFilter() { this = "UnsafeForFilter" }
-}
-
-/**
- * A flow state that signifies safe input.
- * Including this makes `isAdditionalFlowStep` and `isBarrier` simpler.
- */
-class Safe extends DataFlow::FlowState {
-  Safe() { this = "Safe" }
-}

python/ql/lib/semmle/python/security/dataflow/LdapInjectionCustomizations.qll

@@ -0,0 +1,97 @@
+/**
+ * Provides default sources, sinks and sanitizers for detecting
+ * "ldap injection"
+ * vulnerabilities, as well as extension points for adding your own.
+ */
+
+private import python
+private import semmle.python.dataflow.new.DataFlow
+private import semmle.python.Concepts
+private import semmle.python.dataflow.new.RemoteFlowSources
+private import semmle.python.dataflow.new.BarrierGuards
+
+/**
+ * Provides default sources, sinks and sanitizers for detecting
+ * "ldap injection"
+ * vulnerabilities, as well as extension points for adding your own.
+ */
+module LdapInjection {
+  /**
+   * A data flow source for "ldap injection" vulnerabilities.
+   */
+  abstract class Source extends DataFlow::Node { }
+
+  /**
+   * A data flow sink for "ldap injection" vulnerabilities.
+   */
+  abstract class DnSink extends DataFlow::Node { }
+
+  /**
+   * A data flow sink for "ldap injection" vulnerabilities.
+   */
+  abstract class FilterSink extends DataFlow::Node { }
+
+  /**
+   * A sanitizer for "ldap injection" vulnerabilities.
+   */
+  abstract class DnSanitizer extends DataFlow::Node { }
+
+  /**
+   * A sanitizer for "ldap injection" vulnerabilities.
+   */
+  abstract class FilterSanitizer extends DataFlow::Node { }
+
+  /**
+   * A sanitizer guard for "ldap injection" vulnerabilities.
+   */
+  abstract class DnSanitizerGuard extends DataFlow::BarrierGuard { }
+
+  /**
+   * A sanitizer guard for "ldap injection" vulnerabilities.
+   */
+  abstract class FilterSanitizerGuard extends DataFlow::BarrierGuard { }
+
+  /**
+   * A source of remote user input, considered as a flow source.
+   */
+  class RemoteFlowSourceAsSource extends Source, RemoteFlowSource { }
+
+  /**
+   * A logging operation, considered as a flow sink.
+   */
+  class LdapExecutionAsDnSink extends DnSink {
+    LdapExecutionAsDnSink() { this = any(LdapExecution ldap).getBaseDn() }
+  }
+
+  /**
+   * A logging operation, considered as a flow sink.
+   */
+  class LdapExecutionAsFilterSink extends FilterSink {
+    LdapExecutionAsFilterSink() { this = any(LdapExecution ldap).getFilter() }
+  }
+
+  /**
+   * A comparison with a constant string, considered as a sanitizer-guard.
+   */
+  class StringConstCompareAsDnSanitizerGuard extends DnSanitizerGuard, StringConstCompare { }
+
+  /**
+   * A comparison with a constant string, considered as a sanitizer-guard.
+   */
+  class StringConstCompareAsFilterSanitizerGuard extends FilterSanitizerGuard, StringConstCompare {
+  }
+
+  /**
+   * A call to replace line breaks functions as a sanitizer.
+   */
+  class LdapDnEscapingSanitizer extends DnSanitizer, DataFlow::CallCfgNode {
+    LdapDnEscapingSanitizer() { this = any(LdapDnEscaping ldapDnEsc).getOutput() }
+  }
+
+  /**
+   * A call to replace line breaks functions as a sanitizer.
+   */
+  class LdapFilterEscapingSanitizer extends FilterSanitizer, DataFlow::CallCfgNode {
+    LdapFilterEscapingSanitizer() { this = any(LdapFilterEscaping ldapDnEsc).getOutput() }
+  }
+}