Merge branch 'main' of github.com:github/codeql into SharedDataflow_SequenceFlow

python/ql/src/experimental/dataflow/internal/TaintTrackingPrivate.qll

@@ -1,7 +1,14 @@
 private import python
-private import TaintTrackingPublic
 private import experimental.dataflow.DataFlow
 private import experimental.dataflow.internal.DataFlowPrivate
+private import experimental.dataflow.internal.TaintTrackingPublic
+
+/**
+ * Holds if taint can flow in one local step from `nodeFrom` to `nodeTo` excluding
+ * local data flow steps. That is, `nodeFrom` and `nodeTo` are likely to represent
+ * different objects.
+ */
+predicate localAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) { none() }
 
 /**
  * Holds if `node` should be a barrier in all global taint flow configurations
@@ -10,12 +17,11 @@ private import experimental.dataflow.internal.DataFlowPrivate
 predicate defaultTaintBarrier(DataFlow::Node node) { none() }
 
 /**
- * Holds if the additional step from `pred` to `succ` should be included in all
+ * Holds if the additional step from `nodeFrom` to `nodeTo` should be included in all
  * global taint flow configurations.
  */
-predicate defaultAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
-  none()
-  // localAdditionalTaintStep(pred, succ)
-  // or
-  // succ = pred.(DataFlow::NonLocalJumpNode).getAJumpSuccessor(false)
+predicate defaultAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+  localAdditionalTaintStep(nodeFrom, nodeTo)
+  or
+  any(AdditionalTaintStep a).step(nodeFrom, nodeTo)
 }

python/ql/src/experimental/dataflow/internal/TaintTrackingPublic.qll

@@ -6,27 +6,52 @@
 private import python
 private import TaintTrackingPrivate
 private import experimental.dataflow.DataFlow
-// /**
-//  * Holds if taint propagates from `source` to `sink` in zero or more local
-//  * (intra-procedural) steps.
-//  */
-// predicate localTaint(DataFlow::Node source, DataFlow::Node sink) { localTaintStep*(source, sink) }
-// // /**
-// //  * Holds if taint can flow from `e1` to `e2` in zero or more
-// //  * local (intra-procedural) steps.
-// //  */
-// // predicate localExprTaint(Expr e1, Expr e2) {
-// //   localTaint(DataFlow::exprNode(e1), DataFlow::exprNode(e2))
-// // }
-// // /** A member (property or field) that is tainted if its containing object is tainted. */
-// // abstract class TaintedMember extends AssignableMember { }
-// /**
-//  * Holds if taint propagates from `nodeFrom` to `nodeTo` in exactly one local
-//  * (intra-procedural) step.
-//  */
-// predicate localTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
-//   // Ordinary data flow
-//   DataFlow::localFlowStep(nodeFrom, nodeTo)
-//   or
-//   localAdditionalTaintStep(nodeFrom, nodeTo)
-// }
+
+// Local taint flow and helpers
+/**
+ * Holds if taint propagates from `source` to `sink` in zero or more local
+ * (intra-procedural) steps.
+ */
+predicate localTaint(DataFlow::Node source, DataFlow::Node sink) { localTaintStep*(source, sink) }
+
+/**
+ * Holds if taint can flow from `e1` to `e2` in zero or more local (intra-procedural)
+ * steps.
+ */
+predicate localExprTaint(Expr e1, Expr e2) {
+  localTaint(DataFlow::exprNode(e1), DataFlow::exprNode(e2))
+}
+
+/**
+ * Holds if taint propagates from `nodeFrom` to `nodeTo` in exactly one local
+ * (intra-procedural) step.
+ */
+predicate localTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+  // Ordinary data flow
+  DataFlow::localFlowStep(nodeFrom, nodeTo)
+  or
+  localAdditionalTaintStep(nodeFrom, nodeTo)
+}
+
+// AdditionalTaintStep for global taint flow
+private newtype TUnit = TMkUnit()
+
+/** A singleton class containing a single dummy "unit" value. */
+private class Unit extends TUnit {
+  /** Gets a textual representation of this element. */
+  string toString() { result = "unit" }
+}
+
+/**
+ * A unit class for adding additional taint steps.
+ *
+ * Extend this class to add additional taint steps that should apply to all
+ * taint configurations.
+ */
+class AdditionalTaintStep extends Unit {
+  /**
+   * Holds if the step from `nodeFrom` to `nodeTo` should be considered a taint
+   * step for all configurations.
+   */
+  abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
+}

python/ql/test/experimental/dataflow/coverage/argumentRouting1.ql

