Merge pull request #12675 from michaelnebel/csharp/refactorflowapi

C#: Re-factor tainttracking configurations to use the new API.
2026-02-05 01:31:08 +01:00 · 2023-03-30 10:54:11 +02:00
parent 6275a015a4 31e352afb0
commit dde37c64ac
13 changed files with 169 additions and 30 deletions
--- a/csharp/ql/lib/semmle/code/csharp/security/cryptography/EncryptionKeyDataFlowQuery.qll
+++ b/csharp/ql/lib/semmle/code/csharp/security/cryptography/EncryptionKeyDataFlowQuery.qll
@@ -64,9 +64,11 @@ class SymmetricEncryptionCreateDecryptorSink extends SymmetricEncryptionKeySink
 }

 /**
+ * DEPRECATED: Use `SymmetricKey` instead.
+ *
 * Symmetric Key Data Flow configuration.
 */
-class SymmetricKeyTaintTrackingConfiguration extends TaintTracking::Configuration {
+deprecated class SymmetricKeyTaintTrackingConfiguration extends TaintTracking::Configuration {
  SymmetricKeyTaintTrackingConfiguration() { this = "SymmetricKeyTaintTracking" }

  /** Holds if the node is a key source. */
@@ -78,3 +80,22 @@ class SymmetricKeyTaintTrackingConfiguration extends TaintTracking::Configuratio
  /** Holds if the node is a key sanitizer. */
  override predicate isSanitizer(DataFlow::Node sanitizer) { sanitizer instanceof KeySanitizer }
 }
+
+/**
+ * Symmetric Key Data Flow configuration.
+ */
+private module SymmetricKeyConfig implements DataFlow::ConfigSig {
+  /** Holds if the node is a key source. */
+  predicate isSource(DataFlow::Node src) { src instanceof KeySource }
+
+  /** Holds if the node is a symmetric encryption key sink. */
+  predicate isSink(DataFlow::Node sink) { sink instanceof SymmetricEncryptionKeySink }
+
+  /** Holds if the node is a key sanitizer. */
+  predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof KeySanitizer }
+}
+
+/**
+ * Symmetric Key Data Flow configuration.
+ */
+module SymmetricKey = TaintTracking::Global<SymmetricKeyConfig>;
--- a/csharp/ql/lib/semmle/code/csharp/security/cryptography/HardcodedSymmetricEncryptionKey.qll
+++ b/csharp/ql/lib/semmle/code/csharp/security/cryptography/HardcodedSymmetricEncryptionKey.qll
@@ -62,9 +62,11 @@ module HardcodedSymmetricEncryptionKey {
  }

  /**
+   * DEPRECATED: Use `HardCodedSymmetricEncryption` 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 = "HardcodedSymmetricEncryptionKey" }

    override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -85,4 +87,32 @@ module HardcodedSymmetricEncryptionKey {
      )
    }
  }
+
+  /**
+   * A taint-tracking configuration for uncontrolled data in path expression vulnerabilities.
+   */
+  private module HardCodedSymmetricEncryptionConfig 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 }
+
+    /**
+     * Since `CryptographicBuffer` uses native code inside, taint tracking doesn't pass through it.
+     * Need to create an additional custom step.
+     */
+    predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(MethodCall mc, CryptographicBuffer c |
+        pred.asExpr() = mc.getAnArgument() and
+        mc.getTarget() = c.getAMethod() and
+        succ.asExpr() = mc
+      )
+    }
+  }
+
+  /**
+   * A taint-tracking module for uncontrolled data in path expression vulnerabilities.
+   */
+  module HardCodedSymmetricEncryption = TaintTracking::Global<HardCodedSymmetricEncryptionConfig>;
 }
--- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/CodeInjectionQuery.qll
+++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/CodeInjectionQuery.qll
@@ -25,9 +25,11 @@ abstract class Sink extends DataFlow::ExprNode { }
 abstract class Sanitizer extends DataFlow::ExprNode { }

 /**
+ * DEPRECATED: Use `CodeInjection` instead.
+ *
 * A taint-tracking configuration for user input treated as code vulnerabilities.
 */
