Rust: Define queries more consistently and include all sinks in stats

rust/ql/lib/codeql/rust/security/CleartextLoggingExtensions.qll

@@ -36,8 +36,10 @@ module CleartextLogging {
    */
   private class SensitiveDataAsSource extends Source instanceof SensitiveData { }
 
-  /** A sink for logging from model data. */
-  private class ModelsAsDataSinks extends Sink {
-    ModelsAsDataSinks() { exists(string s | sinkNode(this, s) and s.matches("log-injection%")) }
+  /**
+   * A sink for logging from model data.
+   */
+  private class ModelsAsDataSink extends Sink {
+    ModelsAsDataSink() { exists(string s | sinkNode(this, s) and s.matches("log-injection%")) }
   }
 }

rust/ql/lib/codeql/rust/security/CleartextTransmissionExtensions.qll

@@ -7,32 +7,52 @@ private import codeql.util.Unit
 private import rust
 private import codeql.rust.dataflow.DataFlow
 private import codeql.rust.dataflow.FlowSink
+private import codeql.rust.security.SensitiveData
+private import codeql.rust.Concepts
 
 /**
- * A data flow sink for cleartext transmission vulnerabilities. That is,
- * a `DataFlow::Node` of something that is transmitted over a network.
+ * Provides default sources, sinks and barriers for detecting cleartext transmission
+ * vulnerabilities, as well as extension points for adding your own.
  */
-abstract class CleartextTransmissionSink extends DataFlow::Node { }
+module CleartextTransmission {
+  /**
+   * A data flow source for cleartext transmission vulnerabilities.
+   */
+  abstract class Source extends DataFlow::Node { }
 
-/**
- * A barrier for cleartext transmission vulnerabilities.
- */
-abstract class CleartextTransmissionBarrier extends DataFlow::Node { }
+  /**
+   * A data flow sink for cleartext transmission vulnerabilities. That is,
+   * a `DataFlow::Node` of something that is transmitted over a network.
+   */
+  abstract class Sink extends QuerySink::Range {
+    override string getSinkType() { result = "CleartextTransmission" }
+  }
 
-/**
- * A unit class for adding additional flow steps.
- */
-class CleartextTransmissionAdditionalFlowStep extends Unit {
   /**
-   * Holds if the step from `node1` to `node2` should be considered a flow
-   * step for paths related to cleartext transmission vulnerabilities.
+   * A barrier for cleartext transmission vulnerabilities.
    */
-  abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
-}
+  abstract class Barrier extends DataFlow::Node { }
 
-/**
- * A sink defined through MaD.
- */
-private class MadCleartextTransmissionSink extends CleartextTransmissionSink {
-  MadCleartextTransmissionSink() { sinkNode(this, "transmission") }
+  /**
+   * A unit class for adding additional flow steps.
+   */
+  class AdditionalFlowStep extends Unit {
+    /**
+     * Holds if the step from `node1` to `node2` should be considered a flow
+     * step for paths related to cleartext transmission vulnerabilities.
+     */
+    abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
+  }
+
+  /**
+   * Sensitive data, considered as a flow source.
+   */
+  private class SensitiveDataAsSource extends Source instanceof SensitiveData { }
+
+  /**
+   * A sink defined through MaD.
+   */
+  private class ModelsAsDataSink extends Sink {
+    ModelsAsDataSink() { sinkNode(this, "transmission") }
+  }
 }

rust/ql/lib/codeql/rust/security/SqlInjectionExtensions.qll

@@ -51,8 +51,10 @@ module SqlInjection {
     SqlExecutionAsSink() { this = any(SqlExecution e).getSql() }
   }
 
-  /** A sink for sql-injection from model data. */
-  private class ModelsAsDataSinks extends Sink {
-    ModelsAsDataSinks() { sinkNode(this, "sql-injection") }
+  /**
+   * A sink for sql-injection from model data.
+   */
+  private class ModelsAsDataSink extends Sink {
+    ModelsAsDataSink() { sinkNode(this, "sql-injection") }
   }
 }

rust/ql/lib/codeql/rust/security/TaintedPathExtensions.qll

@@ -23,7 +23,9 @@ module TaintedPath {
   /**
    * A data flow sink for path injection vulnerabilities.
    */
-  abstract class Sink extends DataFlow::Node { }
+  abstract class Sink extends QuerySink::Range {
+    override string getSinkType() { result = "TaintedPath" }
+  }
 
   /**
    * A barrier for path injection vulnerabilities.

rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll

@@ -44,6 +44,8 @@ module NormalHashFunction {
    * hashing. That is, a broken or weak hashing algorithm.
    */
   abstract class Sink extends QuerySink::Range {
+    override string getSinkType() { result = "WeakSensitiveDataHashing" }
+
     /**
      * Gets the name of the weak hashing algorithm.
      */
@@ -76,8 +78,6 @@ module NormalHashFunction {
   class WeakHashingOperationInputAsSink extends Sink {
     Cryptography::HashingAlgorithm algorithm;
 
-    override string getSinkType() { result = "WeakSensitiveDataHashing" }
-
     WeakHashingOperationInputAsSink() {
       exists(Cryptography::CryptographicOperation operation |
         algorithm.isWeak() and

rust/ql/lib/codeql/rust/security/regex/RegexInjectionExtensions.qll

@@ -11,60 +11,81 @@ private import codeql.rust.dataflow.FlowSink
 private import codeql.rust.Concepts
 
 /**
- * A data flow sink for regular expression injection vulnerabilities.
+ * Provides default sources, sinks and barriers for detecting regular expression
+ * injection vulnerabilities, as well as extension points for adding your own.
  */
-abstract class RegexInjectionSink extends QuerySink::Range {
-  override string getSinkType() { result = "RegexInjection" }
-}
+module RegexInjection {
+  /**
+   * A data flow source for regular expression injection vulnerabilities.
+   */
+  abstract class Source extends DataFlow::Node { }
 
-/**
- * A barrier for regular expression injection vulnerabilities.
- */
-abstract class RegexInjectionBarrier extends DataFlow::Node { }
+  /**
+   * A data flow sink for regular expression injection vulnerabilities.
+   */
+  abstract class Sink extends QuerySink::Range {
+    override string getSinkType() { result = "RegexInjection" }
+  }
 
-/** A sink for `a` in `Regex::new(a)` when `a` is not a literal. */
-private class NewRegexInjectionSink extends RegexInjectionSink {
-  NewRegexInjectionSink() {
-    exists(CallExprCfgNode call, PathExpr path |
-      path = call.getFunction().getExpr() and
-      path.getResolvedCrateOrigin() = "repo:https://github.com/rust-lang/regex:regex" and
-      path.getResolvedPath() = "<crate::regex::string::Regex>::new" and
-      this.asExpr() = call.getArgument(0) and
-      not this.asExpr() instanceof LiteralExprCfgNode
-    )
+  /**
+   * A barrier for regular expression injection vulnerabilities.
+   */
+  abstract class Barrier extends DataFlow::Node { }
+
+  /**
+   * A unit class for adding additional flow steps.
+   */
+  class AdditionalFlowStep extends Unit {
+    /**
+     * Holds if the step from `node1` to `node2` should be considered a flow
+     * step for paths related to regular expression injection vulnerabilities.
+     */
+    abstract predicate step(DataFlow::Node node1, DataFlow::Node node2);
   }
-}
 
-private class MadRegexInjectionSink extends RegexInjectionSink {
-  MadRegexInjectionSink() { sinkNode(this, "regex-use") }
-}
+  /**
+   * An active threat-model source, considered as a flow source.
+   */
+  private class ActiveThreatModelSourceAsSource extends Source, ActiveThreatModelSource { }
 
-/**
- * A unit class for adding additional flow steps.
- */
-class RegexInjectionAdditionalFlowStep extends Unit {
   /**
-   * Holds if the step from `node1` to `node2` should be considered a flow
-   * step for paths related to regular expression injection vulnerabilities.
+   * A sink for `a` in `Regex::new(a)` when `a` is not a literal.
    */
-  abstract predicate step(DataFlow::Node node1, DataFlow::Node node2);
-}
+  private class NewSink extends Sink {
+    NewSink() {
+      exists(CallExprCfgNode call, PathExpr path |
+        path = call.getFunction().getExpr() and
+        path.getResolvedCrateOrigin() = "repo:https://github.com/rust-lang/regex:regex" and
+        path.getResolvedPath() = "<crate::regex::string::Regex>::new" and
+        this.asExpr() = call.getArgument(0) and
+        not this.asExpr() instanceof LiteralExprCfgNode
+      )
+    }
+  }
 
-/**
- * An escape barrier for regular expression injection vulnerabilities.
- */
-private class RegexInjectionDefaultBarrier extends RegexInjectionBarrier {
-  RegexInjectionDefaultBarrier() {
-    // A barrier is any call to a function named `escape`, in particular this
-    // makes calls to `regex::escape` a barrier.
-    this.asExpr()
-        .getExpr()
-        .(CallExpr)
-        .getFunction()
-        .(PathExpr)
-        .getPath()
-        .getSegment()
-        .getIdentifier()
-        .getText() = "escape"
+  /**
+   * A sink for regular expression injection from model data.
+   */
+  private class ModelsAsDataSink extends Sink {
+    ModelsAsDataSink() { sinkNode(this, "regex-use") }
+  }
+
+  /**
+   * An escape barrier for regular expression injection vulnerabilities.
+   */
+  private class DefaultBarrier extends Barrier {
+    DefaultBarrier() {
+      // A barrier is any call to a function named `escape`, in particular this
+      // makes calls to `regex::escape` a barrier.
+      this.asExpr()
+          .getExpr()
+          .(CallExpr)
+          .getFunction()
+          .(PathExpr)
+          .getPath()
+          .getSegment()
+          .getIdentifier()
+          .getText() = "escape"
+    }
   }
 }

rust/ql/src/queries/security/CWE-020/RegexInjection.ql

@@ -17,21 +17,22 @@
 private import rust
 private import codeql.rust.dataflow.DataFlow
 private import codeql.rust.dataflow.TaintTracking
-private import codeql.rust.Concepts
 private import codeql.rust.security.regex.RegexInjectionExtensions
 
 /**
  * A taint configuration for detecting regular expression injection vulnerabilities.
  */
 module RegexInjectionConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource }
+  import RegexInjection
 
-  predicate isSink(DataFlow::Node sink) { sink instanceof RegexInjectionSink }
+  predicate isSource(DataFlow::Node source) { source instanceof Source }
 
-  predicate isBarrier(DataFlow::Node barrier) { barrier instanceof RegexInjectionBarrier }
+  predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+  predicate isBarrier(DataFlow::Node barrier) { barrier instanceof Barrier }
 
   predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
-    any(RegexInjectionAdditionalFlowStep s).step(nodeFrom, nodeTo)
+    any(AdditionalFlowStep s).step(nodeFrom, nodeTo)
   }
 }
 

rust/ql/src/queries/security/CWE-022/TaintedPath.ql

@@ -16,11 +16,10 @@
 
 import rust
 import codeql.rust.dataflow.DataFlow
-import codeql.rust.dataflow.internal.DataFlowImpl as DataflowImpl
 import codeql.rust.dataflow.TaintTracking
+import codeql.rust.dataflow.internal.DataFlowImpl as DataflowImpl
+import codeql.rust.Concepts
 import codeql.rust.security.TaintedPathExtensions
-import TaintedPathFlow::PathGraph
-private import codeql.rust.Concepts
 
 newtype NormalizationState =
   /** A state signifying that the file path has not been normalized. */
@@ -84,6 +83,8 @@ module TaintedPathConfig implements DataFlow::StateConfigSig {
 
 module TaintedPathFlow = TaintTracking::GlobalWithState<TaintedPathConfig>;
 
+import TaintedPathFlow::PathGraph
+
 from TaintedPathFlow::PathNode source, TaintedPathFlow::PathNode sink
 where TaintedPathFlow::flowPath(source, sink)
 select sink.getNode(), source, sink, "This path depends on a $@.", source.getNode(),

rust/ql/src/queries/security/CWE-089/SqlInjection.ql

@@ -14,21 +14,24 @@ import rust
 import codeql.rust.dataflow.DataFlow
 import codeql.rust.dataflow.TaintTracking
 import codeql.rust.security.SqlInjectionExtensions
-import SqlInjectionFlow::PathGraph
 
 /**
  * A taint configuration for tainted data that reaches a SQL sink.
  */
 module SqlInjectionConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node node) { node instanceof SqlInjection::Source }
+  import SqlInjection
+
+  predicate isSource(DataFlow::Node node) { node instanceof Source }
 
-  predicate isSink(DataFlow::Node node) { node instanceof SqlInjection::Sink }
+  predicate isSink(DataFlow::Node node) { node instanceof Sink }
 
-  predicate isBarrier(DataFlow::Node barrier) { barrier instanceof SqlInjection::Barrier }
+  predicate isBarrier(DataFlow::Node barrier) { barrier instanceof Barrier }
 }
 
 module SqlInjectionFlow = TaintTracking::Global<SqlInjectionConfig>;
 
+import SqlInjectionFlow::PathGraph
+
 from SqlInjectionFlow::PathNode sourceNode, SqlInjectionFlow::PathNode sinkNode
 where SqlInjectionFlow::flowPath(sourceNode, sinkNode)
 select sinkNode.getNode(), sourceNode, sinkNode, "This query depends on a $@.",

rust/ql/src/queries/security/CWE-311/CleartextTransmission.ql

@@ -13,7 +13,6 @@
 
 import rust
 import codeql.rust.dataflow.DataFlow
-import codeql.rust.security.SensitiveData
 import codeql.rust.dataflow.TaintTracking
 import codeql.rust.security.CleartextTransmissionExtensions
 
@@ -22,14 +21,16 @@ import codeql.rust.security.CleartextTransmissionExtensions
  * transmitted over a network.
  */
 module CleartextTransmissionConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node node) { node instanceof SensitiveData }
+  import CleartextTransmission
 
-  predicate isSink(DataFlow::Node node) { node instanceof CleartextTransmissionSink }
+  predicate isSource(DataFlow::Node node) { node instanceof Source }
 
-  predicate isBarrier(DataFlow::Node barrier) { barrier instanceof CleartextTransmissionBarrier }
+  predicate isSink(DataFlow::Node node) { node instanceof Sink }
+
+  predicate isBarrier(DataFlow::Node barrier) { barrier instanceof Barrier }
 
   predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
-    any(CleartextTransmissionAdditionalFlowStep s).step(nodeFrom, nodeTo)
+    any(AdditionalFlowStep s).step(nodeFrom, nodeTo)
   }
 
   predicate isBarrierIn(DataFlow::Node node) {

rust/ql/src/queries/security/CWE-312/CleartextLogging.ql

@@ -14,9 +14,9 @@
  */
 
 import rust
-import codeql.rust.security.CleartextLoggingExtensions
 import codeql.rust.dataflow.DataFlow
 import codeql.rust.dataflow.TaintTracking
+import codeql.rust.security.CleartextLoggingExtensions
 
 /**
  * A taint-tracking configuration for cleartext logging vulnerabilities.

rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql

@@ -13,9 +13,9 @@
  */
 
 import rust
-import codeql.rust.security.WeakSensitiveDataHashingExtensions
 import codeql.rust.dataflow.DataFlow
 import codeql.rust.dataflow.TaintTracking
+import codeql.rust.security.WeakSensitiveDataHashingExtensions
 
 /**
  * Provides a taint-tracking configuration for detecting use of a broken or weak