@@ -0,0 +1,30 @@
+import experimental.dataflow.DataFlow
+
+/**
+ * A configuration to check routing of arguments through magic methods.
+ */
+class ArgumentRoutingConfig extends DataFlow::Configuration {
+  ArgumentRoutingConfig() { this = "ArgumentRoutingConfig" }
+
+  override predicate isSource(DataFlow::Node node) {
+    exists(AssignmentDefinition def |
+      def.getVariable() = node.(DataFlow::EssaNode).getVar() and
+      def.getValue().(DataFlow::DataFlowCall).getCallable().getName().matches("With\\_%")
+    ) and
+    node.(DataFlow::EssaNode).getVar().getName().matches("with\\_%")
+  }
+
+  override predicate isSink(DataFlow::Node node) {
+    exists(CallNode call |
+      call.getFunction().(NameNode).getId() = "SINK1" and
+      node.(DataFlow::CfgNode).getNode() = call.getAnArg()
+    )
+  }
+}
+
+from DataFlow::Node source, DataFlow::Node sink
+where
+  source.getLocation().getFile().getBaseName() = "classes.py" and
+  sink.getLocation().getFile().getBaseName() = "classes.py" and
+  exists(ArgumentRoutingConfig cfg | cfg.hasFlow(source, sink))
+select source, sink

python/ql/test/experimental/dataflow/coverage/argumentRouting2.ql

@@ -0,0 +1,26 @@
+import experimental.dataflow.DataFlow
+
+/**
+ * A configuration to check routing of arguments through magic methods.
+ */
+class ArgumentRoutingConfig extends DataFlow::Configuration {
+  ArgumentRoutingConfig() { this = "ArgumentRoutingConfig" }
+
+  override predicate isSource(DataFlow::Node node) {
+    node.(DataFlow::CfgNode).getNode().(NameNode).getId() = "arg2"
+  }
+
+  override predicate isSink(DataFlow::Node node) {
+    exists(CallNode call |
+      call.getFunction().(NameNode).getId() = "SINK2" and
+      node.(DataFlow::CfgNode).getNode() = call.getAnArg()
+    )
+  }
+}
+
+from DataFlow::Node source, DataFlow::Node sink
+where
+  source.getLocation().getFile().getBaseName() = "classes.py" and
+  sink.getLocation().getFile().getBaseName() = "classes.py" and
+  exists(ArgumentRoutingConfig cfg | cfg.hasFlow(source, sink))
+select source, sink

python/ql/test/experimental/dataflow/coverage/argumentRouting3.ql

@@ -0,0 +1,26 @@
+import experimental.dataflow.DataFlow
+
+/**
+ * A configuration to check routing of arguments through magic methods.
+ */
+class ArgumentRoutingConfig extends DataFlow::Configuration {
+  ArgumentRoutingConfig() { this = "ArgumentRoutingConfig" }
+
+  override predicate isSource(DataFlow::Node node) {
+    node.(DataFlow::CfgNode).getNode().(NameNode).getId() = "arg3"
+  }
+
+  override predicate isSink(DataFlow::Node node) {
+    exists(CallNode call |
+      call.getFunction().(NameNode).getId() = "SINK3" and
+      node.(DataFlow::CfgNode).getNode() = call.getAnArg()
+    )
+  }
+}
+
+from DataFlow::Node source, DataFlow::Node sink
+where
+  source.getLocation().getFile().getBaseName() = "classes.py" and
+  sink.getLocation().getFile().getBaseName() = "classes.py" and
+  exists(ArgumentRoutingConfig cfg | cfg.hasFlow(source, sink))
+select source, sink

python/ql/test/experimental/dataflow/coverage/argumentRouting4.ql

@@ -0,0 +1,26 @@
+import experimental.dataflow.DataFlow
+
+/**
+ * A configuration to check routing of arguments through magic methods.
+ */
+class ArgumentRoutingConfig extends DataFlow::Configuration {
+  ArgumentRoutingConfig() { this = "ArgumentRoutingConfig" }
+
+  override predicate isSource(DataFlow::Node node) {
+    node.(DataFlow::CfgNode).getNode().(NameNode).getId() = "arg4"
+  }
+
+  override predicate isSink(DataFlow::Node node) {
+    exists(CallNode call |
+      call.getFunction().(NameNode).getId() = "SINK4" and
+      node.(DataFlow::CfgNode).getNode() = call.getAnArg()
+    )
+  }
+}
+
+from DataFlow::Node source, DataFlow::Node sink
+where
+  source.getLocation().getFile().getBaseName() = "classes.py" and
+  sink.getLocation().getFile().getBaseName() = "classes.py" and
+  exists(ArgumentRoutingConfig cfg | cfg.hasFlow(source, sink))
+select source, sink

python/ql/test/experimental/dataflow/coverage/classesCallGraph.expected