-class TaintTrackingConfiguration extends TaintTracking::Configuration {
+deprecated class TaintTrackingConfiguration extends TaintTracking::Configuration {
  TaintTrackingConfiguration() { this = "CodeInjection" }

  override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -37,6 +39,22 @@ class TaintTrackingConfiguration extends TaintTracking::Configuration {
  override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
 }

+/**
+ * A taint-tracking configuration for user input treated as code vulnerabilities.
+ */
+private module CodeInjectionConfig 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 user input treated as code vulnerabilities.
+ */
+module CodeInjection = TaintTracking::Global<CodeInjectionConfig>;
+
 /** A source of remote user input. */
 class RemoteSource extends Source instanceof RemoteFlowSource { }

--- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/CommandInjectionQuery.qll
+++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/CommandInjectionQuery.qll
@@ -23,9 +23,11 @@ abstract class Sink extends DataFlow::ExprNode { }
 abstract class Sanitizer extends DataFlow::ExprNode { }

 /**
+ * DEPRECATED: Use `CommandInjection` instead.
+ *
 * A taint-tracking configuration for command injection vulnerabilities.
 */
-class TaintTrackingConfiguration extends TaintTracking::Configuration {
+deprecated class TaintTrackingConfiguration extends TaintTracking::Configuration {
  TaintTrackingConfiguration() { this = "CommandInjection" }

  override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -35,6 +37,32 @@ class TaintTrackingConfiguration extends TaintTracking::Configuration {
  override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
 }

+/**
+ * A taint-tracking configuration for command injection vulnerabilities.
+ */
+module CommandInjectionConfig 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 command injection vulnerabilities.
+ */
+module CommandInjection = TaintTracking::Global<CommandInjectionConfig>;
+
 /** A source of remote user input. */
 class RemoteSource extends Source instanceof RemoteFlowSource { }

--- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/ConditionalBypassQuery.qll
+++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/ConditionalBypassQuery.qll
@@ -30,9 +30,11 @@ abstract class Sink extends DataFlow::ExprNode {
 abstract class Sanitizer extends DataFlow::ExprNode { }

 /**
+ * DEPRECATED: Use `ConditionalBypass` instead.
+ *
 * A taint-tracking configuration for user-controlled bypass of sensitive method.
 */
-class Configuration extends TaintTracking::Configuration {
+deprecated class Configuration extends TaintTracking::Configuration {
  Configuration() { this = "UserControlledBypassOfSensitiveMethodConfiguration" }

  override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -42,6 +44,22 @@ class Configuration extends TaintTracking::Configuration {
  override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
 }

+/**
+ * A taint-tracking configuration for user-controlled bypass of sensitive method.
+ */
+private module ConditionalBypassConfig 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 user-controlled bypass of sensitive method.
+ */
+module ConditionalBypass = TaintTracking::Global<ConditionalBypassConfig>;
+
 /** A source of remote user input. */
 class RemoteSource extends Source instanceof RemoteFlowSource { }

--- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/ExposureOfPrivateInformationQuery.qll
+++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/ExposureOfPrivateInformationQuery.qll
@@ -23,9 +23,11 @@ abstract class Sink extends DataFlow::ExprNode { }
 abstract class Sanitizer extends DataFlow::ExprNode { }

 /**
+ * DEPRECATED: Use `ExposureOfPrivateInformation` instead.
+ *
 * A taint-tracking configuration for private information flowing unencrypted to an external location.
 */
-class TaintTrackingConfiguration extends TaintTracking::Configuration {
+deprecated class TaintTrackingConfiguration extends TaintTracking::Configuration {
  TaintTrackingConfiguration() { this = "ExposureOfPrivateInformation" }

  override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -35,6 +37,22 @@ class TaintTrackingConfiguration extends TaintTracking::Configuration {
  override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
 }

+/**
+ * A taint-tracking configuration for private information flowing unencrypted to an external location.
+ */
+private module ExposureOfPrivateInformationConfig 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 private information flowing unencrypted to an external location.
+ */
+module ExposureOfPrivateInformation = TaintTracking::Global<ExposureOfPrivateInformationConfig>;
+
 private class PrivateDataSource extends Source {
  PrivateDataSource() { this.getExpr() instanceof PrivateDataExpr }
 }
--- a/Features/CWE-078/CommandInjection.ql
+++ b/Features/CWE-078/CommandInjection.ql
@@ -15,9 +15,9 @@

 import csharp
 import semmle.code.csharp.security.dataflow.CommandInjectionQuery
-import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph
+import CommandInjection::PathGraph

-from TaintTrackingConfiguration c, DataFlow::PathNode source, DataFlow::PathNode sink
-where c.hasFlowPath(source, sink)
+from CommandInjection::PathNode source, CommandInjection::PathNode sink
+where CommandInjection::flowPath(source, sink)
 select sink.getNode(), source, sink, "This command line depends on a $@.", source.getNode(),
  "user-provided value"
--- a/Features/CWE-078/StoredCommandInjection.ql
+++ b/Features/CWE-078/StoredCommandInjection.ql
@@ -16,13 +16,19 @@
 import csharp
 import semmle.code.csharp.security.dataflow.flowsources.Stored
 import semmle.code.csharp.security.dataflow.CommandInjectionQuery
-import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph
+import StoredCommandInjection::PathGraph

-class StoredTaintTrackingConfiguration extends TaintTrackingConfiguration {
-  override predicate isSource(DataFlow::Node source) { source instanceof StoredFlowSource }
+module StoredCommandInjectionConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof StoredFlowSource }
+
+  predicate isSink = CommandInjectionConfig::isSink/1;
+
+  predicate isBarrier = CommandInjectionConfig::isBarrier/1;
 }

-from StoredTaintTrackingConfiguration c, DataFlow::PathNode source, DataFlow::PathNode sink
-where c.hasFlowPath(source, sink)
+module StoredCommandInjection = TaintTracking::Global<StoredCommandInjectionConfig>;
+
+from StoredCommandInjection::PathNode source, StoredCommandInjection::PathNode sink
+where StoredCommandInjection::flowPath(source, sink)
 select sink.getNode(), source, sink, "This command line depends on a $@.", source.getNode(),
  "stored (potentially user-provided) value"
--- a/Features/CWE-094/CodeInjection.ql
+++ b/Features/CWE-094/CodeInjection.ql
@@ -15,9 +15,9 @@

 import csharp
 import semmle.code.csharp.security.dataflow.CodeInjectionQuery
-import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph
+import CodeInjection::PathGraph

-from TaintTrackingConfiguration c, DataFlow::PathNode source, DataFlow::PathNode sink
-where c.hasFlowPath(source, sink)
+from CodeInjection::PathNode source, CodeInjection::PathNode sink
+where CodeInjection::flowPath(source, sink)
 select sink.getNode(), source, sink, "This code compilation depends on a $@.", source.getNode(),
  "user-provided value"
--- a/Features/CWE-321/HardcodedEncryptionKey.ql
+++ b/Features/CWE-321/HardcodedEncryptionKey.ql
@@ -15,7 +15,7 @@

 import csharp
 import semmle.code.csharp.security.cryptography.EncryptionKeyDataFlowQuery
-import DataFlow::PathGraph
+import SymmetricKey::PathGraph

 /**
 * The creation of a literal byte array.
@@ -38,10 +38,10 @@ class StringLiteralSource extends KeySource {
 }

 from
-  SymmetricKeyTaintTrackingConfiguration keyFlow, DataFlow::PathNode source,
-  DataFlow::PathNode sink, KeySource srcNode, SymmetricEncryptionKeySink sinkNode
+  SymmetricKey::PathNode source, SymmetricKey::PathNode sink, KeySource srcNode,
+  SymmetricEncryptionKeySink sinkNode
 where
-  keyFlow.hasFlowPath(source, sink) and
+  SymmetricKey::flowPath(source, sink) and
  source.getNode() = srcNode and
  sink.getNode() = sinkNode
 select sink.getNode(), source, sink,
--- a/Features/CWE-321/HardcodedSymmetricEncryptionKey.ql
+++ b/Features/CWE-321/HardcodedSymmetricEncryptionKey.ql
@@ -15,10 +15,10 @@

 import csharp
 import semmle.code.csharp.security.cryptography.HardcodedSymmetricEncryptionKey::HardcodedSymmetricEncryptionKey
-import DataFlow::PathGraph
+import HardCodedSymmetricEncryption::PathGraph

-from TaintTrackingConfiguration c, DataFlow::PathNode source, DataFlow::PathNode sink
-where c.hasFlowPath(source, sink)
+from HardCodedSymmetricEncryption::PathNode source, HardCodedSymmetricEncryption::PathNode sink
+where HardCodedSymmetricEncryption::flowPath(source, sink)
 select sink.getNode(), source, sink,
  "Hard-coded symmetric $@ is used in symmetric algorithm in " +
    sink.getNode().(Sink).getDescription() + ".", source.getNode(), "key"
--- a/Features/CWE-359/ExposureOfPrivateInformation.ql
+++ b/Features/CWE-359/ExposureOfPrivateInformation.ql
@@ -13,10 +13,10 @@

 import csharp
 import semmle.code.csharp.security.dataflow.ExposureOfPrivateInformationQuery
-import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph
+import ExposureOfPrivateInformation::PathGraph

-from TaintTrackingConfiguration c, DataFlow::PathNode source, DataFlow::PathNode sink
-where c.hasFlowPath(source, sink)
+from ExposureOfPrivateInformation::PathNode source, ExposureOfPrivateInformation::PathNode sink
+where ExposureOfPrivateInformation::flowPath(source, sink)
 select sink.getNode(), source, sink,
  "Private data returned by $@ is written to an external location.", source.getNode(),
  source.getNode().toString()
--- a/Features/CWE-807/ConditionalBypass.ql
+++ b/Features/CWE-807/ConditionalBypass.ql
@@ -15,9 +15,9 @@

 import csharp
 import semmle.code.csharp.security.dataflow.ConditionalBypassQuery
-import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph
+import ConditionalBypass::PathGraph

-from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
-where config.hasFlowPath(source, sink)
+from ConditionalBypass::PathNode source, ConditionalBypass::PathNode sink
+where ConditionalBypass::flowPath(source, sink)
 select sink.getNode(), source, sink, "This condition guards a sensitive $@, but a $@ controls it.",
  sink.getNode().(Sink).getSensitiveMethodCall(), "action", source.getNode(), "user-provided value"