Merge pull request #12909 from michaelnebel/csharp/dataflowrefactor7

C#: Re-factor the experimental PotentialTimeBomb to use new API.
2026-04-26 17:25:19 +02:00 · 2023-05-09 11:44:44 +02:00
parent 77ec34587f d01674f930
commit b1659bee66
2 changed files with 72 additions and 68 deletions
--- a/Features/backdoor/PotentialTimeBomb.ql
+++ b/Features/backdoor/PotentialTimeBomb.ql
@@ -11,32 +11,16 @@
 */

 import csharp
-import DataFlow
+import Flow::PathGraph

-query predicate nodes = PathGraph::nodes/3;
-
-query predicate edges(DataFlow::PathNode a, DataFlow::PathNode b) {
-  PathGraph::edges(a, b)
+query predicate edges(Flow::PathNode a, Flow::PathNode b) {
+  Flow::PathGraph::edges(a, b)
  or
-  exists(
-    FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallable conf1,
-    FlowsFromTimeSpanArithmeticToTimeComparisonCallable conf2
-  |
-    conf1 = a.getConfiguration() and
-    conf1.isSink(a.getNode()) and
-    conf2 = b.getConfiguration() and
-    b.isSource()
-  )
+  FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallableConfig::isSink(a.getNode()) and
+  FlowsFromTimeSpanArithmeticToTimeComparisonCallableConfig::isSource(b.getNode())
  or
-  exists(
-    FlowsFromTimeSpanArithmeticToTimeComparisonCallable conf1,
-    FlowsFromTimeComparisonCallableToSelectionStatementCondition conf2
-  |
-    conf1 = a.getConfiguration() and
-    conf1.isSink(a.getNode()) and
-    conf2 = b.getConfiguration() and
-    b.isSource()
-  )
+  FlowsFromTimeSpanArithmeticToTimeComparisonCallableConfig::isSink(a.getNode()) and
+  FlowsFromTimeComparisonCallableToSelectionStatementConditionConfig::isSource(b.getNode())
 }

 /**
@@ -78,22 +62,19 @@ class DateTimeStruct extends Struct {
 }

 /**
- * Dataflow configuration to find flow from a GetLastWriteTime source to a DateTime arithmetic operation
+ * Configuration to find flow from a GetLastWriteTime source to a DateTime arithmetic operation
 */
-private class FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallable extends TaintTracking::Configuration
+private module FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallableConfig implements
+  DataFlow::ConfigSig
 {
-  FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallable() {
-    this = "FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallable"
-  }
-
-  override predicate isSource(DataFlow::Node source) {
+  predicate isSource(DataFlow::Node source) {
    exists(Call call, GetLastWriteTimeMethod m |
      m.getACall() = call and
      source.asExpr() = call
    )
  }

-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
    exists(Call call, DateTimeStruct dateTime |
      call.getAChild*() = sink.asExpr() and
      call = dateTime.getATimeSpanArithmeticCallable().getACall()
@@ -102,21 +83,24 @@ private class FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallable extend
 }

 /**
- * Dataflow configuration to find flow from a DateTime arithmetic operation to a DateTime comparison operation
+ * Tainttracking module to find flow from a GetLastWriteTime source to a DateTime arithmetic operation
 */
-private class FlowsFromTimeSpanArithmeticToTimeComparisonCallable extends TaintTracking::Configuration
-{
-  FlowsFromTimeSpanArithmeticToTimeComparisonCallable() {
-    this = "FlowsFromTimeSpanArithmeticToTimeComparisonCallable"
-  }
+private module FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallable =
+  TaintTracking::Global<FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallableConfig>;

-  override predicate isSource(DataFlow::Node source) {
+/**
+ * Configuration to find flow from a DateTime arithmetic operation to a DateTime comparison operation
+ */
+private module FlowsFromTimeSpanArithmeticToTimeComparisonCallableConfig implements
+  DataFlow::ConfigSig
+{
+  predicate isSource(DataFlow::Node source) {
    exists(DateTimeStruct dateTime, Call call | source.asExpr() = call |
      call = dateTime.getATimeSpanArithmeticCallable().getACall()
    )
  }

-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
    exists(Call call, DateTimeStruct dateTime |
      call.getAnArgument().getAChild*() = sink.asExpr() and
      call = dateTime.getAComparisonCallable().getACall()
@@ -125,53 +109,69 @@ private class FlowsFromTimeSpanArithmeticToTimeComparisonCallable extends TaintT
 }

 /**
- * Dataflow configuration to find flow from a DateTime comparison operation to a Selection Statement (such as an If)
+ * Tainttracking module to find flow from a DateTime arithmetic operation to a DateTime comparison operation
 */
-private class FlowsFromTimeComparisonCallableToSelectionStatementCondition extends TaintTracking::Configuration
-{
-  FlowsFromTimeComparisonCallableToSelectionStatementCondition() {
-    this = "FlowsFromTimeComparisonCallableToSelectionStatementCondition"
-  }
+private module FlowsFromTimeSpanArithmeticToTimeComparisonCallable =
+  TaintTracking::Global<FlowsFromTimeSpanArithmeticToTimeComparisonCallableConfig>;

-  override predicate isSource(DataFlow::Node source) {
+/**
+ * Configuration to find flow from a DateTime comparison operation to a Selection Statement (such as an If)
+ */
+private module FlowsFromTimeComparisonCallableToSelectionStatementConditionConfig implements
+  DataFlow::ConfigSig
+{
+  predicate isSource(DataFlow::Node source) {
    exists(DateTimeStruct dateTime, Call call | source.asExpr() = call |
      call = dateTime.getAComparisonCallable().getACall()
    )
  }

-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
    exists(SelectionStmt sel | sel.getCondition().getAChild*() = sink.asExpr())
  }
 }

+/**
+ * Tainttracking module to find flow from a DateTime comparison operation to a Selection Statement (such as an If)
+ */
+private module FlowsFromTimeComparisonCallableToSelectionStatementCondition =
+  TaintTracking::Global<FlowsFromTimeComparisonCallableToSelectionStatementConditionConfig>;
+
+private module Flow =
+  DataFlow::MergePathGraph3<FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallable::PathNode,
+    FlowsFromTimeSpanArithmeticToTimeComparisonCallable::PathNode,
+    FlowsFromTimeComparisonCallableToSelectionStatementCondition::PathNode,
+    FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallable::PathGraph,
+    FlowsFromTimeSpanArithmeticToTimeComparisonCallable::PathGraph,
+    FlowsFromTimeComparisonCallableToSelectionStatementCondition::PathGraph>;
+
 /**
 * Holds if the last file modification date from the call to getLastWriteTimeMethodCall will be used in a DateTime arithmetic operation timeArithmeticCall,
 * which is then used for a DateTime comparison timeComparisonCall and the result flows to a Selection statement which is likely a TimeBomb trigger
 */
 predicate isPotentialTimeBomb(
-  DataFlow::PathNode pathSource, DataFlow::PathNode pathSink, Call getLastWriteTimeMethodCall,
+  Flow::PathNode pathSource, Flow::PathNode pathSink, Call getLastWriteTimeMethodCall,
  Call timeArithmeticCall, Call timeComparisonCall, SelectionStmt selStatement
 ) {
-  exists(
-    FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallable config1, Node sink,
-    DateTimeStruct dateTime, FlowsFromTimeSpanArithmeticToTimeComparisonCallable config2,
-    Node sink2, FlowsFromTimeComparisonCallableToSelectionStatementCondition config3, Node sink3
-  |
-    pathSource.getNode() = exprNode(getLastWriteTimeMethodCall) and
-    config1.hasFlow(exprNode(getLastWriteTimeMethodCall), sink) and
+  exists(DataFlow::Node sink, DateTimeStruct dateTime, DataFlow::Node sink2, DataFlow::Node sink3 |
+    pathSource.getNode() = DataFlow::exprNode(getLastWriteTimeMethodCall) and
+    FlowsFromGetLastWriteTimeConfigToTimeSpanArithmeticCallable::flow(DataFlow::exprNode(getLastWriteTimeMethodCall),
+      sink) and
    timeArithmeticCall = dateTime.getATimeSpanArithmeticCallable().getACall() and
    timeArithmeticCall.getAChild*() = sink.asExpr() and
-    config2.hasFlow(exprNode(timeArithmeticCall), sink2) and
+    FlowsFromTimeSpanArithmeticToTimeComparisonCallable::flow(DataFlow::exprNode(timeArithmeticCall),
+      sink2) and
    timeComparisonCall = dateTime.getAComparisonCallable().getACall() and
    timeComparisonCall.getAnArgument().getAChild*() = sink2.asExpr() and
-    config3.hasFlow(exprNode(timeComparisonCall), sink3) and
+    FlowsFromTimeComparisonCallableToSelectionStatementCondition::flow(DataFlow::exprNode(timeComparisonCall),
+      sink3) and
    selStatement.getCondition().getAChild*() = sink3.asExpr() and
    pathSink.getNode() = sink3
  )
 }

 from
-  DataFlow::PathNode source, DataFlow::PathNode sink, Call getLastWriteTimeMethodCall,
+  Flow::PathNode source, Flow::PathNode sink, Call getLastWriteTimeMethodCall,
  Call timeArithmeticCall, Call timeComparisonCall, SelectionStmt selStatement
 where
  isPotentialTimeBomb(source, sink, getLastWriteTimeMethodCall, timeArithmeticCall,
--- a/Features/backdoor/PotentialTimeBomb.expected
+++ b/Features/backdoor/PotentialTimeBomb.expected
@@ -1,13 +1,3 @@
-edges
-| test.cs:69:34:69:76 | call to method GetLastWriteTime : DateTime | test.cs:71:36:71:48 | access to local variable lastWriteTime |
-| test.cs:71:13:71:71 | call to method CompareTo : Int32 | test.cs:71:13:71:76 | ... >= ... |
-| test.cs:71:36:71:48 | access to local variable lastWriteTime | test.cs:71:36:71:70 | call to method AddHours |
-| test.cs:71:36:71:70 | call to method AddHours | test.cs:71:13:71:71 | call to method CompareTo |
-| test.cs:71:36:71:70 | call to method AddHours | test.cs:71:13:71:71 | call to method CompareTo : Int32 |
-#select
-| test.cs:71:9:74:9 | if (...) ... | test.cs:69:34:69:76 | call to method GetLastWriteTime : DateTime | test.cs:71:13:71:71 | call to method CompareTo | Possible TimeBomb logic triggered by an $@ that takes into account $@ from the $@ as part of the potential trigger. | test.cs:71:13:71:71 | call to method CompareTo | call to method CompareTo | test.cs:71:36:71:70 | call to method AddHours | offset | test.cs:69:34:69:76 | call to method GetLastWriteTime | last modification time of a file |
-| test.cs:71:9:74:9 | if (...) ... | test.cs:69:34:69:76 | call to method GetLastWriteTime : DateTime | test.cs:71:13:71:71 | call to method CompareTo : Int32 | Possible TimeBomb logic triggered by an $@ that takes into account $@ from the $@ as part of the potential trigger. | test.cs:71:13:71:71 | call to method CompareTo | call to method CompareTo | test.cs:71:36:71:70 | call to method AddHours | offset | test.cs:69:34:69:76 | call to method GetLastWriteTime | last modification time of a file |
-| test.cs:71:9:74:9 | if (...) ... | test.cs:69:34:69:76 | call to method GetLastWriteTime : DateTime | test.cs:71:13:71:76 | ... >= ... | Possible TimeBomb logic triggered by an $@ that takes into account $@ from the $@ as part of the potential trigger. | test.cs:71:13:71:71 | call to method CompareTo | call to method CompareTo | test.cs:71:36:71:70 | call to method AddHours | offset | test.cs:69:34:69:76 | call to method GetLastWriteTime | last modification time of a file |
 nodes
 | test.cs:69:34:69:76 | call to method GetLastWriteTime : DateTime | semmle.label | call to method GetLastWriteTime : DateTime |
 | test.cs:71:13:71:71 | call to method CompareTo | semmle.label | call to method CompareTo |
@@ -15,3 +5,17 @@ nodes
 | test.cs:71:13:71:76 | ... >= ... | semmle.label | ... >= ... |
 | test.cs:71:36:71:48 | access to local variable lastWriteTime | semmle.label | access to local variable lastWriteTime |
 | test.cs:71:36:71:70 | call to method AddHours | semmle.label | call to method AddHours |
+subpaths
+edges
+| test.cs:69:34:69:76 | call to method GetLastWriteTime : DateTime | test.cs:71:36:71:48 | access to local variable lastWriteTime |
+| test.cs:71:13:71:71 | call to method CompareTo : Int32 | test.cs:71:13:71:76 | ... >= ... |
+| test.cs:71:36:71:48 | access to local variable lastWriteTime | test.cs:71:13:71:71 | call to method CompareTo |
+| test.cs:71:36:71:48 | access to local variable lastWriteTime | test.cs:71:13:71:71 | call to method CompareTo : Int32 |
+| test.cs:71:36:71:48 | access to local variable lastWriteTime | test.cs:71:36:71:70 | call to method AddHours |
+| test.cs:71:36:71:70 | call to method AddHours | test.cs:71:13:71:71 | call to method CompareTo |
+| test.cs:71:36:71:70 | call to method AddHours | test.cs:71:13:71:71 | call to method CompareTo : Int32 |
+| test.cs:71:36:71:70 | call to method AddHours | test.cs:71:36:71:70 | call to method AddHours |
+#select
+| test.cs:71:9:74:9 | if (...) ... | test.cs:69:34:69:76 | call to method GetLastWriteTime : DateTime | test.cs:71:13:71:71 | call to method CompareTo | Possible TimeBomb logic triggered by an $@ that takes into account $@ from the $@ as part of the potential trigger. | test.cs:71:13:71:71 | call to method CompareTo | call to method CompareTo | test.cs:71:36:71:70 | call to method AddHours | offset | test.cs:69:34:69:76 | call to method GetLastWriteTime | last modification time of a file |
+| test.cs:71:9:74:9 | if (...) ... | test.cs:69:34:69:76 | call to method GetLastWriteTime : DateTime | test.cs:71:13:71:71 | call to method CompareTo : Int32 | Possible TimeBomb logic triggered by an $@ that takes into account $@ from the $@ as part of the potential trigger. | test.cs:71:13:71:71 | call to method CompareTo | call to method CompareTo | test.cs:71:36:71:70 | call to method AddHours | offset | test.cs:69:34:69:76 | call to method GetLastWriteTime | last modification time of a file |
+| test.cs:71:9:74:9 | if (...) ... | test.cs:69:34:69:76 | call to method GetLastWriteTime : DateTime | test.cs:71:13:71:76 | ... >= ... | Possible TimeBomb logic triggered by an $@ that takes into account $@ from the $@ as part of the potential trigger. | test.cs:71:13:71:71 | call to method CompareTo | call to method CompareTo | test.cs:71:36:71:70 | call to method AddHours | offset | test.cs:69:34:69:76 | call to method GetLastWriteTime | last modification time of a file |