@@ -1,6 +1,6 @@
-| classes.py:19:12:19:31 | ControlFlowNode for Attribute() | classes.py:19:12:19:31 | ControlFlowNode for Attribute() |
-| classes.py:174:7:174:22 | ControlFlowNode for set() | classes.py:174:7:174:22 | ControlFlowNode for set() |
-| classes.py:178:7:178:28 | ControlFlowNode for frozenset() | classes.py:178:7:178:28 | ControlFlowNode for frozenset() |
-| classes.py:182:7:182:26 | ControlFlowNode for dict() | classes.py:182:7:182:26 | ControlFlowNode for dict() |
-| classes.py:303:28:303:51 | ControlFlowNode for dict() | classes.py:303:28:303:51 | ControlFlowNode for dict() |
-| classes.py:466:12:466:24 | ControlFlowNode for Attribute() | classes.py:466:12:466:24 | ControlFlowNode for Attribute() |
+| classes.py:32:12:32:31 | ControlFlowNode for Attribute() | classes.py:32:12:32:31 | ControlFlowNode for Attribute() |
+| classes.py:212:7:212:22 | ControlFlowNode for set() | classes.py:212:7:212:22 | ControlFlowNode for set() |
+| classes.py:216:7:216:28 | ControlFlowNode for frozenset() | classes.py:216:7:216:28 | ControlFlowNode for frozenset() |
+| classes.py:220:7:220:26 | ControlFlowNode for dict() | classes.py:220:7:220:26 | ControlFlowNode for dict() |
+| classes.py:369:27:369:50 | ControlFlowNode for dict() | classes.py:369:27:369:50 | ControlFlowNode for dict() |
+| classes.py:563:12:563:24 | ControlFlowNode for Attribute() | classes.py:563:12:563:24 | ControlFlowNode for Attribute() |

python/ql/test/experimental/dataflow/coverage/classesCallGraph.ql

@@ -1,4 +1,29 @@
-import experimental.dataflow.callGraphConfig
+import experimental.dataflow.DataFlow
+
+/**
+ * A configuration to find the call graph edges.
+ */
+class CallGraphConfig extends DataFlow::Configuration {
+  CallGraphConfig() { this = "CallGraphConfig" }
+
+  override predicate isSource(DataFlow::Node node) {
+    node instanceof DataFlow::ReturnNode
+    or
+    // These sources should allow for the non-standard call syntax
+    node instanceof DataFlow::ArgumentNode
+  }
+
+  override predicate isSink(DataFlow::Node node) {
+    node instanceof DataFlow::OutNode
+    or
+    node instanceof DataFlow::ParameterNode and
+    // exclude parameters to the SINK-functions
+    not exists(DataFlow::DataFlowCallable c |
+      node.(DataFlow::ParameterNode).isParameterOf(c, _) and
+      c.getName().matches("SINK_")
+    )
+  }
+}
 
 from DataFlow::Node source, DataFlow::Node sink
 where
@@ -8,3 +33,4 @@ where
 select source, sink
 // Ideally, we would just have 1-step paths either from argument to parameter
 // or from return to call. This gives a bit more, so should be rewritten.
+// We should also consider splitting this into two, one for each direction.

python/ql/test/experimental/dataflow/tainttracking/basic/GlobalTaintTracking.expected

@@ -0,0 +1,2 @@
+| test.py:3:11:3:16 | ControlFlowNode for SOURCE | test.py:4:6:4:12 | ControlFlowNode for tainted |
+| test.py:7:20:7:25 | ControlFlowNode for SOURCE | test.py:8:10:8:21 | ControlFlowNode for also_tainted |

python/ql/test/experimental/dataflow/tainttracking/basic/GlobalTaintTracking.ql

@@ -0,0 +1,22 @@
+import python
+import experimental.dataflow.TaintTracking
+import experimental.dataflow.DataFlow
+
+class TestTaintTrackingConfiguration extends TaintTracking::Configuration {
+  TestTaintTrackingConfiguration() { this = "TestTaintTrackingConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) {
+    source.(DataFlow::CfgNode).getNode().(NameNode).getId() = "SOURCE"
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(CallNode call |
+      call.getFunction().(NameNode).getId() = "SINK" and
+      sink.(DataFlow::CfgNode).getNode() = call.getAnArg()
+    )
+  }
+}
+
+from TestTaintTrackingConfiguration config, DataFlow::Node source, DataFlow::Node sink
+where config.hasFlow(source, sink)
+select source, sink

python/ql/test/experimental/dataflow/tainttracking/basic/LocalTaintStep.expected

@@ -0,0 +1,5 @@
+| test.py:0:0:0:0 | SSA variable $ | test.py:0:0:0:0 | Exit node for Module test |
+| test.py:7:5:7:16 | SSA variable also_tainted | test.py:8:5:8:22 | SSA variable also_tainted |
+| test.py:7:5:7:16 | SSA variable also_tainted | test.py:8:10:8:21 | ControlFlowNode for also_tainted |
+| test.py:7:20:7:25 | ControlFlowNode for SOURCE | test.py:7:5:7:16 | SSA variable also_tainted |
+| test.py:8:5:8:22 | SSA variable also_tainted | test.py:6:1:6:11 | Exit node for Function func |

python/ql/test/experimental/dataflow/tainttracking/basic/LocalTaintStep.ql

@@ -0,0 +1,7 @@
+import python
+import experimental.dataflow.TaintTracking
+import experimental.dataflow.DataFlow
+
+from DataFlow::Node nodeFrom, DataFlow::Node nodeTo
+where TaintTracking::localTaintStep(nodeFrom, nodeTo)
+select nodeFrom, nodeTo

python/ql/test/experimental/dataflow/tainttracking/basic/test.py

@@ -0,0 +1,8 @@
+# Module level taint is different from inside functions, since shared dataflow library
+# relies on `getEnclosingCallable`
+tainted = SOURCE
+SINK(tainted)
+
+def func():
+    also_tainted = SOURCE
+    SINK(also_tainted)