Merge branch 'main' into better-syntax-for-false-positives-and-negatives-inline-expectation

python/ql/src/experimental/Security-new-dataflow/CWE-022/ChainedConfigs12.qll

@@ -0,0 +1,75 @@
+/**
+ * This defines a `PathGraph` where sinks from `TaintTracking::Configuration`s are identified with
+ * sources from `TaintTracking2::Configuration`s if they represent the same `ControlFlowNode`.
+ *
+ * Paths are then connected appropriately.
+ */
+
+import python
+import experimental.dataflow.DataFlow
+import experimental.dataflow.DataFlow2
+import experimental.dataflow.TaintTracking
+import experimental.dataflow.TaintTracking2
+
+/**
+ * A `DataFlow::Node` that appears as a sink in Config1 and a source in Config2.
+ */
+private predicate crossoverNode(DataFlow::Node n) {
+  any(TaintTracking::Configuration t1).isSink(n) and
+  any(TaintTracking2::Configuration t2).isSource(n)
+}
+
+/**
+ * A new type which represents the union of the two sets of nodes.
+ */
+private newtype TCustomPathNode =
+  Config1Node(DataFlow::PathNode node1) { not crossoverNode(node1.getNode()) } or
+  Config2Node(DataFlow2::PathNode node2) { not crossoverNode(node2.getNode()) } or
+  CrossoverNode(DataFlow::Node node) { crossoverNode(node) }
+
+/**
+ * A class representing the set of all the path nodes in either config.
+ */
+class CustomPathNode extends TCustomPathNode {
+  /** Gets the PathNode if it is in Config1. */
+  DataFlow::PathNode asNode1() {
+    this = Config1Node(result) or this = CrossoverNode(result.getNode())
+  }
+
+  /** Gets the PathNode if it is in Config2. */
+  DataFlow2::PathNode asNode2() {
+    this = Config2Node(result) or this = CrossoverNode(result.getNode())
+  }
+
+  predicate hasLocationInfo(
+    string filepath, int startline, int startcolumn, int endline, int endcolumn
+  ) {
+    asNode1().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+    or
+    asNode2().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+  }
+
+  string toString() {
+    result = asNode1().toString()
+    or
+    result = asNode2().toString()
+  }
+}
+
+/** Holds if `(a,b)` is an edge in the graph of data flow path explanations. */
+query predicate edges(CustomPathNode a, CustomPathNode b) {
+  // Edge is in Config1 graph
+  DataFlow::PathGraph::edges(a.asNode1(), b.asNode1())
+  or
+  // Edge is in Config2 graph
+  DataFlow2::PathGraph::edges(a.asNode2(), b.asNode2())
+}
+
+/** Holds if `n` is a node in the graph of data flow path explanations. */
+query predicate nodes(CustomPathNode n, string key, string val) {
+  // Node is in Config1 graph
+  DataFlow::PathGraph::nodes(n.asNode1(), key, val)
+  or
+  // Node is in Config2 graph
+  DataFlow2::PathGraph::nodes(n.asNode2(), key, val)
+}

python/ql/src/experimental/Security-new-dataflow/CWE-022/PathInjection.ql

@@ -0,0 +1,116 @@
+/**
+ * @name Uncontrolled data used in path expression
+ * @description Accessing paths influenced by users can allow an attacker to access unexpected resources.
+ * @kind path-problem
+ * @problem.severity error
+ * @sub-severity high
+ * @precision high
+ * @id py/path-injection
+ * @tags correctness
+ *       security
+ *       external/owasp/owasp-a1
+ *       external/cwe/cwe-022
+ *       external/cwe/cwe-023
+ *       external/cwe/cwe-036
+ *       external/cwe/cwe-073
+ *       external/cwe/cwe-099
+ *
+ * The query detects cases where a user-controlled path is used in an unsafe manner,
+ * meaning it is not both normalized and _afterwards_ checked.
+ *
+ * It does so by dividing the problematic situation into two cases:
+ *  1. The file path is never normalized.
+ *     This is easily detected by using normalization as a sanitizer.
+ *
+ *  2. The file path is normalized at least once, but never checked afterwards.
+ *     This is detected by finding the earliest normalization and then ensuring that
+ *     no checks happen later. Since we start from the earliest normalization,
+ *     we know that the absence of checks means that no normalization has a
+ *     check after it. (No checks after a second normalization would be ok if
+ *     there was a check between the first and the second.)
+ *
+ * Note that one could make the dual split on whether the file path is ever checked. This does
+ * not work as nicely, however, since checking is modelled as a `BarrierGuard` rather than
+ * as a `Sanitizer`. That means that only some dataflow paths out of a check will be removed,
+ * and so identifying the last check is not possible simply by finding a dataflow path from it
+ * to a sink.
+ */
+
+import python
+import experimental.dataflow.DataFlow
+import experimental.dataflow.DataFlow2
+import experimental.dataflow.TaintTracking
+import experimental.dataflow.TaintTracking2
+import experimental.semmle.python.Concepts
+import experimental.dataflow.RemoteFlowSources
+import ChainedConfigs12
+
+// ---------------------------------------------------------------------------
+// Case 1. The path is never normalized.
+// ---------------------------------------------------------------------------
+/** Configuration to find paths from sources to sinks that contain no normalization. */
+class PathNotNormalizedConfiguration extends TaintTracking::Configuration {
+  PathNotNormalizedConfiguration() { this = "PathNotNormalizedConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) {
+    sink = any(FileSystemAccess e).getAPathArgument()
+  }
+
+  override predicate isSanitizer(DataFlow::Node node) { node instanceof Path::PathNormalization }
+}
+
+predicate pathNotNormalized(CustomPathNode source, CustomPathNode sink) {
+  any(PathNotNormalizedConfiguration config).hasFlowPath(source.asNode1(), sink.asNode1())
+}
+
+// ---------------------------------------------------------------------------
+// Case 2. The path is normalized at least once, but never checked afterwards.
+// ---------------------------------------------------------------------------
+/** Configuration to find paths from sources to normalizations that contain no prior normalizations. */
+class FirstNormalizationConfiguration extends TaintTracking::Configuration {
+  FirstNormalizationConfiguration() { this = "FirstNormalizationConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof Path::PathNormalization }
+
+  override predicate isSanitizerOut(DataFlow::Node node) { node instanceof Path::PathNormalization }
+}
+
+/** Configuration to find paths from normalizations to sinks that do not go through a check. */
+class NormalizedPathNotCheckedConfiguration extends TaintTracking2::Configuration {
+  NormalizedPathNotCheckedConfiguration() { this = "NormalizedPathNotCheckedConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof Path::PathNormalization }
+
+  override predicate isSink(DataFlow::Node sink) {
+    sink = any(FileSystemAccess e).getAPathArgument()
+  }
+
+  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+    guard instanceof Path::SafeAccessCheck
+  }
+}
+
+predicate pathNotCheckedAfterNormalization(CustomPathNode source, CustomPathNode sink) {
+  exists(
+    FirstNormalizationConfiguration config, DataFlow::PathNode mid1, DataFlow2::PathNode mid2,
+    NormalizedPathNotCheckedConfiguration config2
+  |
+    config.hasFlowPath(source.asNode1(), mid1) and
+    config2.hasFlowPath(mid2, sink.asNode2()) and
+    mid1.getNode().asCfgNode() = mid2.getNode().asCfgNode()
+  )
+}
+
+// ---------------------------------------------------------------------------
+// Query: Either case 1 or case 2.
+// ---------------------------------------------------------------------------
+from CustomPathNode source, CustomPathNode sink
+where
+  pathNotNormalized(source, sink)
+  or
+  pathNotCheckedAfterNormalization(source, sink)
+select sink, source, sink, "This path depends on $@.", source, "a user-provided value"

python/ql/src/experimental/Security-new-dataflow/CWE-079/ReflectedXss.ql

@@ -0,0 +1,38 @@
+/**
+ * @name Reflected server-side cross-site scripting
+ * @description Writing user input directly to a web page
+ *              allows for a cross-site scripting vulnerability.
+ * @kind path-problem
+ * @problem.severity error
+ * @sub-severity high
+ * @precision high
+ * @id py/reflective-xss
+ * @tags security
+ *       external/cwe/cwe-079
+ *       external/cwe/cwe-116
+ */
+
+import python
+import experimental.dataflow.DataFlow
+import experimental.dataflow.TaintTracking
+import experimental.semmle.python.Concepts
+import experimental.dataflow.RemoteFlowSources
+import DataFlow::PathGraph
+
+class ReflectedXssConfiguration extends TaintTracking::Configuration {
+  ReflectedXssConfiguration() { this = "ReflectedXssConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(HTTP::Server::HttpResponse response |
+      response.getMimetype().toLowerCase() = "text/html" and
+      sink = response.getBody()
+    )
+  }
+}
+
+from ReflectedXssConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "Cross-site scripting vulnerability due to $@.",
+  source.getNode(), "a user-provided value"

python/ql/src/experimental/dataflow/DataFlow3.qll

@@ -0,0 +1,26 @@
+/**
+ * Provides a library for local (intra-procedural) and global (inter-procedural)
+ * data flow analysis: deciding whether data can flow from a _source_ to a
+ * _sink_.
+ *
+ * Unless configured otherwise, _flow_ means that the exact value of
+ * the source may reach the sink. We do not track flow across pointer
+ * dereferences or array indexing. To track these types of flow, where the
+ * exact value may not be preserved, import
+ * `experimental.dataflow.TaintTracking`.
+ *
+ * To use global (interprocedural) data flow, extend the class
+ * `DataFlow::Configuration` as documented on that class. To use local
+ * (intraprocedural) data flow, call `DataFlow::localFlow` or
+ * `DataFlow::localFlowStep` with arguments of type `DataFlow::Node`.
+ */
+
+private import python
+
+/**
+ * Provides classes for performing local (intra-procedural) and
+ * global (inter-procedural) data flow analyses.
+ */
+module DataFlow3 {
+  import experimental.dataflow.internal.DataFlowImpl3
+}

python/ql/src/experimental/dataflow/DataFlow4.qll

@@ -0,0 +1,26 @@
+/**
+ * Provides a library for local (intra-procedural) and global (inter-procedural)
+ * data flow analysis: deciding whether data can flow from a _source_ to a
+ * _sink_.
+ *
+ * Unless configured otherwise, _flow_ means that the exact value of
+ * the source may reach the sink. We do not track flow across pointer
+ * dereferences or array indexing. To track these types of flow, where the
+ * exact value may not be preserved, import
+ * `experimental.dataflow.TaintTracking`.
+ *
+ * To use global (interprocedural) data flow, extend the class
+ * `DataFlow::Configuration` as documented on that class. To use local
+ * (intraprocedural) data flow, call `DataFlow::localFlow` or
+ * `DataFlow::localFlowStep` with arguments of type `DataFlow::Node`.
+ */
+
+private import python
+
+/**
+ * Provides classes for performing local (intra-procedural) and
+ * global (inter-procedural) data flow analyses.
+ */
+module DataFlow4 {
+  import experimental.dataflow.internal.DataFlowImpl4
+}

python/ql/src/experimental/dataflow/TaintTracking2.qll

@@ -0,0 +1,19 @@
+/**
+ * Provides classes for performing local (intra-procedural) and
+ * global (inter-procedural) taint-tracking analyses.
+ *
+ * To use global (interprocedural) taint tracking, extend the class
+ * `TaintTracking::Configuration` as documented on that class. To use local
+ * (intraprocedural) taint tracking, call `TaintTracking::localTaint` or
+ * `TaintTracking::localTaintStep` with arguments of type `DataFlow::Node`.
+ */
+
+private import python
+
+/**
+ * Provides classes for performing local (intra-procedural) and
+ * global (inter-procedural) taint-tracking analyses.
+ */
+module TaintTracking2 {
+  import experimental.dataflow.internal.tainttracking2.TaintTrackingImpl
+}

python/ql/src/experimental/dataflow/TaintTracking3.qll

@@ -0,0 +1,19 @@
+/**
+ * Provides classes for performing local (intra-procedural) and
+ * global (inter-procedural) taint-tracking analyses.
+ *
+ * To use global (interprocedural) taint tracking, extend the class
+ * `TaintTracking::Configuration` as documented on that class. To use local
+ * (intraprocedural) taint tracking, call `TaintTracking::localTaint` or
+ * `TaintTracking::localTaintStep` with arguments of type `DataFlow::Node`.
+ */
+
+private import python
+
+/**
+ * Provides classes for performing local (intra-procedural) and
+ * global (inter-procedural) taint-tracking analyses.
+ */
+module TaintTracking3 {
+  import experimental.dataflow.internal.tainttracking3.TaintTrackingImpl
+}

