Merge pull request github#12803 from michaelnebel/csharp/refactordataflow3

C#: Re-factor dataflow queries to use the new API.

Author: michaelnebel

csharp/ql/lib/semmle/code/csharp/security/dataflow/TaintedPathQuery.qll

@@ -26,9 +26,11 @@ abstract class Sink extends DataFlow::ExprNode { }
 abstract class Sanitizer extends DataFlow::ExprNode { }
 
 /**
+ * DEPRECATED: Use `TaintedPath` instead.
+ *
  * A taint-tracking configuration for uncontrolled data in path expression vulnerabilities.
  */
-class TaintTrackingConfiguration extends TaintTracking::Configuration {
+deprecated class TaintTrackingConfiguration extends TaintTracking::Configuration {
   TaintTrackingConfiguration() { this = "TaintedPath" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -38,6 +40,22 @@ class TaintTrackingConfiguration extends TaintTracking::Configuration {
   override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
 }
 
+/**
+ * A taint-tracking configuration for uncontrolled data in path expression vulnerabilities.
+ */
+private module TaintedPathConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+  predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
+}
+
+/**
+ * A taint-tracking module for uncontrolled data in path expression vulnerabilities.
+ */
+module TaintedPath = TaintTracking::Global<TaintedPathConfig>;
+
 /** A source of remote user input. */
 class RemoteSource extends Source instanceof RemoteFlowSource { }
 

csharp/ql/lib/semmle/code/csharp/security/dataflow/UrlRedirectQuery.qll

@@ -33,9 +33,11 @@ abstract class Sanitizer extends DataFlow::ExprNode { }
 abstract deprecated class SanitizerGuard extends DataFlow::BarrierGuard { }
 
 /**
+ * DEPRECATED: Use `UrlRedirect` instead.
+ *
  * A taint-tracking configuration for reasoning about unvalidated URL redirect vulnerabilities.
  */
-class TaintTrackingConfiguration extends TaintTracking::Configuration {
+deprecated class TaintTrackingConfiguration extends TaintTracking::Configuration {
   TaintTrackingConfiguration() { this = "UrlRedirect" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -49,6 +51,22 @@ class TaintTrackingConfiguration extends TaintTracking::Configuration {
   }
 }
 
+/**
+ * A taint-tracking configuration for reasoning about unvalidated URL redirect vulnerabilities.
+ */
+private module UrlRedirectConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+  predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
+}
+
+/**
+ * A taint-tracking module for reasoning about unvalidated URL redirect vulnerabilities.
+ */
+module UrlRedirect = TaintTracking::Global<UrlRedirectConfig>;
+
 /** A source of remote user input. */
 class RemoteSource extends Source instanceof RemoteFlowSource { }
 

csharp/ql/lib/semmle/code/csharp/security/dataflow/XMLEntityInjectionQuery.qll

@@ -44,9 +44,11 @@ private class InsecureXmlSink extends Sink {
 abstract class Sanitizer extends DataFlow::Node { }
 
 /**
+ * DEPRECATED: Use `XmlEntityInjection` instead.
+ *
  * A taint-tracking configuration for untrusted user input used in XML processing.
  */
-class TaintTrackingConfiguration extends TaintTracking::Configuration {
+deprecated class TaintTrackingConfiguration extends TaintTracking::Configuration {
   TaintTrackingConfiguration() { this = "XMLInjection" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -61,6 +63,36 @@ class TaintTrackingConfiguration extends TaintTracking::Configuration {
   }
 }
 
+/**
+ * A taint-tracking configuration for untrusted user input used in XML processing.
+ */
+private module XmlEntityInjectionConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+  predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
+}
+
+/**
+ * A taint-tracking module for untrusted user input used in XML processing.
+ */
+module XmlEntityInjection implements DataFlow::GlobalFlowSig {
+  import TaintTracking::Global<XmlEntityInjectionConfig> as Super
+  import Super
+
+  /**
+   * Holds if data can flow from `source` to `sink`.
+   *
+   * The corresponding paths are generated from the end-points and the graph
+   * included in the module `PathGraph`.
+   */
+  predicate flowPath(XmlEntityInjection::PathNode source, XmlEntityInjection::PathNode sink) {
+    Super::flowPath(source, sink) and
+    exists(sink.getNode().(Sink).getReason())
+  }
+}
+
 private class SimpleTypeSanitizer extends Sanitizer, SimpleTypeSanitizedExpr { }
 
 private class GuidSanitizer extends Sanitizer, GuidSanitizedExpr { }

csharp/ql/lib/semmle/code/csharp/security/dataflow/XPathInjectionQuery.qll

@@ -24,9 +24,11 @@ abstract class Sink extends DataFlow::ExprNode { }
 abstract class Sanitizer extends DataFlow::ExprNode { }
 
 /**
+ * DEPRECATED: Use `XpathInjection` instead.
+ *
  * A taint-tracking configuration for untrusted user input used in XPath expression.
  */
-class TaintTrackingConfiguration extends TaintTracking::Configuration {
+deprecated class TaintTrackingConfiguration extends TaintTracking::Configuration {
   TaintTrackingConfiguration() { this = "XPathInjection" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -36,6 +38,32 @@ class TaintTrackingConfiguration extends TaintTracking::Configuration {
   override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
 }
 
+/**
+ * A taint-tracking configuration for untrusted user input used in XPath expression.
+ */
+module XpathInjectionConfig implements DataFlow::ConfigSig {
+  /**
+   * Holds if `source` is a relevant data flow source.
+   */
+  predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+  /**
+   * Holds if `sink` is a relevant data flow sink.
+   */
+  predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+  /**
+   * Holds if data flow through `node` is prohibited. This completely removes
+   * `node` from the data flow graph.
+   */
+  predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
+}
+
+/**
+ * A taint-tracking module for untrusted user input used in XPath expression.
+ */
+module XpathInjection = TaintTracking::Global<XpathInjectionConfig>;
+
 /** A source of remote user input. */
 class RemoteSource extends Source instanceof RemoteFlowSource { }
 

csharp/ql/lib/semmle/code/csharp/security/dataflow/ZipSlipQuery.qll

@@ -27,8 +27,12 @@ abstract class Sanitizer extends DataFlow::ExprNode { }
  */
 abstract deprecated class SanitizerGuard extends DataFlow::BarrierGuard { }
 
-/** A taint tracking configuration for Zip Slip */
-class TaintTrackingConfiguration extends TaintTracking::Configuration {
+/**
+ * DEPRECATED: Use `ZipSlip` instead.
+ *
+ * A taint tracking configuration for Zip Slip.
+ */
+deprecated class TaintTrackingConfiguration extends TaintTracking::Configuration {
   TaintTrackingConfiguration() { this = "ZipSlipTaintTracking" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -42,6 +46,22 @@ class TaintTrackingConfiguration extends TaintTracking::Configuration {
   }
 }
 
+/**
+ * A taint tracking configuration for Zip Slip.
+ */
+private module ZipSlipConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+  predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
+}
+
+/**
+ * A taint tracking module for Zip Slip.
+ */
+module ZipSlip = TaintTracking::Global<ZipSlipConfig>;
+
 /** An access to the `FullName` property of a `ZipArchiveEntry`. */
 class ArchiveFullNameSource extends Source {
   ArchiveFullNameSource() {

csharp/ql/src/Likely Bugs/LeapYear/UnsafeYearConstruction.ql

@@ -10,32 +10,30 @@
  */
 
 import csharp
-import DataFlow::PathGraph
+import UnsafeYearCreationFromArithmetic::PathGraph
 
-class UnsafeYearCreationFromArithmeticConfiguration extends TaintTracking::Configuration {
-  UnsafeYearCreationFromArithmeticConfiguration() {
-    this = "UnsafeYearCreationFromArithmeticConfiguration"
-  }
-
-  override predicate isSource(DataFlow::Node source) {
+module UnsafeYearCreationFromArithmeticConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
     exists(ArithmeticOperation ao, PropertyAccess pa | ao = source.asExpr() |
       pa = ao.getAChild*() and
       pa.getProperty().hasQualifiedName("System.DateTime", "Year")
     )
   }
 
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     exists(ObjectCreation oc |
       sink.asExpr() = oc.getArgumentForName("year") and
       oc.getObjectType().getABaseType*().hasQualifiedName("System", "DateTime")
     )
   }
 }
 
+module UnsafeYearCreationFromArithmetic =
+  TaintTracking::Global<UnsafeYearCreationFromArithmeticConfig>;
+
 from
-  UnsafeYearCreationFromArithmeticConfiguration config, DataFlow::PathNode source,
-  DataFlow::PathNode sink
-where config.hasFlowPath(source, sink)
+  UnsafeYearCreationFromArithmetic::PathNode source, UnsafeYearCreationFromArithmetic::PathNode sink
+where UnsafeYearCreationFromArithmetic::flowPath(source, sink)
 select sink, source, sink,
   "This $@ based on a 'System.DateTime.Year' property is used in a construction of a new 'System.DateTime' object, flowing to the 'year' argument.",
   source, "arithmetic operation"

csharp/ql/src/Likely Bugs/ThreadUnsafeICryptoTransformLambda.ql

@@ -18,21 +18,20 @@ import csharp
 import ParallelSink
 import ICryptoTransform
 
-class NotThreadSafeCryptoUsageIntoParallelInvokeConfig extends TaintTracking::Configuration {
-  NotThreadSafeCryptoUsageIntoParallelInvokeConfig() {
-    this = "NotThreadSafeCryptoUsageIntoParallelInvokeConfig"
-  }
-
-  override predicate isSource(DataFlow::Node source) {
+module NotThreadSafeCryptoUsageIntoParallelInvokeConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
     source instanceof LambdaCapturingICryptoTransformSource
   }
 
-  override predicate isSink(DataFlow::Node sink) { sink instanceof ParallelSink }
+  predicate isSink(DataFlow::Node sink) { sink instanceof ParallelSink }
 }
 
-from Expr e, string m, LambdaExpr l, NotThreadSafeCryptoUsageIntoParallelInvokeConfig config
+module NotThreadSafeCryptoUsageIntoParallelInvoke =
+  TaintTracking::Global<NotThreadSafeCryptoUsageIntoParallelInvokeConfig>;
+
+from Expr e, string m, LambdaExpr l
 where
-  config.hasFlow(DataFlow::exprNode(l), DataFlow::exprNode(e)) and
+  NotThreadSafeCryptoUsageIntoParallelInvoke::flow(DataFlow::exprNode(l), DataFlow::exprNode(e)) and
   m =
     "A $@ seems to be used to start a new thread is capturing a local variable that either implements 'System.Security.Cryptography.ICryptoTransform' or has a field of this type."
 select e, m, l, "lambda expression"

csharp/ql/src/Security Features/CWE-022/TaintedPath.ql

@@ -16,9 +16,9 @@
 
 import csharp
 import semmle.code.csharp.security.dataflow.TaintedPathQuery
-import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph
+import TaintedPath::PathGraph
 
-from TaintTrackingConfiguration c, DataFlow::PathNode source, DataFlow::PathNode sink
-where c.hasFlowPath(source, sink)
+from TaintedPath::PathNode source, TaintedPath::PathNode sink
+where TaintedPath::flowPath(source, sink)
 select sink.getNode(), source, sink, "This path depends on a $@.", source.getNode(),
   "user-provided value"

csharp/ql/src/Security Features/CWE-022/ZipSlip.ql

@@ -14,10 +14,10 @@
 
 import csharp
 import semmle.code.csharp.security.dataflow.ZipSlipQuery
-import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph
+import ZipSlip::PathGraph
 
-from TaintTrackingConfiguration zipTaintTracking, DataFlow::PathNode source, DataFlow::PathNode sink
-where zipTaintTracking.hasFlowPath(source, sink)
+from ZipSlip::PathNode source, ZipSlip::PathNode sink
+where ZipSlip::flowPath(source, sink)
 select source.getNode(), source, sink,
   "Unsanitized archive entry, which may contain '..', is used in a $@.", sink.getNode(),
   "file system operation"

csharp/ql/src/Security Features/CWE-091/XMLInjection.ql

@@ -12,19 +12,17 @@
  */
 
 import csharp
-import DataFlow::PathGraph
 import semmle.code.csharp.security.dataflow.flowsources.Remote
 import semmle.code.csharp.frameworks.system.Xml
+import XmlInjection::PathGraph
 
 /**
  * A taint-tracking configuration for untrusted user input used in XML.
  */
-class TaintTrackingConfiguration extends TaintTracking::Configuration {
-  TaintTrackingConfiguration() { this = "XMLInjection" }
+module XmlInjectionConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
 
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     exists(MethodCall mc |
       mc.getTarget().hasName("WriteRaw") and
       mc.getTarget().getDeclaringType().getABaseType*().hasQualifiedName("System.Xml", "XmlWriter")
@@ -33,7 +31,7 @@ class TaintTrackingConfiguration extends TaintTracking::Configuration {
     )
   }
 
-  override predicate isSanitizer(DataFlow::Node node) {
+  predicate isBarrier(DataFlow::Node node) {
     exists(MethodCall mc |
       mc.getTarget().hasName("Escape") and
       mc.getTarget()
@@ -46,7 +44,12 @@ class TaintTrackingConfiguration extends TaintTracking::Configuration {
   }
 }
 
-from TaintTrackingConfiguration c, DataFlow::PathNode source, DataFlow::PathNode sink
-where c.hasFlowPath(source, sink)
+/**
+ * A taint-tracking module for untrusted user input used in XML.
+ */
+module XmlInjection = TaintTracking::Global<XmlInjectionConfig>;
+
+from XmlInjection::PathNode source, XmlInjection::PathNode sink
+where XmlInjection::flowPath(source, sink)
 select sink.getNode(), source, sink, "This XML element depends on a $@.", source.getNode(),
   "user-provided value"