python/ql/src/experimental/dataflow/TaintTracking4.qll

@@ -0,0 +1,19 @@
+/**
+ * Provides classes for performing local (intra-procedural) and
+ * global (inter-procedural) taint-tracking analyses.
+ *
+ * To use global (interprocedural) taint tracking, extend the class
+ * `TaintTracking::Configuration` as documented on that class. To use local
+ * (intraprocedural) taint tracking, call `TaintTracking::localTaint` or
+ * `TaintTracking::localTaintStep` with arguments of type `DataFlow::Node`.
+ */
+
+private import python
+
+/**
+ * Provides classes for performing local (intra-procedural) and
+ * global (inter-procedural) taint-tracking analyses.
+ */
+module TaintTracking4 {
+  import experimental.dataflow.internal.tainttracking4.TaintTrackingImpl
+}

python/ql/src/experimental/dataflow/internal/tainttracking2/TaintTrackingImpl.qll

@@ -0,0 +1,115 @@
+/**
+ * Provides an implementation of global (interprocedural) taint tracking.
+ * This file re-exports the local (intraprocedural) taint-tracking analysis
+ * from `TaintTrackingParameter::Public` and adds a global analysis, mainly
+ * exposed through the `Configuration` class. For some languages, this file
+ * exists in several identical copies, allowing queries to use multiple
+ * `Configuration` classes that depend on each other without introducing
+ * mutual recursion among those configurations.
+ */
+
+import TaintTrackingParameter::Public
+private import TaintTrackingParameter::Private
+
+/**
+ * A configuration of interprocedural taint tracking analysis. This defines
+ * sources, sinks, and any other configurable aspect of the analysis. Each
+ * use of the taint tracking library must define its own unique extension of
+ * this abstract class.
+ *
+ * A taint-tracking configuration is a special data flow configuration
+ * (`DataFlow::Configuration`) that allows for flow through nodes that do not
+ * necessarily preserve values but are still relevant from a taint tracking
+ * perspective. (For example, string concatenation, where one of the operands
+ * is tainted.)
+ *
+ * To create a configuration, extend this class with a subclass whose
+ * characteristic predicate is a unique singleton string. For example, write
+ *
+ * ```ql
+ * class MyAnalysisConfiguration extends TaintTracking::Configuration {
+ *   MyAnalysisConfiguration() { this = "MyAnalysisConfiguration" }
+ *   // Override `isSource` and `isSink`.
+ *   // Optionally override `isSanitizer`.
+ *   // Optionally override `isSanitizerIn`.
+ *   // Optionally override `isSanitizerOut`.
+ *   // Optionally override `isSanitizerGuard`.
+ *   // Optionally override `isAdditionalTaintStep`.
+ * }
+ * ```
+ *
+ * Then, to query whether there is flow between some `source` and `sink`,
+ * write
+ *
+ * ```ql
+ * exists(MyAnalysisConfiguration cfg | cfg.hasFlow(source, sink))
+ * ```
+ *
+ * Multiple configurations can coexist, but it is unsupported to depend on
+ * another `TaintTracking::Configuration` or a `DataFlow::Configuration` in the
+ * overridden predicates that define sources, sinks, or additional steps.
+ * Instead, the dependency should go to a `TaintTracking2::Configuration` or a
+ * `DataFlow2::Configuration`, `DataFlow3::Configuration`, etc.
+ */
+abstract class Configuration extends DataFlow::Configuration {
+  bindingset[this]
+  Configuration() { any() }
+
+  /**
+   * Holds if `source` is a relevant taint source.
+   *
+   * The smaller this predicate is, the faster `hasFlow()` will converge.
+   */
+  // overridden to provide taint-tracking specific qldoc
+  abstract override predicate isSource(DataFlow::Node source);
+
+  /**
+   * Holds if `sink` is a relevant taint sink.
+   *
+   * The smaller this predicate is, the faster `hasFlow()` will converge.
+   */
+  // overridden to provide taint-tracking specific qldoc
+  abstract override predicate isSink(DataFlow::Node sink);
+
+  /** Holds if the node `node` is a taint sanitizer. */
+  predicate isSanitizer(DataFlow::Node node) { none() }
+
+  final override predicate isBarrier(DataFlow::Node node) {
+    isSanitizer(node) or
+    defaultTaintSanitizer(node)
+  }
+
+  /** Holds if taint propagation into `node` is prohibited. */
+  predicate isSanitizerIn(DataFlow::Node node) { none() }
+
+  final override predicate isBarrierIn(DataFlow::Node node) { isSanitizerIn(node) }
+
+  /** Holds if taint propagation out of `node` is prohibited. */
+  predicate isSanitizerOut(DataFlow::Node node) { none() }
+
+  final override predicate isBarrierOut(DataFlow::Node node) { isSanitizerOut(node) }
+
+  /** Holds if taint propagation through nodes guarded by `guard` is prohibited. */
+  predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { none() }
+
+  final override predicate isBarrierGuard(DataFlow::BarrierGuard guard) { isSanitizerGuard(guard) }
+
+  /**
+   * Holds if the additional taint propagation step from `node1` to `node2`
+   * must be taken into account in the analysis.
+   */
+  predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { none() }
+
+  final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    isAdditionalTaintStep(node1, node2) or
+    defaultAdditionalTaintStep(node1, node2)
+  }
+
+  /**
+   * Holds if taint may flow from `source` to `sink` for this configuration.
+   */
+  // overridden to provide taint-tracking specific qldoc
+  override predicate hasFlow(DataFlow::Node source, DataFlow::Node sink) {
+    super.hasFlow(source, sink)
+  }
+}

python/ql/src/experimental/dataflow/internal/tainttracking2/TaintTrackingParameter.qll

@@ -0,0 +1,6 @@
+import experimental.dataflow.internal.TaintTrackingPublic as Public
+
+module Private {
+  import experimental.dataflow.DataFlow2::DataFlow2 as DataFlow
+  import experimental.dataflow.internal.TaintTrackingPrivate
+}

python/ql/src/experimental/dataflow/internal/tainttracking3/TaintTrackingImpl.qll

@@ -0,0 +1,115 @@
+/**
+ * Provides an implementation of global (interprocedural) taint tracking.
+ * This file re-exports the local (intraprocedural) taint-tracking analysis
+ * from `TaintTrackingParameter::Public` and adds a global analysis, mainly
+ * exposed through the `Configuration` class. For some languages, this file
+ * exists in several identical copies, allowing queries to use multiple
+ * `Configuration` classes that depend on each other without introducing
+ * mutual recursion among those configurations.
+ */
+
+import TaintTrackingParameter::Public
+private import TaintTrackingParameter::Private
+
+/**
+ * A configuration of interprocedural taint tracking analysis. This defines
+ * sources, sinks, and any other configurable aspect of the analysis. Each
+ * use of the taint tracking library must define its own unique extension of
+ * this abstract class.
+ *
+ * A taint-tracking configuration is a special data flow configuration
+ * (`DataFlow::Configuration`) that allows for flow through nodes that do not
+ * necessarily preserve values but are still relevant from a taint tracking
+ * perspective. (For example, string concatenation, where one of the operands
+ * is tainted.)
+ *
+ * To create a configuration, extend this class with a subclass whose
+ * characteristic predicate is a unique singleton string. For example, write
+ *
+ * ```ql
+ * class MyAnalysisConfiguration extends TaintTracking::Configuration {
+ *   MyAnalysisConfiguration() { this = "MyAnalysisConfiguration" }
+ *   // Override `isSource` and `isSink`.
+ *   // Optionally override `isSanitizer`.
+ *   // Optionally override `isSanitizerIn`.
+ *   // Optionally override `isSanitizerOut`.
+ *   // Optionally override `isSanitizerGuard`.
+ *   // Optionally override `isAdditionalTaintStep`.
+ * }
+ * ```
+ *
+ * Then, to query whether there is flow between some `source` and `sink`,
+ * write
+ *
+ * ```ql
+ * exists(MyAnalysisConfiguration cfg | cfg.hasFlow(source, sink))
+ * ```
+ *
+ * Multiple configurations can coexist, but it is unsupported to depend on
+ * another `TaintTracking::Configuration` or a `DataFlow::Configuration` in the
+ * overridden predicates that define sources, sinks, or additional steps.
+ * Instead, the dependency should go to a `TaintTracking2::Configuration` or a
+ * `DataFlow2::Configuration`, `DataFlow3::Configuration`, etc.
+ */
+abstract class Configuration extends DataFlow::Configuration {
+  bindingset[this]
+  Configuration() { any() }
+
+  /**
+   * Holds if `source` is a relevant taint source.
+   *
+   * The smaller this predicate is, the faster `hasFlow()` will converge.
+   */
+  // overridden to provide taint-tracking specific qldoc
+  abstract override predicate isSource(DataFlow::Node source);
+
+  /**
+   * Holds if `sink` is a relevant taint sink.
+   *
+   * The smaller this predicate is, the faster `hasFlow()` will converge.
+   */
+  // overridden to provide taint-tracking specific qldoc
+  abstract override predicate isSink(DataFlow::Node sink);
+
+  /** Holds if the node `node` is a taint sanitizer. */
+  predicate isSanitizer(DataFlow::Node node) { none() }
+
+  final override predicate isBarrier(DataFlow::Node node) {
+    isSanitizer(node) or
+    defaultTaintSanitizer(node)
+  }
+
+  /** Holds if taint propagation into `node` is prohibited. */
+  predicate isSanitizerIn(DataFlow::Node node) { none() }
+
+  final override predicate isBarrierIn(DataFlow::Node node) { isSanitizerIn(node) }
+
+  /** Holds if taint propagation out of `node` is prohibited. */
+  predicate isSanitizerOut(DataFlow::Node node) { none() }
+
+  final override predicate isBarrierOut(DataFlow::Node node) { isSanitizerOut(node) }
+
+  /** Holds if taint propagation through nodes guarded by `guard` is prohibited. */
+  predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { none() }
+
+  final override predicate isBarrierGuard(DataFlow::BarrierGuard guard) { isSanitizerGuard(guard) }
+
+  /**
+   * Holds if the additional taint propagation step from `node1` to `node2`
+   * must be taken into account in the analysis.
+   */
+  predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { none() }
+
+  final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    isAdditionalTaintStep(node1, node2) or
+    defaultAdditionalTaintStep(node1, node2)
+  }
+
+  /**
+   * Holds if taint may flow from `source` to `sink` for this configuration.
+   */
+  // overridden to provide taint-tracking specific qldoc
+  override predicate hasFlow(DataFlow::Node source, DataFlow::Node sink) {
+    super.hasFlow(source, sink)
+  }
+}

python/ql/src/experimental/dataflow/internal/tainttracking3/TaintTrackingParameter.qll

@@ -0,0 +1,6 @@
+import experimental.dataflow.internal.TaintTrackingPublic as Public
+
+module Private {
+  import experimental.dataflow.DataFlow3::DataFlow3 as DataFlow
+  import experimental.dataflow.internal.TaintTrackingPrivate
+}

python/ql/src/experimental/dataflow/internal/tainttracking4/TaintTrackingImpl.qll

@@ -0,0 +1,115 @@
+/**
+ * Provides an implementation of global (interprocedural) taint tracking.
+ * This file re-exports the local (intraprocedural) taint-tracking analysis
+ * from `TaintTrackingParameter::Public` and adds a global analysis, mainly
+ * exposed through the `Configuration` class. For some languages, this file
+ * exists in several identical copies, allowing queries to use multiple
+ * `Configuration` classes that depend on each other without introducing
+ * mutual recursion among those configurations.
+ */
+
+import TaintTrackingParameter::Public
+private import TaintTrackingParameter::Private
+
+/**
+ * A configuration of interprocedural taint tracking analysis. This defines
+ * sources, sinks, and any other configurable aspect of the analysis. Each
+ * use of the taint tracking library must define its own unique extension of
+ * this abstract class.
+ *
+ * A taint-tracking configuration is a special data flow configuration
+ * (`DataFlow::Configuration`) that allows for flow through nodes that do not
+ * necessarily preserve values but are still relevant from a taint tracking
+ * perspective. (For example, string concatenation, where one of the operands
+ * is tainted.)
+ *
+ * To create a configuration, extend this class with a subclass whose
+ * characteristic predicate is a unique singleton string. For example, write
+ *
+ * ```ql
+ * class MyAnalysisConfiguration extends TaintTracking::Configuration {
+ *   MyAnalysisConfiguration() { this = "MyAnalysisConfiguration" }
+ *   // Override `isSource` and `isSink`.
+ *   // Optionally override `isSanitizer`.
+ *   // Optionally override `isSanitizerIn`.
+ *   // Optionally override `isSanitizerOut`.
+ *   // Optionally override `isSanitizerGuard`.
+ *   // Optionally override `isAdditionalTaintStep`.
+ * }
+ * ```
+ *
+ * Then, to query whether there is flow between some `source` and `sink`,
+ * write
+ *
+ * ```ql
+ * exists(MyAnalysisConfiguration cfg | cfg.hasFlow(source, sink))
+ * ```
+ *
+ * Multiple configurations can coexist, but it is unsupported to depend on
+ * another `TaintTracking::Configuration` or a `DataFlow::Configuration` in the
+ * overridden predicates that define sources, sinks, or additional steps.
+ * Instead, the dependency should go to a `TaintTracking2::Configuration` or a
+ * `DataFlow2::Configuration`, `DataFlow3::Configuration`, etc.
+ */
+abstract class Configuration extends DataFlow::Configuration {
+  bindingset[this]
+  Configuration() { any() }
+
+  /**
+   * Holds if `source` is a relevant taint source.
+   *
+   * The smaller this predicate is, the faster `hasFlow()` will converge.
+   */
+  // overridden to provide taint-tracking specific qldoc
+  abstract override predicate isSource(DataFlow::Node source);
+
+  /**
+   * Holds if `sink` is a relevant taint sink.
+   *
+   * The smaller this predicate is, the faster `hasFlow()` will converge.
+   */
+  // overridden to provide taint-tracking specific qldoc
+  abstract override predicate isSink(DataFlow::Node sink);
+
+  /** Holds if the node `node` is a taint sanitizer. */
+  predicate isSanitizer(DataFlow::Node node) { none() }
+
+  final override predicate isBarrier(DataFlow::Node node) {
+    isSanitizer(node) or
+    defaultTaintSanitizer(node)
+  }
+
+  /** Holds if taint propagation into `node` is prohibited. */
+  predicate isSanitizerIn(DataFlow::Node node) { none() }
+
+  final override predicate isBarrierIn(DataFlow::Node node) { isSanitizerIn(node) }
+
+  /** Holds if taint propagation out of `node` is prohibited. */
+  predicate isSanitizerOut(DataFlow::Node node) { none() }
+
+  final override predicate isBarrierOut(DataFlow::Node node) { isSanitizerOut(node) }
+
+  /** Holds if taint propagation through nodes guarded by `guard` is prohibited. */
+  predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { none() }
+
+  final override predicate isBarrierGuard(DataFlow::BarrierGuard guard) { isSanitizerGuard(guard) }
+
+  /**
+   * Holds if the additional taint propagation step from `node1` to `node2`
+   * must be taken into account in the analysis.
+   */
+  predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { none() }
+
+  final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    isAdditionalTaintStep(node1, node2) or
+    defaultAdditionalTaintStep(node1, node2)
+  }
+
+  /**
+   * Holds if taint may flow from `source` to `sink` for this configuration.
+   */
+  // overridden to provide taint-tracking specific qldoc
+  override predicate hasFlow(DataFlow::Node source, DataFlow::Node sink) {
+    super.hasFlow(source, sink)
+  }
+}

python/ql/src/experimental/dataflow/internal/tainttracking4/TaintTrackingParameter.qll

@@ -0,0 +1,6 @@
+import experimental.dataflow.internal.TaintTrackingPublic as Public
+
+module Private {
+  import experimental.dataflow.DataFlow4::DataFlow4 as DataFlow
+  import experimental.dataflow.internal.TaintTrackingPrivate
+}

python/ql/src/experimental/semmle/python/Concepts.qll

@@ -40,6 +40,74 @@ module SystemCommandExecution {
   }
 }
 
+/**
+ * A data flow node that performs a file system access, including reading and writing data,
+ * creating and deleting files and folders, checking and updating permissions, and so on.
+ *
+ * Extend this class to refine existing API models. If you want to model new APIs,
+ * extend `FileSystemAccess::Range` instead.
+ */
+class FileSystemAccess extends DataFlow::Node {
+  FileSystemAccess::Range range;
+
+  FileSystemAccess() { this = range }
+
+  /** Gets an argument to this file system access that is interpreted as a path. */
+  DataFlow::Node getAPathArgument() { result = range.getAPathArgument() }
+}
+
+/** Provides a class for modeling new file system access APIs. */
+module FileSystemAccess {
+  /**
+   * A data-flow node that performs a file system access, including reading and writing data,
+   * creating and deleting files and folders, checking and updating permissions, and so on.
+   *
+   * Extend this class to model new APIs. If you want to refine existing API models,
+   * extend `FileSystemAccess` instead.
+   */
+  abstract class Range extends DataFlow::Node {
+    /** Gets an argument to this file system access that is interpreted as a path. */
+    abstract DataFlow::Node getAPathArgument();
+  }
+}
+
+/** Provides classes for modeling path-related APIs. */
+module Path {
+  /**
+   * A data-flow node that performs path normalization. This is often needed in order
+   * to safely access paths.
+   */
+  class PathNormalization extends DataFlow::Node {
+    PathNormalization::Range range;
+
+    PathNormalization() { this = range }
+  }
+
+  /** Provides a class for modeling new path normalization APIs. */
+  module PathNormalization {
+    /**
+     * A data-flow node that performs path normalization. This is often needed in order
+     * to safely access paths.
+     */
+    abstract class Range extends DataFlow::Node { }
+  }
+
+  /** A data-flow node that checks that a path is safe to access. */
+  class SafeAccessCheck extends DataFlow::BarrierGuard {
+    SafeAccessCheck::Range range;
+
+    SafeAccessCheck() { this = range }
+
+    override predicate checks(ControlFlowNode node, boolean branch) { range.checks(node, branch) }
+  }
+
+  /** Provides a class for modeling new path safety checks. */
+  module SafeAccessCheck {
+    /** A data-flow node that checks that a path is safe to access. */
+    abstract class Range extends DataFlow::BarrierGuard { }
+  }
+}
+
 /**
  * A data-flow node that decodes data from a binary or textual format. This
  * is intended to include deserialization, unmarshalling, decoding, unpickling,
@@ -160,7 +228,7 @@ module HTTP {
   /** Provides classes for modeling HTTP servers. */
   module Server {
     /**
-     * An data-flow node that sets up a route on a server.
+     * A data-flow node that sets up a route on a server.
      *
      * Extend this class to refine existing API models. If you want to model new APIs,
      * extend `RouteSetup::Range` instead.
@@ -186,7 +254,7 @@ module HTTP {
     /** Provides a class for modeling new HTTP routing APIs. */
     module RouteSetup {
       /**
-       * An data-flow node that sets up a route on a server.
+       * A data-flow node that sets up a route on a server.
        *
        * Extend this class to model new APIs. If you want to refine existing API models,
        * extend `RouteSetup` instead.
@@ -219,5 +287,60 @@ module HTTP {
 
       override string getSourceType() { result = "RoutedParameter" }
     }
+
+    /**
+     * A data-flow node that creates a HTTP response on a server.
+     *
+     * Note: we don't require that this response must be sent to a client (a kind of
+     * "if a tree falls in a forest and nobody hears it" situation).
+     *
+     * Extend this class to refine existing API models. If you want to model new APIs,
+     * extend `HttpResponse::Range` instead.
+     */
+    class HttpResponse extends DataFlow::Node {
+      HttpResponse::Range range;
+
+      HttpResponse() { this = range }
+
+      /** Gets the data-flow node that specifies the body of this HTTP response. */
+      DataFlow::Node getBody() { result = range.getBody() }
+
+      /** Gets the mimetype of this HTTP response, if it can be statically determined. */
+      string getMimetype() { result = range.getMimetype() }
+    }
+
+    /** Provides a class for modeling new HTTP response APIs. */
+    module HttpResponse {
+      /**
+       * A data-flow node that creates a HTTP response on a server.
+       *
+       * Note: we don't require that this response must be sent to a client (a kind of
+       * "if a tree falls in a forest and nobody hears it" situation).
+       *
+       * Extend this class to model new APIs. If you want to refine existing API models,
+       * extend `HttpResponse` instead.
+       */
+      abstract class Range extends DataFlow::Node {
+        /** Gets the data-flow node that specifies the body of this HTTP response. */
+        abstract DataFlow::Node getBody();
+
+        /** Gets the data-flow node that specifies the content-type/mimetype of this HTTP response, if any. */
+        abstract DataFlow::Node getMimetypeOrContentTypeArg();
+
+        /** Gets the default mimetype that should be used if `getMimetypeOrContentTypeArg` has no results. */
+        abstract string getMimetypeDefault();
+
+        /** Gets the mimetype of this HTTP response, if it can be statically determined. */
+        string getMimetype() {
+          exists(StrConst str |
+            DataFlow::localFlow(DataFlow::exprNode(str), this.getMimetypeOrContentTypeArg()) and
+            result = str.getText().splitAt(";", 0)
+          )
+          or
+          not exists(this.getMimetypeOrContentTypeArg()) and
+          result = this.getMimetypeDefault()
+        }
+      }
+    }
   }
 }

python/ql/src/experimental/semmle/python/frameworks/Flask.qll

@@ -15,6 +15,9 @@ private import experimental.semmle.python.frameworks.Werkzeug
  * See https://flask.palletsprojects.com/en/1.1.x/.
  */
 private module FlaskModel {
+  // ---------------------------------------------------------------------------
+  // flask
+  // ---------------------------------------------------------------------------
   /** Gets a reference to the `flask` module. */
   private DataFlow::Node flask(DataFlow::TypeTracker t) {
     t.start() and
@@ -26,21 +29,52 @@ private module FlaskModel {
   /** Gets a reference to the `flask` module. */
   DataFlow::Node flask() { result = flask(DataFlow::TypeTracker::end()) }
 
+  /**
+   * Gets a reference to the attribute `attr_name` of the `flask` module.
+   * WARNING: Only holds for a few predefined attributes.
+   */
+  private DataFlow::Node flask_attr(DataFlow::TypeTracker t, string attr_name) {
+    attr_name in ["request", "make_response", "Response"] and
+    (
+      t.start() and
+      result = DataFlow::importNode("flask" + "." + attr_name)
+      or
+      t.startInAttr(attr_name) and
+      result = flask()
+    )
+    or
+    // Due to bad performance when using normal setup with `flask_attr(t2, attr_name).track(t2, t)`
+    // we have inlined that code and forced a join
+    exists(DataFlow::TypeTracker t2 |
+      exists(DataFlow::StepSummary summary |
+        flask_attr_first_join(t2, attr_name, result, summary) and
+        t = t2.append(summary)
+      )
+    )
+  }
+
+  pragma[nomagic]
+  private predicate flask_attr_first_join(
+    DataFlow::TypeTracker t2, string attr_name, DataFlow::Node res, DataFlow::StepSummary summary
+  ) {
+    DataFlow::StepSummary::step(flask_attr(t2, attr_name), res, summary)
+  }
+
+  /**
+   * Gets a reference to the attribute `attr_name` of the `flask` module.
+   * WARNING: Only holds for a few predefined attributes.
+   */
+  private DataFlow::Node flask_attr(string attr_name) {
+    result = flask_attr(DataFlow::TypeTracker::end(), attr_name)
+  }
+
   /** Provides models for the `flask` module. */
   module flask {
     /** Gets a reference to the `flask.request` object. */
-    private DataFlow::Node request(DataFlow::TypeTracker t) {
-      t.start() and
-      result = DataFlow::importNode("flask.request")
-      or
-      t.startInAttr("request") and
-      result = flask()
-      or
-      exists(DataFlow::TypeTracker t2 | result = request(t2).track(t2, t))
-    }
+    DataFlow::Node request() { result = flask_attr("request") }
 
-    /** Gets a reference to the `flask.request` object. */
-    DataFlow::Node request() { result = request(DataFlow::TypeTracker::end()) }
+    /** Gets a reference to the `flask.make_response` function. */
+    DataFlow::Node make_response() { result = flask_attr("make_response") }
 
     /**
      * Provides models for the `flask.Flask` class
@@ -96,7 +130,7 @@ private module FlaskModel {
        * WARNING: Only holds for a few predefined attributes.
        */
       private DataFlow::Node instance_attr(DataFlow::TypeTracker t, string attr_name) {
-        attr_name in ["route", "add_url_rule"] and
+        attr_name in ["route", "add_url_rule", "make_response"] and
         t.startInAttr(attr_name) and
         result = flask::Flask::instance()
         or
@@ -131,9 +165,99 @@ private module FlaskModel {
 
       /** Gets a reference to the `add_url_rule` method on an instance of `flask.Flask`. */
       DataFlow::Node add_url_rule() { result = instance_attr("add_url_rule") }
+
+      /** Gets a reference to the `make_response` method on an instance of `flask.Flask`. */
+      // HACK: We can't call this predicate `make_response` since shadowing is
+      // completely disallowed in QL. I added an underscore to move things forward for
+      // now :(
+      DataFlow::Node make_response_() { result = instance_attr("make_response") }
+
+      /** Gets a reference to the `response_class` attribute on the `flask.Flask` class or an instance. */
+      private DataFlow::Node response_class(DataFlow::TypeTracker t) {
+        t.startInAttr("response_class") and
+        result in [classRef(), instance()]
+        or
+        exists(DataFlow::TypeTracker t2 | result = response_class(t2).track(t2, t))
+      }
+
+      /**
+       * Gets a reference to the `response_class` attribute on the `flask.Flask` class or an instance.
+       *
+       * See https://flask.palletsprojects.com/en/1.1.x/api/#flask.Flask.response_class
+       */
+      DataFlow::Node response_class() { result = response_class(DataFlow::TypeTracker::end()) }
     }
   }
 
+  /**
+   * Provides models for the `flask.Response` class
+   *
+   * See https://flask.palletsprojects.com/en/1.1.x/api/#flask.Response.
+   */
+  module Response {
+    /** Gets a reference to the `flask.Response` class. */
+    private DataFlow::Node classRef(DataFlow::TypeTracker t) {
+      t.start() and
+      result in [flask_attr("Response"), flask::Flask::response_class()]
+      or
+      exists(DataFlow::TypeTracker t2 | result = classRef(t2).track(t2, t))
+    }
+
+    /** Gets a reference to the `flask.Response` class. */
+    DataFlow::Node classRef() { result = classRef(DataFlow::TypeTracker::end()) }
+
+    /**
+     * A source of an instance of `flask.Response`.
+     *
+     * This can include instantiation of the class, return value from function
+     * calls, or a special parameter that will be set when functions are call by external
+     * library.
+     *
+     * Use `Response::instance()` predicate to get references to instances of `flask.Response`.
+     */
+    abstract class InstanceSource extends HTTP::Server::HttpResponse::Range, DataFlow::Node { }
+
+    /** A direct instantiation of `flask.Response`. */
+    private class ClassInstantiation extends InstanceSource, DataFlow::CfgNode {
+      override CallNode node;
+
+      ClassInstantiation() { node.getFunction() = classRef().asCfgNode() }
+
+      override DataFlow::Node getBody() { result.asCfgNode() = node.getArg(0) }
+
+      override string getMimetypeDefault() { result = "text/html" }
+
+      /** Gets the argument passed to the `mimetype` parameter, if any. */
+      private DataFlow::Node getMimetypeArg() {
+        result.asCfgNode() in [node.getArg(3), node.getArgByName("mimetype")]
+      }
+
+      /** Gets the argument passed to the `content_type` parameter, if any. */
+      private DataFlow::Node getContentTypeArg() {
+        result.asCfgNode() in [node.getArg(4), node.getArgByName("content_type")]
+      }
+
+      override DataFlow::Node getMimetypeOrContentTypeArg() {
+        result = this.getContentTypeArg()
+        or
+        // content_type argument takes priority over mimetype argument
+        not exists(this.getContentTypeArg()) and
+        result = this.getMimetypeArg()
+      }
+    }
+
+    /** Gets a reference to an instance of `flask.Response`. */
+    private DataFlow::Node instance(DataFlow::TypeTracker t) {
+      t.start() and
+      result instanceof InstanceSource
+      or
+      exists(DataFlow::TypeTracker t2 | result = instance(t2).track(t2, t))
+    }
+
+    /** Gets a reference to an instance of `flask.Response`. */
+    DataFlow::Node instance() { result = instance(DataFlow::TypeTracker::end()) }
+  }
+
   // ---------------------------------------------------------------------------
   // routing modeling
   // ---------------------------------------------------------------------------
@@ -324,8 +448,50 @@ private module FlaskModel {
   private class RequestInputFiles extends RequestInputMultiDict {
     RequestInputFiles() { attr_name = "files" }
   }
+
   // TODO: Somehow specify that elements of `RequestInputFiles` are
   // Werkzeug::werkzeug::datastructures::FileStorage and should have those additional taint steps
   // AND that the 0-indexed argument to its' save method is a sink for path-injection.
   // https://werkzeug.palletsprojects.com/en/1.0.x/datastructures/#werkzeug.datastructures.FileStorage.save
+  // ---------------------------------------------------------------------------
+  // Response modeling
+  // ---------------------------------------------------------------------------
+  /**
+   * A call to either `flask.make_response` function, or the `make_response` method on
+   * an instance of `flask.Flask`.
+   *
+   * See
+   * - https://flask.palletsprojects.com/en/1.1.x/api/#flask.Flask.make_response
+   * - https://flask.palletsprojects.com/en/1.1.x/api/#flask.make_response
+   */
+  private class FlaskMakeResponseCall extends HTTP::Server::HttpResponse::Range, DataFlow::CfgNode {
+    override CallNode node;
+
+    FlaskMakeResponseCall() {
+      node.getFunction() = flask::make_response().asCfgNode()
+      or
+      node.getFunction() = flask::Flask::make_response_().asCfgNode()
+    }
+
+    override DataFlow::Node getBody() { result.asCfgNode() = node.getArg(0) }
+
+    override string getMimetypeDefault() { result = "text/html" }
+
+    override DataFlow::Node getMimetypeOrContentTypeArg() { none() }
+  }
+
+  private class FlaskRouteHandlerReturn extends HTTP::Server::HttpResponse::Range, DataFlow::CfgNode {
+    FlaskRouteHandlerReturn() {
+      exists(Function routeHandler |
+        routeHandler = any(FlaskRouteSetup rs).getARouteHandler() and
+        node = routeHandler.getAReturnValueFlowNode()
+      )
+    }
+
+    override DataFlow::Node getBody() { result = this }
+
+    override DataFlow::Node getMimetypeOrContentTypeArg() { none() }
+
+    override string getMimetypeDefault() { result = "text/html" }
+  }
 }

python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll

@@ -82,19 +82,76 @@ private module Stdlib {
 
     /** Provides models for the `os.path` module */
     module path {
-      /** Gets a reference to the `os.path.join` function. */
-      private DataFlow::Node join(DataFlow::TypeTracker t) {
-        t.start() and
-        result = DataFlow::importNode("os.path.join")
+      /**
+       * Gets a reference to the attribute `attr_name` of the `os.path` module.
+       * WARNING: Only holds for a few predefined attributes.
+       *
+       * For example, using `attr_name = "join"` will get all uses of `os.path.join`.
+       */
+      private DataFlow::Node path_attr(DataFlow::TypeTracker t, string attr_name) {
+        attr_name in ["join", "normpath"] and
+        (
+          t.start() and
+          result = DataFlow::importNode("os.path." + attr_name)
+          or
+          t.startInAttr(attr_name) and
+          result = os::path()
+        )
         or
-        t.startInAttr("join") and
-        result = os::path()
-        or
-        exists(DataFlow::TypeTracker t2 | result = join(t2).track(t2, t))
+        // Due to bad performance when using normal setup with `path_attr(t2, attr_name).track(t2, t)`
+        // we have inlined that code and forced a join
+        exists(DataFlow::TypeTracker t2 |
+          exists(DataFlow::StepSummary summary |
+            path_attr_first_join(t2, attr_name, result, summary) and
+            t = t2.append(summary)
+          )
+        )
+      }
+
+      pragma[nomagic]
+      private predicate path_attr_first_join(
+        DataFlow::TypeTracker t2, string attr_name, DataFlow::Node res,
+        DataFlow::StepSummary summary
+      ) {
+        DataFlow::StepSummary::step(path_attr(t2, attr_name), res, summary)
+      }
+
+      /**
+       * Gets a reference to the attribute `attr_name` of the `os.path` module.
+       * WARNING: Only holds for a few predefined attributes.
+       *
+       * For example, using `attr_name = "join"` will get all uses of `os.path.join`.
+       */
+      DataFlow::Node path_attr(string attr_name) {
+        result = path_attr(DataFlow::TypeTracker::end(), attr_name)
       }
 
       /** Gets a reference to the `os.path.join` function. */
-      DataFlow::Node join() { result = join(DataFlow::TypeTracker::end()) }
+      DataFlow::Node join() { result = path_attr("join") }
+    }
+  }
+
+  /**
+   * A call to `os.path.normpath`.
+   * See https://docs.python.org/3/library/os.path.html#os.path.normpath
+   */
+  private class OsPathNormpathCall extends Path::PathNormalization::Range, DataFlow::CfgNode {
+    override CallNode node;
+
+    OsPathNormpathCall() { node.getFunction() = os::path::path_attr("normpath").asCfgNode() }
+
+    DataFlow::Node getPathArg() {
+      result.asCfgNode() in [node.getArg(0), node.getArgByName("path")]
+    }
+  }
+
+  /** An additional taint step for calls to `os.path.normpath` */
+  private class OsPathNormpathCallAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
+    override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+      exists(OsPathNormpathCall call |
+        nodeTo = call and
+        nodeFrom = call.getPathArg()
+      )
     }
   }
 
@@ -681,3 +738,30 @@ private class ExecStatement extends CodeExecution::Range {
 
   override DataFlow::Node getCode() { result = this }
 }
+
+/**
+ * A call to the builtin `open` function.
+ * See https://docs.python.org/3/library/functions.html#open
+ */
+private class OpenCall extends FileSystemAccess::Range, DataFlow::CfgNode {
+  override CallNode node;
+
+  OpenCall() { node.getFunction().(NameNode).getId() = "open" }
+
+  override DataFlow::Node getAPathArgument() {
+    result.asCfgNode() in [node.getArg(0), node.getArgByName("file")]
+  }
+}
+
+/**
+ * A call to the `startswith` method on a string.
+ * See https://docs.python.org/3.9/library/stdtypes.html#str.startswith
+ */
+private class StartswithCall extends Path::SafeAccessCheck::Range {
+  StartswithCall() { this.(CallNode).getFunction().(AttrNode).getName() = "startswith" }
+
+  override predicate checks(ControlFlowNode node, boolean branch) {
+    node = this.(CallNode).getFunction().(AttrNode).getObject() and
+    branch = true
+  }
+}

python/ql/src/semmle/python/regex.qll

@@ -27,7 +27,7 @@ predicate used_as_regex(Expr s, string mode) {
   (s instanceof Bytes or s instanceof Unicode) and
   /* Call to re.xxx(regex, ... [mode]) */
   exists(CallNode call, string name |
-    call.getArg(0).refersTo(_, _, s.getAFlowNode()) and
+    call.getArg(0).pointsTo(_, _, s.getAFlowNode()) and
     call.getFunction().pointsTo(Module::named("re").attr(name)) and
     not name = "escape"
   |

python/ql/test/experimental/library-tests/frameworks/django-v1/ConceptsTest.ql

@@ -1,2 +1,12 @@
 import python
 import experimental.meta.ConceptsTest
+
+class DedicatedResponseTest extends HttpServerHttpResponseTest {
+  DedicatedResponseTest() { file.getShortName() = "response_test.py" }
+}
+
+class OtherResponseTest extends HttpServerHttpResponseTest {
+  OtherResponseTest() { not this instanceof DedicatedResponseTest }
+
+  override string getARelevantTag() { result = "HttpResponse" }
+}

python/ql/test/experimental/library-tests/frameworks/django-v1/response_test.py

@@ -2,28 +2,52 @@ from django.http.response import HttpResponse, HttpResponseRedirect, JsonRespons
 
 # Not an XSS sink, since the Content-Type is not "text/html"
 # FP reported in https://github.com/github/codeql-python-team/issues/38
-def fp_json_response(request):
+def safe__json_response(request):
     # implicitly sets Content-Type to "application/json"
-    return JsonResponse({"foo": request.GET.get("foo")})
+    return JsonResponse({"foo": request.GET.get("foo")})  # $HttpResponse $mimetype=application/json $responseBody=Dict
 
 # Not an XSS sink, since the Content-Type is not "text/html"
-def fp_manual_json_response(request):
+def safe__manual_json_response(request):
     json_data = '{"json": "{}"}'.format(request.GET.get("foo"))
-    return HttpResponse(json_data, content_type="application/json")
+    return HttpResponse(json_data, content_type="application/json")  # $HttpResponse $mimetype=application/json $responseBody=json_data
 
 # Not an XSS sink, since the Content-Type is not "text/html"
-def fp_manual_content_type(request):
-    return HttpResponse('<img src="0" onerror="alert(1)">', content_type="text/plain")
+def safe__manual_content_type(request):
+    return HttpResponse('<img src="0" onerror="alert(1)">', content_type="text/plain")  # $HttpResponse $mimetype=text/plain $responseBody='<img src="0" onerror="alert(1)">'
 
 # XSS FP reported in https://github.com/github/codeql/issues/3466
-# Note: This should be a open-redirect sink, but not a XSS sink.
-def fp_redirect(request):
-    return HttpResponseRedirect(request.GET.get("next"))
+# Note: This should be an open-redirect sink, but not an XSS sink.
+def or__redirect(request):
+    return HttpResponseRedirect(request.GET.get("next"))  # $HttpResponse $mimetype=text/html; charset=utf-8 $responseBody=Attribute()
 
 # Ensure that simple subclasses are still vuln to XSS
-def tp_not_found(request):
-    return HttpResponseNotFound(request.GET.get("name"))
+def xss__not_found(request):
+    return HttpResponseNotFound(request.GET.get("name"))  # $HttpResponse $mimetype=text/html; charset=utf-8 $responseBody=Attribute()
 
-# Ensure we still have a XSS sink when manually setting the content_type to HTML
-def tp_manual_response_type(request):
-    return HttpResponse(request.GET.get("name"), content_type="text/html; charset=utf-8")
+# Ensure we still have an XSS sink when manually setting the content_type to HTML
+def xss__manual_response_type(request):
+    return HttpResponse(request.GET.get("name"), content_type="text/html; charset=utf-8")  # $HttpResponse $mimetype=text/html $responseBody=Attribute()
+
+def xss__write(request):
+    response = HttpResponse()  # $HttpResponse $mimetype=text/html; charset=utf-8
+    response.write(request.GET.get("name"))  # $HttpResponse $mimetype=text/html; charset=utf-8 $responseBody=Attribute()
+
+# This is safe but probably a bug if the argument to `write` is not a result of `json.dumps` or similar.
+def safe__write_json(request):
+    response = JsonResponse()  # $HttpResponse $mimetype=application/json
+    response.write(request.GET.get("name"))  # $HttpResponse $mimetype=application/json $responseBody=Attribute()
+
+# Ensure manual subclasses are vulnerable
+class CustomResponse(HttpResponse):
+    def __init__(self, banner, content, *args, **kwargs):
+        super().__init__(content, *args, content_type="text/html", **kwargs)
+
+def xss__custom_response(request):
+    return CustomResponse("ACME Responses", request.GET("name"))  # $HttpResponse $f-:mimetype=text/html $f-:responseBody=Attribute() $f+:responseBody="ACME Responses"
+
+class CustomJsonResponse(JsonResponse):
+    def __init__(self, banner, content, *args, **kwargs):
+        super().__init__(content, *args, content_type="text/html", **kwargs)
+
+def safe__custom_json_response(request):
+    return CustomJsonResponse("ACME Responses", {"foo": request.GET.get("foo")})  # $HttpResponse $mimetype=application/json $f-:responseBody=Dict $f+:responseBody="ACME Responses"

python/ql/test/experimental/library-tests/frameworks/django-v1/routing_test.py

@@ -5,20 +5,20 @@ from django.views.generic import View
 
 
 def url_match_xss(request, foo, bar, no_taint=None):  # $routeHandler routedParameter=foo routedParameter=bar
-    return HttpResponse('url_match_xss: {} {}'.format(foo, bar))
+    return HttpResponse('url_match_xss: {} {}'.format(foo, bar))  # $HttpResponse
 
 
 def get_params_xss(request):  # $routeHandler
-    return HttpResponse(request.GET.get("untrusted"))
+    return HttpResponse(request.GET.get("untrusted"))  # $HttpResponse
 
 
 def post_params_xss(request):  # $routeHandler
-    return HttpResponse(request.POST.get("untrusted"))
+    return HttpResponse(request.POST.get("untrusted"))  # $HttpResponse
 
 
 def http_resp_write(request):  # $routeHandler
-    rsp = HttpResponse()
-    rsp.write(request.GET.get("untrusted"))
+    rsp = HttpResponse()  # $HttpResponse
+    rsp.write(request.GET.get("untrusted"))  # $HttpResponse
     return rsp
 
 
@@ -27,22 +27,22 @@ class Foo(object):
 
 
     def post(self, request, untrusted):  # $ MISSING: routeHandler routedParameter=untrusted
-        return HttpResponse('Foo post: {}'.format(untrusted))
+        return HttpResponse('Foo post: {}'.format(untrusted))  # $HttpResponse
 
 
 class ClassView(View, Foo):
 
     def get(self, request, untrusted):  # $ MISSING: routeHandler routedParameter=untrusted
-        return HttpResponse('ClassView get: {}'.format(untrusted))
+        return HttpResponse('ClassView get: {}'.format(untrusted))  # $HttpResponse
 
 
 def show_articles(request, page_number=1):  # $routeHandler routedParameter=page_number
     page_number = int(page_number)
-    return HttpResponse('articles page: {}'.format(page_number))
+    return HttpResponse('articles page: {}'.format(page_number))  # $HttpResponse
 
 
 def xxs_positional_arg(request, arg0, arg1, no_taint=None):  # $routeHandler routedParameter=arg0 routedParameter=arg1
-    return HttpResponse('xxs_positional_arg: {} {}'.format(arg0, arg1))
+    return HttpResponse('xxs_positional_arg: {} {}'.format(arg0, arg1))  # $HttpResponse
 
 
 urlpatterns = [
@@ -63,7 +63,7 @@ urlpatterns = [
 # Using patterns() for routing
 
 def show_user(request, username):  # $routeHandler routedParameter=username
-    return HttpResponse('show_user {}'.format(username))
+    return HttpResponse('show_user {}'.format(username))  # $HttpResponse
 
 
 urlpatterns = patterns(url(r"^users/(?P<username>[^/]+)", show_user))  # $routeSetup="^users/(?P<username>[^/]+)"
@@ -72,7 +72,7 @@ urlpatterns = patterns(url(r"^users/(?P<username>[^/]+)", show_user))  # $routeS
 # Show we understand the keyword arguments to django.conf.urls.url
 
 def kw_args(request):  # $routeHandler
-    return HttpResponse('kw_args')
+    return HttpResponse('kw_args')  # $HttpResponse
 
 urlpatterns = [
     url(view=kw_args, regex=r"^kw_args")  # $routeSetup="^kw_args"

python/ql/test/experimental/library-tests/frameworks/django-v2-v3/ConceptsTest.ql

@@ -1,2 +1,12 @@
 import python
 import experimental.meta.ConceptsTest
+
+class DedicatedResponseTest extends HttpServerHttpResponseTest {
+  DedicatedResponseTest() { file.getShortName() = "response_test.py" }
+}
+
+class OtherResponseTest extends HttpServerHttpResponseTest {
+  OtherResponseTest() { not this instanceof DedicatedResponseTest }
+
+  override string getARelevantTag() { result = "HttpResponse" }
+}

python/ql/test/experimental/library-tests/frameworks/django-v2-v3/routing_test.py

@@ -5,20 +5,20 @@ from django.views import View
 
 
 def url_match_xss(request, foo, bar, no_taint=None):  # $routeHandler routedParameter=foo routedParameter=bar
-    return HttpResponse('url_match_xss: {} {}'.format(foo, bar))
+    return HttpResponse('url_match_xss: {} {}'.format(foo, bar))  # $HttpResponse
 
 
 def get_params_xss(request):  # $routeHandler
-    return HttpResponse(request.GET.get("untrusted"))
+    return HttpResponse(request.GET.get("untrusted"))  # $HttpResponse
 
 
 def post_params_xss(request):  # $routeHandler
-    return HttpResponse(request.POST.get("untrusted"))
+    return HttpResponse(request.POST.get("untrusted"))  # $HttpResponse
 
 
 def http_resp_write(request):  # $routeHandler
-    rsp = HttpResponse()
-    rsp.write(request.GET.get("untrusted"))
+    rsp = HttpResponse()  # $HttpResponse
+    rsp.write(request.GET.get("untrusted"))  # $HttpResponse
     return rsp
 
 
@@ -27,22 +27,22 @@ class Foo(object):
 
 
     def post(self, request, untrusted):  # $ MISSING: routeHandler routedParameter=untrusted
-        return HttpResponse('Foo post: {}'.format(untrusted))
+        return HttpResponse('Foo post: {}'.format(untrusted))  # $HttpResponse
 
 
 class ClassView(View, Foo):
 
-    def get(self, request, untrusted):  #  $ MISSING: routeHandler routedParameter=untrusted
-        return HttpResponse('ClassView get: {}'.format(untrusted))
+    def get(self, request, untrusted):  # $ MISSING: routeHandler routedParameter=untrusted
+        return HttpResponse('ClassView get: {}'.format(untrusted))  # $HttpResponse
 
 
 def show_articles(request, page_number=1):  # $routeHandler routedParameter=page_number
     page_number = int(page_number)
-    return HttpResponse('articles page: {}'.format(page_number))
+    return HttpResponse('articles page: {}'.format(page_number))  # $HttpResponse
 
 
 def xxs_positional_arg(request, arg0, arg1, no_taint=None):  # $routeHandler routedParameter=arg0 routedParameter=arg1
-    return HttpResponse('xxs_positional_arg: {} {}'.format(arg0, arg1))
+    return HttpResponse('xxs_positional_arg: {} {}'.format(arg0, arg1))  # $HttpResponse
 
 
 urlpatterns = [
@@ -63,7 +63,7 @@ urlpatterns = [
 # Show we understand the keyword arguments to django.urls.re_path
 
 def re_path_kwargs(request):  # $routeHandler
-    return HttpResponse('re_path_kwargs')
+    return HttpResponse('re_path_kwargs')  # $HttpResponse
 
 
 urlpatterns = [
@@ -76,16 +76,16 @@ urlpatterns = [
 
 # saying page_number is an externally controlled *string* is a bit strange, when we have an int converter :O
 def page_number(request, page_number=1):  # $routeHandler routedParameter=page_number
-    return HttpResponse('page_number: {}'.format(page_number))
+    return HttpResponse('page_number: {}'.format(page_number))  # $HttpResponse
 
-def foo_bar_baz(request, foo, bar, baz):  # $routeHandler routedParameter=foo routedParameter=bar routedParameter=baz
-    return HttpResponse('foo_bar_baz: {} {} {}'.format(foo, bar, baz))
+def foo_bar_baz(request, foo, bar, baz):  # $routeHandler $routedParameter=foo routedParameter=bar routedParameter=baz
+    return HttpResponse('foo_bar_baz: {} {} {}'.format(foo, bar, baz))  # $HttpResponse
 
 def path_kwargs(request, foo, bar):  # $routeHandler routedParameter=foo routedParameter=bar
-    return HttpResponse('path_kwargs: {} {} {}'.format(foo, bar))
+    return HttpResponse('path_kwargs: {} {} {}'.format(foo, bar))  # $HttpResponse
 
 def not_valid_identifier(request):  # $routeHandler
-    return HttpResponse('<foo!>')
+    return HttpResponse('<foo!>')  # $HttpResponse
 
 urlpatterns = [
     path("articles/", page_number),  # $routeSetup="articles/"
@@ -102,7 +102,7 @@ urlpatterns = [
 from django.conf.urls import url
 
 def deprecated(request):  # $routeHandler
-    return HttpResponse('deprecated')
+    return HttpResponse('deprecated')  # $HttpResponse
 
 urlpatterns = [
     url(r"^deprecated/", deprecated),  # $routeSetup="^deprecated/"

python/ql/test/experimental/library-tests/frameworks/django-v2-v3/testapp/views.py

@@ -1,10 +1,10 @@
 from django.http import HttpRequest, HttpResponse
 
 def foo(request: HttpRequest):  # $routeHandler
-    return HttpResponse("foo")
+    return HttpResponse("foo")  # $HttpResponse
 
 def bar_baz(request: HttpRequest):  # $routeHandler
-    return HttpResponse("bar_baz")
+    return HttpResponse("bar_baz")  # $HttpResponse
 
 def deprecated(request: HttpRequest):  # $routeHandler
-    return HttpResponse("deprecated")
+    return HttpResponse("deprecated")  # $HttpResponse

python/ql/test/experimental/library-tests/frameworks/flask/ConceptsTest.ql

@@ -1,2 +1,12 @@
 import python
 import experimental.meta.ConceptsTest
+
+class DedicatedResponseTest extends HttpServerHttpResponseTest {
+  DedicatedResponseTest() { file.getShortName() = "response_test.py" }
+}
+
+class OtherResponseTest extends HttpServerHttpResponseTest {
+  OtherResponseTest() { not this instanceof DedicatedResponseTest }
+
+  override string getARelevantTag() { result = "HttpResponse" }
+}

python/ql/test/experimental/library-tests/frameworks/flask/old_test.py

@@ -5,7 +5,7 @@ app = Flask(__name__)
 
 @app.route("/")  # $routeSetup="/"
 def hello_world():  # $routeHandler
-    return "Hello World!"
+    return "Hello World!"  # $HttpResponse
 
 from flask.views import MethodView
 
@@ -26,42 +26,42 @@ app.add_url_rule('/the/', defaults={'user_id': None},  # $routeSetup="/the/"
 
 @app.route("/dangerous")  # $routeSetup="/dangerous"
 def dangerous():  # $routeHandler
-    return request.args.get('payload')
+    return request.args.get('payload')  # $HttpResponse
 
 @app.route("/dangerous-with-cfg-split")  # $routeSetup="/dangerous-with-cfg-split"
 def dangerous2():  # $routeHandler
     x = request.form['param0']
     if request.method == "POST":
-        return request.form['param1']
-    return None
+        return request.form['param1']  # $HttpResponse
+    return None  # $f+:HttpResponse
 
 @app.route("/unsafe")  # $routeSetup="/unsafe"
 def unsafe():  # $routeHandler
     first_name = request.args.get('name', '')
-    return make_response("Your name is " + first_name)
+    return make_response("Your name is " + first_name)  # $HttpResponse
 
 @app.route("/safe")  # $routeSetup="/safe"
 def safe():  # $routeHandler
     first_name = request.args.get('name', '')
-    return make_response("Your name is " + escape(first_name))
+    return make_response("Your name is " + escape(first_name))  # $HttpResponse
 
 @app.route("/hello/<name>")  # $routeSetup="/hello/<name>"
 def hello(name):  # $routeHandler routedParameter=name
-    return make_response("Your name is " + name)
+    return make_response("Your name is " + name)  # $HttpResponse
 
 @app.route("/foo/<path:subpath>")  # $routeSetup="/foo/<path:subpath>"
 def foo(subpath):  # $routeHandler routedParameter=subpath
-    return make_response("The subpath is " + subpath)
+    return make_response("The subpath is " + subpath)  # $HttpResponse
 
 @app.route("/multiple/")  # $routeSetup="/multiple/"
 @app.route("/multiple/foo/<foo>")  # $routeSetup="/multiple/foo/<foo>"
 @app.route("/multiple/bar/<bar>")  # $routeSetup="/multiple/bar/<bar>"
 def multiple(foo=None, bar=None):  # $routeHandler routedParameter=foo routedParameter=bar
-    return make_response("foo={!r} bar={!r}".format(foo, bar))
+    return make_response("foo={!r} bar={!r}".format(foo, bar))  # $HttpResponse
 
 @app.route("/complex/<string(length=2):lang_code>")  # $routeSetup="/complex/<string(length=2):lang_code>"
 def complex(lang_code):  # $routeHandler routedParameter=lang_code
-    return make_response("lang_code {}".format(lang_code))
+    return make_response("lang_code {}".format(lang_code))  # $HttpResponse
 
 if __name__ == "__main__":
     app.run(debug=True)

python/ql/test/experimental/library-tests/frameworks/flask/response_test.py

@@ -0,0 +1,179 @@
+import json
+
+from flask import Flask, make_response, jsonify, Response, request
+
+app = Flask(__name__)
+
+
+@app.route("/html1")  # $routeSetup="/html1"
+def html1():  # $routeHandler
+    return "<h1>hello</h1>"  # $HttpResponse $mimetype=text/html $responseBody="<h1>hello</h1>"
+
+
+@app.route("/html2")  # $routeSetup="/html2"
+def html2():  # $routeHandler
+    # note that response saved in a variable intentionally -- we wan the annotations to
+    # show that we recognize the response creation, and not the return (hopefully). (and
+    # do the same in the following of the file)
+    resp = make_response("<h1>hello</h1>")  # $HttpResponse $mimetype=text/html $responseBody="<h1>hello</h1>"
+    return resp  # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
+
+
+@app.route("/html3")  # $routeSetup="/html3"
+def html3():  # $routeHandler
+    resp = app.make_response("<h1>hello</h1>")  # $HttpResponse $mimetype=text/html $responseBody="<h1>hello</h1>"
+    return resp  # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
+
+
+# TODO: Create test-cases for the many ways that `make_response` can be used
+# https://flask.palletsprojects.com/en/1.1.x/api/#flask.Flask.make_response
+
+
+@app.route("/html4")  # $routeSetup="/html4"
+def html4():  # $routeHandler
+    resp = Response("<h1>hello</h1>")  # $HttpResponse $mimetype=text/html $responseBody="<h1>hello</h1>"
+    return resp  # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
+
+
+@app.route("/html5")  # $routeSetup="/html5"
+def html5():  # $routeHandler
+    # note: flask.Flask.response_class is set to `flask.Response` by default.
+    # it can be overridden, but we don't try to handle that right now.
+    resp = Flask.response_class("<h1>hello</h1>")  # $HttpResponse $mimetype=text/html $responseBody="<h1>hello</h1>"
+    return resp  # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
+
+
+@app.route("/html6")  # $routeSetup="/html6"
+def html6():  # $routeHandler
+    # note: app.response_class (flask.Flask.response_class) is set to `flask.Response` by default.
+    # it can be overridden, but we don't try to handle that right now.
+    resp = app.response_class("<h1>hello</h1>")  # $HttpResponse $mimetype=text/html $responseBody="<h1>hello</h1>"
+    return resp  # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
+
+
+@app.route("/html7")  # $routeSetup="/html7"
+def html7():  # $routeHandler
+    resp = make_response()  # $HttpResponse $mimetype=text/html
+    resp.set_data("<h1>hello</h1>")  # $f-:responseBody="<h1>hello</h1>"
+    return resp  # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
+
+
+@app.route("/jsonify")  # $routeSetup="/jsonify"
+def jsonify_route():  # $routeHandler
+    data = {"foo": "bar"}
+    resp = jsonify(data)  # $f-:HttpResponse $f-:mimetype=application/json $f-:responseBody=data
+    return resp  # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
+
+################################################################################
+# Tricky return handling
+################################################################################
+
+@app.route("/tricky-return1")  # $routeSetup="/tricky-return1"
+def tricky_return1():  # $routeHandler
+    if "raw" in request.args:
+        resp = "<h1>hellu</h1>"
+    else:
+        resp = make_response("<h1>hello</h1>")  # $HttpResponse $mimetype=text/html $responseBody="<h1>hello</h1>"
+    return resp  # $HttpResponse $mimetype=text/html $responseBody=resp
+
+def helper():
+    if "raw" in request.args:
+        return "<h1>hellu</h1>"
+    else:
+        return make_response("<h1>hello</h1>")  # $HttpResponse $mimetype=text/html $responseBody="<h1>hello</h1>"
+
+@app.route("/tricky-return2")  # $routeSetup="/tricky-return2"
+def tricky_return2():  # $routeHandler
+    resp = helper()
+    return resp  # $HttpResponse $mimetype=text/html $responseBody=resp
+
+
+################################################################################
+# Setting content-type manually
+################################################################################
+
+
+@app.route("/content-type/response-modification1")  # $routeSetup="/content-type/response-modification1"
+def response_modification1():  # $routeHandler
+    resp = make_response("<h1>hello</h1>")  # $HttpResponse $mimetype=text/html $responseBody="<h1>hello</h1>"
+    resp.content_type = "text/plain"  # $f-:HttpResponse $f-:mimetype=text/plain
+    return resp  # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
+
+
+@app.route("/content-type/response-modification2")  # $routeSetup="/content-type/response-modification2"
+def response_modification2():  # $routeHandler
+    resp = make_response("<h1>hello</h1>")  # $HttpResponse $mimetype=text/html $responseBody="<h1>hello</h1>"
+    resp.headers["content-type"] = "text/plain"  # $f-:HttpResponse $f-:mimetype=text/plain
+    return resp  # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
+
+
+# Exploration of mimetype/content_type/headers arguments to `app.response_class` -- things can get tricky
+# see https://werkzeug.palletsprojects.com/en/1.0.x/wrappers/#werkzeug.wrappers.Response
+
+
+@app.route("/content-type/Response1")  # $routeSetup="/content-type/Response1"
+def Response1():  # $routeHandler
+    resp = Response("<h1>hello</h1>", mimetype="text/plain")  # $HttpResponse $mimetype=text/plain $responseBody="<h1>hello</h1>"
+    return resp  # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
+
+
+@app.route("/content-type/Response2")  # $routeSetup="/content-type/Response2"
+def Response2():  # $routeHandler
+    resp = Response("<h1>hello</h1>", content_type="text/plain; charset=utf-8")  # $HttpResponse $mimetype=text/plain $responseBody="<h1>hello</h1>"
+    return resp  # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
+
+
+@app.route("/content-type/Response3")  # $routeSetup="/content-type/Response3"
+def Response3():  # $routeHandler
+    # content_type argument takes priority (and result is text/plain)
+    resp = Response("<h1>hello</h1>", content_type="text/plain; charset=utf-8", mimetype="text/html")  # $HttpResponse $mimetype=text/plain $responseBody="<h1>hello</h1>"
+    return resp  # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
+
+
+@app.route("/content-type/Response4")  # $routeSetup="/content-type/Response4"
+def Response4():  # $routeHandler
+    # note: capitalization of Content-Type does not matter
+    resp = Response("<h1>hello</h1>", headers={"Content-TYPE": "text/plain"})  # $HttpResponse $f+:mimetype=text/html $f-:mimetype=text/plain $responseBody="<h1>hello</h1>"
+    return resp  # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
+
+
+@app.route("/content-type/Response5")  # $routeSetup="/content-type/Response5"
+def Response5():  # $routeHandler
+    # content_type argument takes priority (and result is text/plain)
+    # note: capitalization of Content-Type does not matter
+    resp = Response("<h1>hello</h1>", headers={"Content-TYPE": "text/html"}, content_type="text/plain; charset=utf-8")  # $HttpResponse $mimetype=text/plain $responseBody="<h1>hello</h1>"
+    return resp  # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
+
+
+@app.route("/content-type/Response6")  # $routeSetup="/content-type/Response6"
+def Response6():  # $routeHandler
+    # mimetype argument takes priority over header (and result is text/plain)
+    # note: capitalization of Content-Type does not matter
+    resp = Response("<h1>hello</h1>", headers={"Content-TYPE": "text/html"}, mimetype="text/plain")  # $HttpResponse $mimetype=text/plain $responseBody="<h1>hello</h1>"
+    return resp  # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
+
+
+@app.route("/content-type/Flask-response-class")  # $routeSetup="/content-type/Flask-response-class"
+def Flask_response_class():  # $routeHandler
+    # note: flask.Flask.response_class is set to `flask.Response` by default.
+    # it can be overridden, but we don't try to handle that right now.
+    resp = Flask.response_class("<h1>hello</h1>", mimetype="text/plain")  # $HttpResponse $mimetype=text/plain $responseBody="<h1>hello</h1>"
+    return resp  # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
+
+
+@app.route("/content-type/app-response-class")  # $routeSetup="/content-type/app-response-class"
+def app_response_class():  # $routeHandler
+    # note: app.response_class (flask.Flask.response_class) is set to `flask.Response` by default.
+    # it can be overridden, but we don't try to handle that right now.
+    resp = app.response_class("<h1>hello</h1>", mimetype="text/plain")  # $HttpResponse $mimetype=text/plain $responseBody="<h1>hello</h1>"
+    return resp  # $f+:HttpResponse $f+:mimetype=text/html $f+:responseBody=resp
+
+
+# TODO: add tests for setting status code
+# TODO: add test that manually creates a redirect by setting status code and suitable header.
+
+################################################################################
+
+
+if __name__ == "__main__":
+    app.run(debug=True)

python/ql/test/experimental/library-tests/frameworks/flask/routing_test.py

@@ -7,24 +7,24 @@ app = Flask(__name__)
 SOME_ROUTE = "/some/route"
 @app.route(SOME_ROUTE) # $routeSetup="/some/route"
 def some_route():  # $routeHandler
-    return make_response("some_route")
+    return make_response("some_route")  # $HttpResponse
 
 
 def index():  # $routeHandler
-    return make_response("index")
+    return make_response("index")  # $HttpResponse
 app.add_url_rule('/index', 'index', index)  # $routeSetup="/index"
 
 
 # We don't support this yet, and I think that's OK
 def later_set():  # $ MISSING: routeHandler
-    return make_response("later_set")
+    return make_response("later_set")  # $HttpResponse
 app.add_url_rule('/later-set', 'later_set', view_func=None)  # $routeSetup="/later-set"
 app.view_functions['later_set'] = later_set
 
 
 @app.route(UNKNOWN_ROUTE) # $routeSetup
 def unkown_route(foo, bar):  # $routeHandler routedParameter=foo routedParameter=bar
-    return make_response("unkown_route")
+    return make_response("unkown_route")  # $HttpResponse
 
 
 if __name__ == "__main__":

python/ql/test/experimental/library-tests/frameworks/flask/taint_test.py

@@ -200,7 +200,7 @@ def debug(foo, bar):  # $routeHandler routedParameter=foo routedParameter=bar
 
     print("request.pragma {!r}".format(request.pragma))
 
-    return 'ok'
+    return 'ok'  # $HttpResponse
 
 @app.route("/stream", methods=['POST'])  # $routeSetup="/stream"
 def stream():  # $routeHandler
@@ -210,7 +210,7 @@ def stream():  # $routeHandler
     # just works :)
     print(s.read())
 
-    return 'ok'
+    return 'ok'  # $HttpResponse
 
 @app.route("/input_stream", methods=['POST'])  # $routeSetup="/input_stream"
 def input_stream():  # $routeHandler
@@ -221,14 +221,14 @@ def input_stream():  # $routeHandler
     # be handled manually
     print(s.read())
 
-    return 'ok'
+    return 'ok'  # $HttpResponse
 
 @app.route("/form", methods=['POST'])  # $routeSetup="/form"
 def form():  # $routeHandler
     print(request.path)
     print("request.form", request.form)
 
-    return 'ok'
+    return 'ok'  # $HttpResponse
 
 @app.route("/cache_control", methods=['POST'])  # $routeSetup="/cache_control"
 def cache_control():  # $routeHandler
@@ -237,7 +237,7 @@ def cache_control():  # $routeHandler
     print("request.cache_control.max_stale", request.cache_control.max_stale, type(request.cache_control.max_stale))
     print("request.cache_control.min_fresh", request.cache_control.min_fresh, type(request.cache_control.min_fresh))
 
-    return 'ok'
+    return 'ok'  # $HttpResponse
 
 @app.route("/file_upload", methods=['POST'])  # $routeSetup="/file_upload"
 def file_upload():  # $routeHandler
@@ -245,14 +245,14 @@ def file_upload():  # $routeHandler
     for k,v in request.files.items():
         print(k, v, v.name, v.filename, v.stream)
 
-    return 'ok'
+    return 'ok'  # $HttpResponse
 
 @app.route("/args", methods=['GET'])  # $routeSetup="/args"
 def args():  # $routeHandler
     print(request.path)
     print("request.args", request.args)
 
-    return 'ok'
+    return 'ok'  # $HttpResponse
 
 # curl --header "My-Header: some-value" http://localhost:5000/debug/fooval/barval
 # curl --header "Pragma: foo, bar" --header "Pragma: stuff, foo" http://localhost:5000/debug/fooval/barval

python/ql/test/experimental/library-tests/frameworks/stdlib/FileSystemAccess.py

@@ -0,0 +1,7 @@
+open("filepath")  # $getAPathArgument="filepath"
+open(file="filepath")  # $getAPathArgument="filepath"
+
+o = open
+
+o("filepath")  # f-:$getAPathArgument="filepath"
+o(file="filepath")  # f-:$getAPathArgument="filepath"

python/ql/test/experimental/library-tests/frameworks/stdlib/PathNormalization.py

@@ -0,0 +1,19 @@
+import os.path
+
+path = "un\\normalized/path"
+
+p1 = os.path.normpath(path)  # $pathNormalization
+p2 = os.path.normpath(path=path)  # $pathNormalization
+
+np = os.path.normpath
+
+p3 = np(path)  # $pathNormalization
+p4 = np(path=path)  # $pathNormalization
+
+
+def normalize(path):
+    return os.path.normpath(path)  # $pathNormalization
+
+
+p5 = normalize(path)
+p6 = normalize(path=path)

python/ql/test/experimental/library-tests/frameworks/stdlib/SafeAccessCheck.py

@@ -0,0 +1,8 @@
+s = "taintedString"
+
+if s.startswith("tainted"):  # $checks=s $branch=true
+    pass
+
+sw = s.startswith  # $f-:checks=s $f-:branch=true
+if sw("safe"):
+    pass

python/ql/test/experimental/meta/ConceptsTest.qll

@@ -142,3 +142,101 @@ class HttpServerRouteSetupTest extends InlineExpectationsTest {
     )
   }
 }
+
+class HttpServerHttpResponseTest extends InlineExpectationsTest {
+  File file;
+
+  HttpServerHttpResponseTest() {
+    file.getExtension() = "py" and
+    this = "HttpServerHttpResponseTest: " + file
+  }
+
+  override string getARelevantTag() { result in ["HttpResponse", "responseBody", "mimetype"] }
+
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    // By adding `file` as a class field, and these two restrictions, it's possible to
+    // say that we only want to check _some_ tags for certain files. This helped make
+    // flask tests more readable since adding full annotations for HttpResponses in the
+    // the tests for routing setup is both annoying and not very useful.
+    location.getFile() = file and
+    tag = getARelevantTag() and
+    (
+      exists(HTTP::Server::HttpResponse response |
+        location = response.getLocation() and
+        element = response.toString() and
+        value = "" and
+        tag = "HttpResponse"
+      )
+      or
+      exists(HTTP::Server::HttpResponse response |
+        location = response.getLocation() and
+        element = response.toString() and
+        value = value_from_expr(response.getBody().asExpr()) and
+        tag = "responseBody"
+      )
+      or
+      exists(HTTP::Server::HttpResponse response |
+        location = response.getLocation() and
+        element = response.toString() and
+        value = response.getMimetype() and
+        tag = "mimetype"
+      )
+    )
+  }
+}
+
+class FileSystemAccessTest extends InlineExpectationsTest {
+  FileSystemAccessTest() { this = "FileSystemAccessTest" }
+
+  override string getARelevantTag() { result = "getAPathArgument" }
+
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    exists(FileSystemAccess a, DataFlow::Node path |
+      exists(location.getFile().getRelativePath()) and
+      path = a.getAPathArgument() and
+      location = a.getLocation() and
+      element = path.toString() and
+      value = value_from_expr(path.asExpr()) and
+      tag = "getAPathArgument"
+    )
+  }
+}
+
+class PathNormalizationTest extends InlineExpectationsTest {
+  PathNormalizationTest() { this = "PathNormalizationTest" }
+
+  override string getARelevantTag() { result = "pathNormalization" }
+
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    exists(Path::PathNormalization n |
+      exists(location.getFile().getRelativePath()) and
+      location = n.getLocation() and
+      element = n.toString() and
+      value = "" and
+      tag = "pathNormalization"
+    )
+  }
+}
+
+class SafeAccessCheckTest extends InlineExpectationsTest {
+  SafeAccessCheckTest() { this = "SafeAccessCheckTest" }
+
+  override string getARelevantTag() { result in ["checks", "branch"] }
+
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    exists(Path::SafeAccessCheck c, DataFlow::Node checks, boolean branch |
+      exists(location.getFile().getRelativePath()) and
+      c.checks(checks.asCfgNode(), branch) and
+      location = c.getLocation() and
+      (
+        element = checks.toString() and
+        value = value_from_expr(checks.asExpr()) and
+        tag = "checks"
+        or
+        element = branch.toString() and
+        value = branch.toString() and
+        tag = "branch"
+      )
+    )
+  }
+}

python/ql/test/experimental/query-tests/Security-new-dataflow/CWE-022/PathInjection.expected

@@ -0,0 +1,94 @@
+edges
+| path_injection.py:9:12:9:23 | ControlFlowNode for Attribute | path_injection.py:10:14:10:44 | ControlFlowNode for Attribute() |
+| path_injection.py:15:12:15:23 | ControlFlowNode for Attribute | path_injection.py:16:13:16:61 | ControlFlowNode for Attribute() |
+| path_injection.py:16:13:16:61 | ControlFlowNode for Attribute() | path_injection.py:17:14:17:18 | ControlFlowNode for npath |
+| path_injection.py:24:12:24:23 | ControlFlowNode for Attribute | path_injection.py:25:13:25:61 | ControlFlowNode for Attribute() |
+| path_injection.py:25:13:25:61 | ControlFlowNode for Attribute() | path_injection.py:28:14:28:18 | ControlFlowNode for npath |
+| path_injection.py:33:12:33:23 | ControlFlowNode for Attribute | path_injection.py:34:13:34:61 | ControlFlowNode for Attribute() |
+| test.py:9:12:9:23 | ControlFlowNode for Attribute | test.py:9:12:9:39 | ControlFlowNode for Attribute() |
+| test.py:9:12:9:23 | ControlFlowNode for Attribute | test.py:9:12:9:39 | ControlFlowNode for Attribute() |
+| test.py:9:12:9:39 | ControlFlowNode for Attribute() | test.py:18:9:18:16 | ControlFlowNode for source() |
+| test.py:9:12:9:39 | ControlFlowNode for Attribute() | test.py:24:9:24:16 | ControlFlowNode for source() |
+| test.py:9:12:9:39 | ControlFlowNode for Attribute() | test.py:31:9:31:16 | ControlFlowNode for source() |
+| test.py:9:12:9:39 | ControlFlowNode for Attribute() | test.py:38:9:38:16 | ControlFlowNode for source() |
+| test.py:9:12:9:39 | ControlFlowNode for Attribute() | test.py:46:9:46:16 | ControlFlowNode for source() |
+| test.py:12:15:12:15 | SSA variable x | test.py:13:12:13:30 | ControlFlowNode for Attribute() |
+| test.py:13:12:13:30 | ControlFlowNode for Attribute() | test.py:25:9:25:20 | ControlFlowNode for normalize() |
+| test.py:13:12:13:30 | ControlFlowNode for Attribute() | test.py:48:13:48:24 | ControlFlowNode for normalize() |
+| test.py:18:9:18:16 | ControlFlowNode for source() | test.py:19:10:19:10 | ControlFlowNode for x |
+| test.py:24:9:24:16 | ControlFlowNode for source() | test.py:25:19:25:19 | ControlFlowNode for x |
+| test.py:25:9:25:20 | ControlFlowNode for normalize() | test.py:26:10:26:10 | ControlFlowNode for y |
+| test.py:25:19:25:19 | ControlFlowNode for x | test.py:12:15:12:15 | SSA variable x |
+| test.py:31:9:31:16 | ControlFlowNode for source() | test.py:33:14:33:14 | ControlFlowNode for x |
+| test.py:38:9:38:16 | ControlFlowNode for source() | test.py:39:19:39:19 | ControlFlowNode for x |
+| test.py:39:19:39:19 | ControlFlowNode for x | test.py:12:15:12:15 | SSA variable x |
+| test.py:46:9:46:16 | ControlFlowNode for source() | test.py:48:23:48:23 | ControlFlowNode for x |
+| test.py:48:13:48:24 | ControlFlowNode for normalize() | test.py:49:14:49:14 | ControlFlowNode for y |
+| test.py:48:23:48:23 | ControlFlowNode for x | test.py:12:15:12:15 | SSA variable x |
+| test_chaining.py:9:12:9:23 | ControlFlowNode for Attribute | test_chaining.py:9:12:9:39 | ControlFlowNode for Attribute() |
+| test_chaining.py:9:12:9:39 | ControlFlowNode for Attribute() | test_chaining.py:20:9:20:16 | ControlFlowNode for source() |
+| test_chaining.py:9:12:9:39 | ControlFlowNode for Attribute() | test_chaining.py:28:9:28:16 | ControlFlowNode for source() |
+| test_chaining.py:9:12:9:39 | ControlFlowNode for Attribute() | test_chaining.py:41:9:41:16 | ControlFlowNode for source() |
+| test_chaining.py:14:15:14:15 | SSA variable x | test_chaining.py:15:12:15:30 | ControlFlowNode for Attribute() |
+| test_chaining.py:15:12:15:30 | ControlFlowNode for Attribute() | test_chaining.py:31:13:31:24 | ControlFlowNode for normalize() |
+| test_chaining.py:20:9:20:16 | ControlFlowNode for source() | test_chaining.py:21:19:21:19 | ControlFlowNode for x |
+| test_chaining.py:21:19:21:19 | ControlFlowNode for x | test_chaining.py:14:15:14:15 | SSA variable x |
+| test_chaining.py:28:9:28:16 | ControlFlowNode for source() | test_chaining.py:29:19:29:19 | ControlFlowNode for x |
+| test_chaining.py:29:19:29:19 | ControlFlowNode for x | test_chaining.py:14:15:14:15 | SSA variable x |
+| test_chaining.py:31:13:31:24 | ControlFlowNode for normalize() | test_chaining.py:32:14:32:14 | ControlFlowNode for z |
+| test_chaining.py:41:9:41:16 | ControlFlowNode for source() | test_chaining.py:42:9:42:19 | ControlFlowNode for normpath() |
+| test_chaining.py:44:13:44:23 | ControlFlowNode for normpath() | test_chaining.py:45:14:45:14 | ControlFlowNode for z |
+nodes
+| path_injection.py:9:12:9:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| path_injection.py:10:14:10:44 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| path_injection.py:15:12:15:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| path_injection.py:16:13:16:61 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| path_injection.py:17:14:17:18 | ControlFlowNode for npath | semmle.label | ControlFlowNode for npath |
+| path_injection.py:24:12:24:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| path_injection.py:25:13:25:61 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| path_injection.py:28:14:28:18 | ControlFlowNode for npath | semmle.label | ControlFlowNode for npath |
+| path_injection.py:33:12:33:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| path_injection.py:34:13:34:61 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| test.py:9:12:9:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| test.py:9:12:9:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| test.py:9:12:9:39 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| test.py:9:12:9:39 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| test.py:12:15:12:15 | SSA variable x | semmle.label | SSA variable x |
+| test.py:13:12:13:30 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| test.py:18:9:18:16 | ControlFlowNode for source() | semmle.label | ControlFlowNode for source() |
+| test.py:19:10:19:10 | ControlFlowNode for x | semmle.label | ControlFlowNode for x |
+| test.py:24:9:24:16 | ControlFlowNode for source() | semmle.label | ControlFlowNode for source() |
+| test.py:25:9:25:20 | ControlFlowNode for normalize() | semmle.label | ControlFlowNode for normalize() |
+| test.py:25:19:25:19 | ControlFlowNode for x | semmle.label | ControlFlowNode for x |
+| test.py:26:10:26:10 | ControlFlowNode for y | semmle.label | ControlFlowNode for y |
+| test.py:31:9:31:16 | ControlFlowNode for source() | semmle.label | ControlFlowNode for source() |
+| test.py:33:14:33:14 | ControlFlowNode for x | semmle.label | ControlFlowNode for x |
+| test.py:38:9:38:16 | ControlFlowNode for source() | semmle.label | ControlFlowNode for source() |
+| test.py:39:19:39:19 | ControlFlowNode for x | semmle.label | ControlFlowNode for x |
+| test.py:46:9:46:16 | ControlFlowNode for source() | semmle.label | ControlFlowNode for source() |
+| test.py:48:13:48:24 | ControlFlowNode for normalize() | semmle.label | ControlFlowNode for normalize() |
+| test.py:48:23:48:23 | ControlFlowNode for x | semmle.label | ControlFlowNode for x |
+| test.py:49:14:49:14 | ControlFlowNode for y | semmle.label | ControlFlowNode for y |
+| test_chaining.py:9:12:9:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| test_chaining.py:9:12:9:39 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| test_chaining.py:14:15:14:15 | SSA variable x | semmle.label | SSA variable x |
+| test_chaining.py:15:12:15:30 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| test_chaining.py:20:9:20:16 | ControlFlowNode for source() | semmle.label | ControlFlowNode for source() |
+| test_chaining.py:21:19:21:19 | ControlFlowNode for x | semmle.label | ControlFlowNode for x |
+| test_chaining.py:28:9:28:16 | ControlFlowNode for source() | semmle.label | ControlFlowNode for source() |
+| test_chaining.py:29:19:29:19 | ControlFlowNode for x | semmle.label | ControlFlowNode for x |
+| test_chaining.py:31:13:31:24 | ControlFlowNode for normalize() | semmle.label | ControlFlowNode for normalize() |
+| test_chaining.py:32:14:32:14 | ControlFlowNode for z | semmle.label | ControlFlowNode for z |
+| test_chaining.py:41:9:41:16 | ControlFlowNode for source() | semmle.label | ControlFlowNode for source() |
+| test_chaining.py:42:9:42:19 | ControlFlowNode for normpath() | semmle.label | ControlFlowNode for normpath() |
+| test_chaining.py:44:13:44:23 | ControlFlowNode for normpath() | semmle.label | ControlFlowNode for normpath() |
+| test_chaining.py:45:14:45:14 | ControlFlowNode for z | semmle.label | ControlFlowNode for z |
+#select
+| path_injection.py:10:14:10:44 | ControlFlowNode for Attribute() | path_injection.py:9:12:9:23 | ControlFlowNode for Attribute | path_injection.py:10:14:10:44 | ControlFlowNode for Attribute() | This path depends on $@. | path_injection.py:9:12:9:23 | ControlFlowNode for Attribute | a user-provided value |
+| path_injection.py:17:14:17:18 | ControlFlowNode for npath | path_injection.py:15:12:15:23 | ControlFlowNode for Attribute | path_injection.py:17:14:17:18 | ControlFlowNode for npath | This path depends on $@. | path_injection.py:15:12:15:23 | ControlFlowNode for Attribute | a user-provided value |
+| path_injection.py:28:14:28:18 | ControlFlowNode for npath | path_injection.py:24:12:24:23 | ControlFlowNode for Attribute | path_injection.py:28:14:28:18 | ControlFlowNode for npath | This path depends on $@. | path_injection.py:24:12:24:23 | ControlFlowNode for Attribute | a user-provided value |
+| test.py:19:10:19:10 | ControlFlowNode for x | test.py:9:12:9:23 | ControlFlowNode for Attribute | test.py:19:10:19:10 | ControlFlowNode for x | This path depends on $@. | test.py:9:12:9:23 | ControlFlowNode for Attribute | a user-provided value |
+| test.py:26:10:26:10 | ControlFlowNode for y | test.py:9:12:9:23 | ControlFlowNode for Attribute | test.py:26:10:26:10 | ControlFlowNode for y | This path depends on $@. | test.py:9:12:9:23 | ControlFlowNode for Attribute | a user-provided value |
+| test.py:33:14:33:14 | ControlFlowNode for x | test.py:9:12:9:23 | ControlFlowNode for Attribute | test.py:33:14:33:14 | ControlFlowNode for x | This path depends on $@. | test.py:9:12:9:23 | ControlFlowNode for Attribute | a user-provided value |
+| test.py:49:14:49:14 | ControlFlowNode for y | test.py:9:12:9:23 | ControlFlowNode for Attribute | test.py:49:14:49:14 | ControlFlowNode for y | This path depends on $@. | test.py:9:12:9:23 | ControlFlowNode for Attribute | a user-provided value |
+| test_chaining.py:32:14:32:14 | ControlFlowNode for z | test_chaining.py:9:12:9:23 | ControlFlowNode for Attribute | test_chaining.py:32:14:32:14 | ControlFlowNode for z | This path depends on $@. | test_chaining.py:9:12:9:23 | ControlFlowNode for Attribute | a user-provided value |

python/ql/test/experimental/query-tests/Security-new-dataflow/CWE-022/PathInjection.qlref

@@ -0,0 +1 @@
+experimental/Security-new-dataflow/CWE-022/PathInjection.ql

python/ql/test/experimental/query-tests/Security-new-dataflow/CWE-022/path_injection.py

@@ -0,0 +1,36 @@
+
+import os.path
+
+from flask import Flask, request
+app = Flask(__name__)
+
+@app.route("/path1")
+def path_injection():
+    path = request.args.get('path', '')
+    f = open(os.path.join(os.getcwd(), path))
+
+@app.route("/path2")
+def path_injection():
+    # Normalized, but not checked
+    path = request.args.get('path', '')
+    npath = os.path.normpath(os.path.join(os.getcwd(), path))
+    f = open(npath)  # Path not found
+
+SAFE = "/tmp/scratch_area/"
+
+@app.route("/path3")
+def safe_path():
+    # Normalized, but check doesn't reach open().
+    path = request.args.get('path', '')
+    npath = os.path.normpath(os.path.join(os.getcwd(), path))
+    if npath.startswith(SAFE):
+        pass
+    f = open(npath)  # Path not found
+
+@app.route("/path4")
+def safe_path():
+    # Normalized, and checked properly
+    path = request.args.get('path', '')
+    npath = os.path.normpath(os.path.join(os.getcwd(), path))
+    if npath.startswith(SAFE):
+        f = open(npath)

python/ql/test/experimental/query-tests/Security-new-dataflow/CWE-022/test.py

@@ -0,0 +1,49 @@
+import os.path
+
+from flask import Flask, request
+
+app = Flask(__name__)
+
+
+def source():
+    return request.args.get("path", "")
+
+
+def normalize(x):
+    return os.path.normpath(x)
+
+
+@app.route("/path")
+def simple():
+    x = source()
+    open(x)  # NOT OK
+
+
+@app.route("/path")
+def normalization():
+    x = source()
+    y = normalize(x)
+    open(y)  # NOT OK
+
+
+@app.route("/path")
+def check():
+    x = source()
+    if x.startswith("subfolder/"):
+        open(x)  # NOT OK
+
+
+@app.route("/path")
+def normalize_then_check():
+    x = source()
+    y = normalize(x)
+    if y.startswith("subfolder/"):
+        open(y)  # OK
+
+
+@app.route("/path")
+def check_then_normalize():
+    x = source()
+    if x.startswith("subfolder/"):
+        y = normalize(x)
+        open(y)  # NOT OK

python/ql/test/experimental/query-tests/Security-new-dataflow/CWE-022/test_chaining.py

@@ -0,0 +1,45 @@
+import os.path
+
+from flask import Flask, request
+
+app = Flask(__name__)
+
+
+def source():
+    return request.args.get("path", "")
+
+
+# Wrap normalization, so we can fool the chained configurations.
+# (Call context is lost at cross-over nodes.)
+def normalize(x):
+    return os.path.normpath(x)
+
+
+@app.route("/path")
+def normalize_then_check():
+    x = source()
+    y = normalize(x)  # <--- this call...
+    if y.startswith("subfolder/"):
+        open(y)  # OK
+
+
+@app.route("/path")
+def normalize_check_normalize():
+    x = source()
+    y = normalize(x)  # (...or this call...)
+    if y.startswith("subfolder/"):
+        z = normalize(y)  # <--- ...can jump to here, resulting in FP
+        open(z)  # OK
+
+
+# The problem does not manifest if we simply define an alias
+normpath = os.path.normpath
+
+
+@app.route("/path")
+def normalize_check_normalize_alias():
+    x = source()
+    y = normpath(x)
+    if y.startswith("subfolder/"):
+        z = normpath(y)
+        open(z)  # OK

python/ql/test/experimental/query-tests/Security-new-dataflow/CWE-079/ReflectedXss.expected

@@ -0,0 +1,7 @@
+edges
+| reflected_xss.py:8:18:8:29 | ControlFlowNode for Attribute | reflected_xss.py:9:26:9:53 | ControlFlowNode for BinaryExpr |
+nodes
+| reflected_xss.py:8:18:8:29 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
+| reflected_xss.py:9:26:9:53 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
+#select
+| reflected_xss.py:9:26:9:53 | ControlFlowNode for BinaryExpr | reflected_xss.py:8:18:8:29 | ControlFlowNode for Attribute | reflected_xss.py:9:26:9:53 | ControlFlowNode for BinaryExpr | Cross-site scripting vulnerability due to $@. | reflected_xss.py:8:18:8:29 | ControlFlowNode for Attribute | a user-provided value |

python/ql/test/experimental/query-tests/Security-new-dataflow/CWE-079/ReflectedXss.qlref

@@ -0,0 +1 @@
+experimental/Security-new-dataflow/CWE-079/ReflectedXss.ql

python/ql/test/experimental/query-tests/Security-new-dataflow/CWE-079/reflected_xss.py

@@ -0,0 +1,15 @@
+from flask import Flask, request, make_response, escape
+
+app = Flask(__name__)
+
+
+@app.route("/unsafe")
+def unsafe():
+    first_name = request.args.get("name", "")
+    return make_response("Your name is " + first_name)  # NOT OK
+
+
+@app.route("/safe")
+def safe():
+    first_name = request.args.get("name", "")
+    return make_response("Your name is " + escape(first_name))  # OK