From 25701f203d0ffe98b9ea57bb781e4273b0490a73 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Wed, 21 Aug 2019 14:25:29 +0200 Subject: [PATCH 1/2] C++/C#/Java: Shared TaintTrackingImpl.qll This file is now identical in all languages. Unifying this file led to the following changes: - The documentation spelling fixes and example from the C++ version were copied to the other versions and updated. - The steps through `NonLocalJumpNode` from C# were abstracted into a `globalAdditionalTaintStep` predicate that's empty for C++ and Java. - The `defaultTaintBarrier` predicate from Java is now present but empty on C++ and C#. - The C++ `isAdditionalFlowStep` predicate on `TaintTracking::Configuration` no longer includes `localFlowStep`. That should avoid some unnecessary tuple copying. --- config/identical-files.json | 10 +- .../dataflow/internal/TaintTrackingUtil.qll | 27 ++++- .../tainttracking1/TaintTrackingImpl.qll | 99 +++++++++++-------- .../tainttracking2/TaintTrackingImpl.qll | 99 +++++++++++-------- .../internal/TaintTrackingPrivate.qll | 14 +++ .../tainttracking1/TaintTrackingImpl.qll | 86 ++++++++++++---- .../tainttracking2/TaintTrackingImpl.qll | 86 ++++++++++++---- .../tainttracking3/TaintTrackingImpl.qll | 86 ++++++++++++---- .../tainttracking4/TaintTrackingImpl.qll | 86 ++++++++++++---- .../tainttracking5/TaintTrackingImpl.qll | 86 ++++++++++++---- .../dataflow/internal/TaintTrackingUtil.qll | 8 +- .../tainttracking1/TaintTrackingImpl.qll | 45 +++++++-- .../tainttracking2/TaintTrackingImpl.qll | 45 +++++++-- 13 files changed, 566 insertions(+), 211 deletions(-) diff --git a/config/identical-files.json b/config/identical-files.json index 0f5bd3aafb21..08c3554de5a2 100644 --- a/config/identical-files.json +++ b/config/identical-files.json @@ -25,18 +25,14 @@ "cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImplCommon.qll", "csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImplCommon.qll" ], - "Taint tracking C/C++": [ + "TaintTracking::Configuration Java/C++/C#": [ "cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll", - "cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll" - ], - "Taint tracking C#": [ + "cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll", "csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll", "csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll", "csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking3/TaintTrackingImpl.qll", "csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking4/TaintTrackingImpl.qll", - "csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking5/TaintTrackingImpl.qll" - ], - "Taint tracking Java": [ + "csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking5/TaintTrackingImpl.qll", "java/ql/src/semmle/code/java/dataflow/internal/tainttracking1/TaintTrackingImpl.qll", "java/ql/src/semmle/code/java/dataflow/internal/tainttracking2/TaintTrackingImpl.qll" ], diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll index fec8e44be66f..79af1a82c0f2 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll @@ -18,10 +18,29 @@ private module DataFlow { * Holds if taint propagates from `nodeFrom` to `nodeTo` in exactly one local * (intra-procedural) step. */ -predicate localTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) { - // Taint can flow into using ordinary data flow. - DataFlow::localFlowStep(nodeFrom, nodeTo) - or +predicate localTaintStep(DataFlow::Node src, DataFlow::Node sink) { + DataFlow::localFlowStep(src, sink) or + localAdditionalTaintStep(src, sink) +} + +/** + * Holds if the additional step from `src` to `sink` should be included in all + * global taint flow configurations but not in local taint. + */ +predicate globalAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) { none() } + +/** + * Holds if `node` should be a barrier in all global taint flow configurations + * but not in local taint. + */ +predicate defaultTaintBarrier(DataFlow::Node node) { none() } + +/** + * Holds if taint can flow in one local step from `nodeFrom` to `nodeTo` excluding + * local data flow steps. That is, `nodeFrom` and `nodeTo` are likely to represent + * different objects. + */ +predicate localAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) { // Taint can flow through expressions that alter the value but preserve // more than one bit of it _or_ expressions that follow data through // pointer indirections. diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll index 0e8da75360fc..bd5b40a2df76 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll @@ -1,12 +1,3 @@ -/** - * Provides classes for performing local (intra-procedural) and - * global (inter-procedural) taint-tracking analyses. - * - * We define _taint propagation_ informally to mean that a substantial part of - * the information from the source is preserved at the sink. For example, taint - * propagates from `x` to `x + 100`, but it does not propagate from `x` to `x > - * 100` since we consider a single bit of information to be too little. - */ import TaintTrackingParameter::Public private import TaintTrackingParameter::Private @@ -18,7 +9,7 @@ private import TaintTrackingParameter::Private * * 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 + * necessarily preserve values but are still relevant from a taint tracking * perspective. (For example, string concatenation, where one of the operands * is tainted.) * @@ -30,7 +21,9 @@ private import TaintTrackingParameter::Private * MyAnalysisConfiguration() { this = "MyAnalysisConfiguration" } * // Override `isSource` and `isSink`. * // Optionally override `isSanitizer`. - * // Optionally override `isSanitizerEdge`. + * // Optionally override `isSanitizerIn`. + * // Optionally override `isSanitizerOut`. + * // Optionally override `isSanitizerGuard`. * // Optionally override `isAdditionalTaintStep`. * } * ``` @@ -42,57 +35,79 @@ private import TaintTrackingParameter::Private * exists(MyAnalysisConfiguration cfg | cfg.hasFlow(source, sink)) * ``` * - * Multiple configurations can coexist, but it is unsupported to depend on a - * `TaintTracking::Configuration` or a `DataFlow::Configuration` in the + * 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 `DataFlow{2,3,4}::Configuration`. + * 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 taint source. */ + /** + * 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 taint sink. */ + /** + * 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 taint should not flow into `node`. - */ + /** Holds if the node `node` is a taint sanitizer. */ predicate isSanitizer(DataFlow::Node node) { none() } - /** Holds if data flow from `node1` to `node2` is prohibited. */ - predicate isSanitizerEdge(DataFlow::Node node1, DataFlow::Node node2) { - none() + final override predicate isBarrier(DataFlow::Node node) { + isSanitizer(node) or + defaultTaintBarrier(node) + } + + /** DEPRECATED: override `isSanitizerIn` and `isSanitizerOut` instead. */ + deprecated predicate isSanitizerEdge(DataFlow::Node node1, DataFlow::Node node2) { none() } + + deprecated final override predicate isBarrierEdge(DataFlow::Node node1, DataFlow::Node node2) { + isSanitizerEdge(node1, node2) } + /** Holds if data flow into `node` is prohibited. */ + predicate isSanitizerIn(DataFlow::Node node) { none() } + + final override predicate isBarrierIn(DataFlow::Node node) { isSanitizerIn(node) } + + /** Holds if data flow out of `node` is prohibited. */ + predicate isSanitizerOut(DataFlow::Node node) { none() } + + final override predicate isBarrierOut(DataFlow::Node node) { isSanitizerOut(node) } + + /** Holds if data flow 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 `source` to `target` must be taken into account in the analysis. - * This step will only be followed if `target` is not in the `isSanitizer` - * predicate. + * Holds if the additional taint propagation step from `node1` to `node2` + * must be taken into account in the analysis. */ - predicate isAdditionalTaintStep(DataFlow::Node source, - DataFlow::Node target) - { none() } - - final override - predicate isBarrier(DataFlow::Node node) { isSanitizer(node) } + predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { none() } - /** DEPRECATED: use `isSanitizerEdge` instead. */ - override deprecated - predicate isBarrierEdge(DataFlow::Node node1, DataFlow::Node node2) { - this.isSanitizerEdge(node1, node2) + final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + isAdditionalTaintStep(node1, node2) or + localAdditionalTaintStep(node1, node2) or + globalAdditionalTaintStep(node1, node2) } - final override - predicate isAdditionalFlowStep(DataFlow::Node source, DataFlow::Node target) { - this.isAdditionalTaintStep(source, target) - or - localTaintStep(source, target) + /** + * 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) } } diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll index 0e8da75360fc..bd5b40a2df76 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll @@ -1,12 +1,3 @@ -/** - * Provides classes for performing local (intra-procedural) and - * global (inter-procedural) taint-tracking analyses. - * - * We define _taint propagation_ informally to mean that a substantial part of - * the information from the source is preserved at the sink. For example, taint - * propagates from `x` to `x + 100`, but it does not propagate from `x` to `x > - * 100` since we consider a single bit of information to be too little. - */ import TaintTrackingParameter::Public private import TaintTrackingParameter::Private @@ -18,7 +9,7 @@ private import TaintTrackingParameter::Private * * 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 + * necessarily preserve values but are still relevant from a taint tracking * perspective. (For example, string concatenation, where one of the operands * is tainted.) * @@ -30,7 +21,9 @@ private import TaintTrackingParameter::Private * MyAnalysisConfiguration() { this = "MyAnalysisConfiguration" } * // Override `isSource` and `isSink`. * // Optionally override `isSanitizer`. - * // Optionally override `isSanitizerEdge`. + * // Optionally override `isSanitizerIn`. + * // Optionally override `isSanitizerOut`. + * // Optionally override `isSanitizerGuard`. * // Optionally override `isAdditionalTaintStep`. * } * ``` @@ -42,57 +35,79 @@ private import TaintTrackingParameter::Private * exists(MyAnalysisConfiguration cfg | cfg.hasFlow(source, sink)) * ``` * - * Multiple configurations can coexist, but it is unsupported to depend on a - * `TaintTracking::Configuration` or a `DataFlow::Configuration` in the + * 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 `DataFlow{2,3,4}::Configuration`. + * 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 taint source. */ + /** + * 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 taint sink. */ + /** + * 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 taint should not flow into `node`. - */ + /** Holds if the node `node` is a taint sanitizer. */ predicate isSanitizer(DataFlow::Node node) { none() } - /** Holds if data flow from `node1` to `node2` is prohibited. */ - predicate isSanitizerEdge(DataFlow::Node node1, DataFlow::Node node2) { - none() + final override predicate isBarrier(DataFlow::Node node) { + isSanitizer(node) or + defaultTaintBarrier(node) + } + + /** DEPRECATED: override `isSanitizerIn` and `isSanitizerOut` instead. */ + deprecated predicate isSanitizerEdge(DataFlow::Node node1, DataFlow::Node node2) { none() } + + deprecated final override predicate isBarrierEdge(DataFlow::Node node1, DataFlow::Node node2) { + isSanitizerEdge(node1, node2) } + /** Holds if data flow into `node` is prohibited. */ + predicate isSanitizerIn(DataFlow::Node node) { none() } + + final override predicate isBarrierIn(DataFlow::Node node) { isSanitizerIn(node) } + + /** Holds if data flow out of `node` is prohibited. */ + predicate isSanitizerOut(DataFlow::Node node) { none() } + + final override predicate isBarrierOut(DataFlow::Node node) { isSanitizerOut(node) } + + /** Holds if data flow 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 `source` to `target` must be taken into account in the analysis. - * This step will only be followed if `target` is not in the `isSanitizer` - * predicate. + * Holds if the additional taint propagation step from `node1` to `node2` + * must be taken into account in the analysis. */ - predicate isAdditionalTaintStep(DataFlow::Node source, - DataFlow::Node target) - { none() } - - final override - predicate isBarrier(DataFlow::Node node) { isSanitizer(node) } + predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { none() } - /** DEPRECATED: use `isSanitizerEdge` instead. */ - override deprecated - predicate isBarrierEdge(DataFlow::Node node1, DataFlow::Node node2) { - this.isSanitizerEdge(node1, node2) + final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + isAdditionalTaintStep(node1, node2) or + localAdditionalTaintStep(node1, node2) or + globalAdditionalTaintStep(node1, node2) } - final override - predicate isAdditionalFlowStep(DataFlow::Node source, DataFlow::Node target) { - this.isAdditionalTaintStep(source, target) - or - localTaintStep(source, target) + /** + * 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) } } diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll index 36053eeccd61..56a955663642 100755 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll @@ -10,6 +10,20 @@ private import semmle.code.csharp.frameworks.JsonNET private import cil private import dotnet +/** + * Holds if `node` should be a barrier in all global taint flow configurations + * but not in local taint. + */ +predicate defaultTaintBarrier(DataFlow::Node node) { none() } + +/** + * Holds if the additional step from `src` to `sink` should be included in all + * global taint flow configurations but not in local taint. + */ +predicate globalAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { + succ = pred.(DataFlow::NonLocalJumpNode).getAJumpSuccessor(false) +} + private CIL::DataFlowNode asCilDataFlowNode(DataFlow::Node node) { result = node.asParameter() or result = node.asExpr() diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll index 6bb1e17558db..bd5b40a2df76 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll @@ -2,19 +2,44 @@ import TaintTrackingParameter::Public private import TaintTrackingParameter::Private /** - * A taint tracking configuration. + * 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 dataflow configuration + * 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 + * necessarily preserve values but are still relevant from a taint tracking * perspective. (For example, string concatenation, where one of the operands * is tainted.) * - * Each use of the taint tracking library must define its own unique extension - * of this abstract class. A configuration defines a set of relevant sources - * (`isSource`) and sinks (`isSink`), and may additionally treat intermediate - * nodes as "sanitizers" (`isSanitizer`) as well as add custom taint flow steps - * (`isAdditionalTaintStep()`). + * To create a configuration, extend this class with a subclass whose + * characteristic predicate is a unique singleton string. For example, write + * + * ``` + * 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 + * + * ``` + * 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] @@ -36,23 +61,46 @@ abstract class Configuration extends DataFlow::Configuration { // overridden to provide taint-tracking specific qldoc abstract override predicate isSink(DataFlow::Node sink); - /** Holds if the intermediate node `node` is a taint sanitizer. */ + /** Holds if the node `node` is a taint sanitizer. */ predicate isSanitizer(DataFlow::Node node) { none() } - final override predicate isBarrier(DataFlow::Node node) { isSanitizer(node) } + final override predicate isBarrier(DataFlow::Node node) { + isSanitizer(node) or + defaultTaintBarrier(node) + } + + /** DEPRECATED: override `isSanitizerIn` and `isSanitizerOut` instead. */ + deprecated predicate isSanitizerEdge(DataFlow::Node node1, DataFlow::Node node2) { none() } + + deprecated final override predicate isBarrierEdge(DataFlow::Node node1, DataFlow::Node node2) { + isSanitizerEdge(node1, node2) + } + + /** Holds if data flow into `node` is prohibited. */ + predicate isSanitizerIn(DataFlow::Node node) { none() } + + final override predicate isBarrierIn(DataFlow::Node node) { isSanitizerIn(node) } + + /** Holds if data flow out of `node` is prohibited. */ + predicate isSanitizerOut(DataFlow::Node node) { none() } + + final override predicate isBarrierOut(DataFlow::Node node) { isSanitizerOut(node) } + + /** Holds if data flow 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 `pred` to `succ` + * Holds if the additional taint propagation step from `node1` to `node2` * must be taken into account in the analysis. */ - predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { none() } - - final override predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { - isAdditionalTaintStep(pred, succ) - or - localAdditionalTaintStep(pred, succ) - or - succ = pred.(DataFlow::NonLocalJumpNode).getAJumpSuccessor(false) + predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { none() } + + final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + isAdditionalTaintStep(node1, node2) or + localAdditionalTaintStep(node1, node2) or + globalAdditionalTaintStep(node1, node2) } /** diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll index 6bb1e17558db..bd5b40a2df76 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll @@ -2,19 +2,44 @@ import TaintTrackingParameter::Public private import TaintTrackingParameter::Private /** - * A taint tracking configuration. + * 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 dataflow configuration + * 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 + * necessarily preserve values but are still relevant from a taint tracking * perspective. (For example, string concatenation, where one of the operands * is tainted.) * - * Each use of the taint tracking library must define its own unique extension - * of this abstract class. A configuration defines a set of relevant sources - * (`isSource`) and sinks (`isSink`), and may additionally treat intermediate - * nodes as "sanitizers" (`isSanitizer`) as well as add custom taint flow steps - * (`isAdditionalTaintStep()`). + * To create a configuration, extend this class with a subclass whose + * characteristic predicate is a unique singleton string. For example, write + * + * ``` + * 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 + * + * ``` + * 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] @@ -36,23 +61,46 @@ abstract class Configuration extends DataFlow::Configuration { // overridden to provide taint-tracking specific qldoc abstract override predicate isSink(DataFlow::Node sink); - /** Holds if the intermediate node `node` is a taint sanitizer. */ + /** Holds if the node `node` is a taint sanitizer. */ predicate isSanitizer(DataFlow::Node node) { none() } - final override predicate isBarrier(DataFlow::Node node) { isSanitizer(node) } + final override predicate isBarrier(DataFlow::Node node) { + isSanitizer(node) or + defaultTaintBarrier(node) + } + + /** DEPRECATED: override `isSanitizerIn` and `isSanitizerOut` instead. */ + deprecated predicate isSanitizerEdge(DataFlow::Node node1, DataFlow::Node node2) { none() } + + deprecated final override predicate isBarrierEdge(DataFlow::Node node1, DataFlow::Node node2) { + isSanitizerEdge(node1, node2) + } + + /** Holds if data flow into `node` is prohibited. */ + predicate isSanitizerIn(DataFlow::Node node) { none() } + + final override predicate isBarrierIn(DataFlow::Node node) { isSanitizerIn(node) } + + /** Holds if data flow out of `node` is prohibited. */ + predicate isSanitizerOut(DataFlow::Node node) { none() } + + final override predicate isBarrierOut(DataFlow::Node node) { isSanitizerOut(node) } + + /** Holds if data flow 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 `pred` to `succ` + * Holds if the additional taint propagation step from `node1` to `node2` * must be taken into account in the analysis. */ - predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { none() } - - final override predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { - isAdditionalTaintStep(pred, succ) - or - localAdditionalTaintStep(pred, succ) - or - succ = pred.(DataFlow::NonLocalJumpNode).getAJumpSuccessor(false) + predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { none() } + + final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + isAdditionalTaintStep(node1, node2) or + localAdditionalTaintStep(node1, node2) or + globalAdditionalTaintStep(node1, node2) } /** diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking3/TaintTrackingImpl.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking3/TaintTrackingImpl.qll index 6bb1e17558db..bd5b40a2df76 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking3/TaintTrackingImpl.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking3/TaintTrackingImpl.qll @@ -2,19 +2,44 @@ import TaintTrackingParameter::Public private import TaintTrackingParameter::Private /** - * A taint tracking configuration. + * 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 dataflow configuration + * 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 + * necessarily preserve values but are still relevant from a taint tracking * perspective. (For example, string concatenation, where one of the operands * is tainted.) * - * Each use of the taint tracking library must define its own unique extension - * of this abstract class. A configuration defines a set of relevant sources - * (`isSource`) and sinks (`isSink`), and may additionally treat intermediate - * nodes as "sanitizers" (`isSanitizer`) as well as add custom taint flow steps - * (`isAdditionalTaintStep()`). + * To create a configuration, extend this class with a subclass whose + * characteristic predicate is a unique singleton string. For example, write + * + * ``` + * 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 + * + * ``` + * 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] @@ -36,23 +61,46 @@ abstract class Configuration extends DataFlow::Configuration { // overridden to provide taint-tracking specific qldoc abstract override predicate isSink(DataFlow::Node sink); - /** Holds if the intermediate node `node` is a taint sanitizer. */ + /** Holds if the node `node` is a taint sanitizer. */ predicate isSanitizer(DataFlow::Node node) { none() } - final override predicate isBarrier(DataFlow::Node node) { isSanitizer(node) } + final override predicate isBarrier(DataFlow::Node node) { + isSanitizer(node) or + defaultTaintBarrier(node) + } + + /** DEPRECATED: override `isSanitizerIn` and `isSanitizerOut` instead. */ + deprecated predicate isSanitizerEdge(DataFlow::Node node1, DataFlow::Node node2) { none() } + + deprecated final override predicate isBarrierEdge(DataFlow::Node node1, DataFlow::Node node2) { + isSanitizerEdge(node1, node2) + } + + /** Holds if data flow into `node` is prohibited. */ + predicate isSanitizerIn(DataFlow::Node node) { none() } + + final override predicate isBarrierIn(DataFlow::Node node) { isSanitizerIn(node) } + + /** Holds if data flow out of `node` is prohibited. */ + predicate isSanitizerOut(DataFlow::Node node) { none() } + + final override predicate isBarrierOut(DataFlow::Node node) { isSanitizerOut(node) } + + /** Holds if data flow 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 `pred` to `succ` + * Holds if the additional taint propagation step from `node1` to `node2` * must be taken into account in the analysis. */ - predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { none() } - - final override predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { - isAdditionalTaintStep(pred, succ) - or - localAdditionalTaintStep(pred, succ) - or - succ = pred.(DataFlow::NonLocalJumpNode).getAJumpSuccessor(false) + predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { none() } + + final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + isAdditionalTaintStep(node1, node2) or + localAdditionalTaintStep(node1, node2) or + globalAdditionalTaintStep(node1, node2) } /** diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking4/TaintTrackingImpl.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking4/TaintTrackingImpl.qll index 6bb1e17558db..bd5b40a2df76 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking4/TaintTrackingImpl.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking4/TaintTrackingImpl.qll @@ -2,19 +2,44 @@ import TaintTrackingParameter::Public private import TaintTrackingParameter::Private /** - * A taint tracking configuration. + * 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 dataflow configuration + * 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 + * necessarily preserve values but are still relevant from a taint tracking * perspective. (For example, string concatenation, where one of the operands * is tainted.) * - * Each use of the taint tracking library must define its own unique extension - * of this abstract class. A configuration defines a set of relevant sources - * (`isSource`) and sinks (`isSink`), and may additionally treat intermediate - * nodes as "sanitizers" (`isSanitizer`) as well as add custom taint flow steps - * (`isAdditionalTaintStep()`). + * To create a configuration, extend this class with a subclass whose + * characteristic predicate is a unique singleton string. For example, write + * + * ``` + * 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 + * + * ``` + * 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] @@ -36,23 +61,46 @@ abstract class Configuration extends DataFlow::Configuration { // overridden to provide taint-tracking specific qldoc abstract override predicate isSink(DataFlow::Node sink); - /** Holds if the intermediate node `node` is a taint sanitizer. */ + /** Holds if the node `node` is a taint sanitizer. */ predicate isSanitizer(DataFlow::Node node) { none() } - final override predicate isBarrier(DataFlow::Node node) { isSanitizer(node) } + final override predicate isBarrier(DataFlow::Node node) { + isSanitizer(node) or + defaultTaintBarrier(node) + } + + /** DEPRECATED: override `isSanitizerIn` and `isSanitizerOut` instead. */ + deprecated predicate isSanitizerEdge(DataFlow::Node node1, DataFlow::Node node2) { none() } + + deprecated final override predicate isBarrierEdge(DataFlow::Node node1, DataFlow::Node node2) { + isSanitizerEdge(node1, node2) + } + + /** Holds if data flow into `node` is prohibited. */ + predicate isSanitizerIn(DataFlow::Node node) { none() } + + final override predicate isBarrierIn(DataFlow::Node node) { isSanitizerIn(node) } + + /** Holds if data flow out of `node` is prohibited. */ + predicate isSanitizerOut(DataFlow::Node node) { none() } + + final override predicate isBarrierOut(DataFlow::Node node) { isSanitizerOut(node) } + + /** Holds if data flow 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 `pred` to `succ` + * Holds if the additional taint propagation step from `node1` to `node2` * must be taken into account in the analysis. */ - predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { none() } - - final override predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { - isAdditionalTaintStep(pred, succ) - or - localAdditionalTaintStep(pred, succ) - or - succ = pred.(DataFlow::NonLocalJumpNode).getAJumpSuccessor(false) + predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { none() } + + final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + isAdditionalTaintStep(node1, node2) or + localAdditionalTaintStep(node1, node2) or + globalAdditionalTaintStep(node1, node2) } /** diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking5/TaintTrackingImpl.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking5/TaintTrackingImpl.qll index 6bb1e17558db..bd5b40a2df76 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking5/TaintTrackingImpl.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking5/TaintTrackingImpl.qll @@ -2,19 +2,44 @@ import TaintTrackingParameter::Public private import TaintTrackingParameter::Private /** - * A taint tracking configuration. + * 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 dataflow configuration + * 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 + * necessarily preserve values but are still relevant from a taint tracking * perspective. (For example, string concatenation, where one of the operands * is tainted.) * - * Each use of the taint tracking library must define its own unique extension - * of this abstract class. A configuration defines a set of relevant sources - * (`isSource`) and sinks (`isSink`), and may additionally treat intermediate - * nodes as "sanitizers" (`isSanitizer`) as well as add custom taint flow steps - * (`isAdditionalTaintStep()`). + * To create a configuration, extend this class with a subclass whose + * characteristic predicate is a unique singleton string. For example, write + * + * ``` + * 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 + * + * ``` + * 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] @@ -36,23 +61,46 @@ abstract class Configuration extends DataFlow::Configuration { // overridden to provide taint-tracking specific qldoc abstract override predicate isSink(DataFlow::Node sink); - /** Holds if the intermediate node `node` is a taint sanitizer. */ + /** Holds if the node `node` is a taint sanitizer. */ predicate isSanitizer(DataFlow::Node node) { none() } - final override predicate isBarrier(DataFlow::Node node) { isSanitizer(node) } + final override predicate isBarrier(DataFlow::Node node) { + isSanitizer(node) or + defaultTaintBarrier(node) + } + + /** DEPRECATED: override `isSanitizerIn` and `isSanitizerOut` instead. */ + deprecated predicate isSanitizerEdge(DataFlow::Node node1, DataFlow::Node node2) { none() } + + deprecated final override predicate isBarrierEdge(DataFlow::Node node1, DataFlow::Node node2) { + isSanitizerEdge(node1, node2) + } + + /** Holds if data flow into `node` is prohibited. */ + predicate isSanitizerIn(DataFlow::Node node) { none() } + + final override predicate isBarrierIn(DataFlow::Node node) { isSanitizerIn(node) } + + /** Holds if data flow out of `node` is prohibited. */ + predicate isSanitizerOut(DataFlow::Node node) { none() } + + final override predicate isBarrierOut(DataFlow::Node node) { isSanitizerOut(node) } + + /** Holds if data flow 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 `pred` to `succ` + * Holds if the additional taint propagation step from `node1` to `node2` * must be taken into account in the analysis. */ - predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { none() } - - final override predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { - isAdditionalTaintStep(pred, succ) - or - localAdditionalTaintStep(pred, succ) - or - succ = pred.(DataFlow::NonLocalJumpNode).getAJumpSuccessor(false) + predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { none() } + + final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + isAdditionalTaintStep(node1, node2) or + localAdditionalTaintStep(node1, node2) or + globalAdditionalTaintStep(node1, node2) } /** diff --git a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll index 28ed6c5e86c1..116251330ebf 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll @@ -40,6 +40,12 @@ predicate localAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) { ) } +/** + * Holds if the additional step from `src` to `sink` should be included in all + * global taint flow configurations but not in local taint. + */ +predicate globalAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) { none() } + /** * Holds if `node` should be a barrier in all global taint flow configurations * but not in local taint. @@ -555,8 +561,8 @@ class ObjectOutputStreamVar extends LocalVariableDecl { result.getMethod().hasName("writeObject") } } - private import StringBuilderVarModule + module StringBuilderVarModule { /** * A local variable that is initialized to a `StringBuilder` diff --git a/java/ql/src/semmle/code/java/dataflow/internal/tainttracking1/TaintTrackingImpl.qll b/java/ql/src/semmle/code/java/dataflow/internal/tainttracking1/TaintTrackingImpl.qll index 985768177d47..bd5b40a2df76 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/tainttracking1/TaintTrackingImpl.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/tainttracking1/TaintTrackingImpl.qll @@ -2,19 +2,44 @@ import TaintTrackingParameter::Public private import TaintTrackingParameter::Private /** - * A taint tracking configuration. + * 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 dataflow configuration + * 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 + * necessarily preserve values but are still relevant from a taint tracking * perspective. (For example, string concatenation, where one of the operands * is tainted.) * - * Each use of the taint tracking library must define its own unique extension - * of this abstract class. A configuration defines a set of relevant sources - * (`isSource`) and sinks (`isSink`), and may additionally treat intermediate - * nodes as "sanitizers" (`isSanitizer`) as well as add custom taint flow steps - * (`isAdditionalTaintStep()`). + * To create a configuration, extend this class with a subclass whose + * characteristic predicate is a unique singleton string. For example, write + * + * ``` + * 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 + * + * ``` + * 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] @@ -74,7 +99,8 @@ abstract class Configuration extends DataFlow::Configuration { final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { isAdditionalTaintStep(node1, node2) or - localAdditionalTaintStep(node1, node2) + localAdditionalTaintStep(node1, node2) or + globalAdditionalTaintStep(node1, node2) } /** @@ -85,4 +111,3 @@ abstract class Configuration extends DataFlow::Configuration { super.hasFlow(source, sink) } } - diff --git a/java/ql/src/semmle/code/java/dataflow/internal/tainttracking2/TaintTrackingImpl.qll b/java/ql/src/semmle/code/java/dataflow/internal/tainttracking2/TaintTrackingImpl.qll index 985768177d47..bd5b40a2df76 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/tainttracking2/TaintTrackingImpl.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/tainttracking2/TaintTrackingImpl.qll @@ -2,19 +2,44 @@ import TaintTrackingParameter::Public private import TaintTrackingParameter::Private /** - * A taint tracking configuration. + * 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 dataflow configuration + * 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 + * necessarily preserve values but are still relevant from a taint tracking * perspective. (For example, string concatenation, where one of the operands * is tainted.) * - * Each use of the taint tracking library must define its own unique extension - * of this abstract class. A configuration defines a set of relevant sources - * (`isSource`) and sinks (`isSink`), and may additionally treat intermediate - * nodes as "sanitizers" (`isSanitizer`) as well as add custom taint flow steps - * (`isAdditionalTaintStep()`). + * To create a configuration, extend this class with a subclass whose + * characteristic predicate is a unique singleton string. For example, write + * + * ``` + * 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 + * + * ``` + * 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] @@ -74,7 +99,8 @@ abstract class Configuration extends DataFlow::Configuration { final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { isAdditionalTaintStep(node1, node2) or - localAdditionalTaintStep(node1, node2) + localAdditionalTaintStep(node1, node2) or + globalAdditionalTaintStep(node1, node2) } /** @@ -85,4 +111,3 @@ abstract class Configuration extends DataFlow::Configuration { super.hasFlow(source, sink) } } - From ad9ee54b659e2ec2fa95b6fcff8e83c1be6903a5 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Thu, 22 Aug 2019 11:14:06 +0200 Subject: [PATCH 2/2] C++/C#/Java: defaultAdditionalTaintStep --- .../semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll | 6 ++++-- .../dataflow/internal/tainttracking1/TaintTrackingImpl.qll | 3 +-- .../dataflow/internal/tainttracking2/TaintTrackingImpl.qll | 3 +-- .../code/csharp/dataflow/internal/TaintTrackingPrivate.qll | 6 ++++-- .../dataflow/internal/tainttracking1/TaintTrackingImpl.qll | 3 +-- .../dataflow/internal/tainttracking2/TaintTrackingImpl.qll | 3 +-- .../dataflow/internal/tainttracking3/TaintTrackingImpl.qll | 3 +-- .../dataflow/internal/tainttracking4/TaintTrackingImpl.qll | 3 +-- .../dataflow/internal/tainttracking5/TaintTrackingImpl.qll | 3 +-- .../code/java/dataflow/internal/TaintTrackingUtil.qll | 6 ++++-- .../dataflow/internal/tainttracking1/TaintTrackingImpl.qll | 3 +-- .../dataflow/internal/tainttracking2/TaintTrackingImpl.qll | 3 +-- 12 files changed, 21 insertions(+), 24 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll index 79af1a82c0f2..6eb8b7d3f099 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll @@ -25,9 +25,11 @@ predicate localTaintStep(DataFlow::Node src, DataFlow::Node sink) { /** * Holds if the additional step from `src` to `sink` should be included in all - * global taint flow configurations but not in local taint. + * global taint flow configurations. */ -predicate globalAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) { none() } +predicate defaultAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) { + localAdditionalTaintStep(src, sink) +} /** * Holds if `node` should be a barrier in all global taint flow configurations diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll index bd5b40a2df76..c05de75bfab0 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll @@ -99,8 +99,7 @@ abstract class Configuration extends DataFlow::Configuration { final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { isAdditionalTaintStep(node1, node2) or - localAdditionalTaintStep(node1, node2) or - globalAdditionalTaintStep(node1, node2) + defaultAdditionalTaintStep(node1, node2) } /** diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll index bd5b40a2df76..c05de75bfab0 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll @@ -99,8 +99,7 @@ abstract class Configuration extends DataFlow::Configuration { final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { isAdditionalTaintStep(node1, node2) or - localAdditionalTaintStep(node1, node2) or - globalAdditionalTaintStep(node1, node2) + defaultAdditionalTaintStep(node1, node2) } /** diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll index 56a955663642..4d0bc59d2a21 100755 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll @@ -18,9 +18,11 @@ predicate defaultTaintBarrier(DataFlow::Node node) { none() } /** * Holds if the additional step from `src` to `sink` should be included in all - * global taint flow configurations but not in local taint. + * global taint flow configurations. */ -predicate globalAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { +predicate defaultAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { + localAdditionalTaintStep(pred, succ) + or succ = pred.(DataFlow::NonLocalJumpNode).getAJumpSuccessor(false) } diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll index bd5b40a2df76..c05de75bfab0 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll @@ -99,8 +99,7 @@ abstract class Configuration extends DataFlow::Configuration { final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { isAdditionalTaintStep(node1, node2) or - localAdditionalTaintStep(node1, node2) or - globalAdditionalTaintStep(node1, node2) + defaultAdditionalTaintStep(node1, node2) } /** diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll index bd5b40a2df76..c05de75bfab0 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll @@ -99,8 +99,7 @@ abstract class Configuration extends DataFlow::Configuration { final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { isAdditionalTaintStep(node1, node2) or - localAdditionalTaintStep(node1, node2) or - globalAdditionalTaintStep(node1, node2) + defaultAdditionalTaintStep(node1, node2) } /** diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking3/TaintTrackingImpl.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking3/TaintTrackingImpl.qll index bd5b40a2df76..c05de75bfab0 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking3/TaintTrackingImpl.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking3/TaintTrackingImpl.qll @@ -99,8 +99,7 @@ abstract class Configuration extends DataFlow::Configuration { final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { isAdditionalTaintStep(node1, node2) or - localAdditionalTaintStep(node1, node2) or - globalAdditionalTaintStep(node1, node2) + defaultAdditionalTaintStep(node1, node2) } /** diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking4/TaintTrackingImpl.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking4/TaintTrackingImpl.qll index bd5b40a2df76..c05de75bfab0 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking4/TaintTrackingImpl.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking4/TaintTrackingImpl.qll @@ -99,8 +99,7 @@ abstract class Configuration extends DataFlow::Configuration { final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { isAdditionalTaintStep(node1, node2) or - localAdditionalTaintStep(node1, node2) or - globalAdditionalTaintStep(node1, node2) + defaultAdditionalTaintStep(node1, node2) } /** diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking5/TaintTrackingImpl.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking5/TaintTrackingImpl.qll index bd5b40a2df76..c05de75bfab0 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking5/TaintTrackingImpl.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking5/TaintTrackingImpl.qll @@ -99,8 +99,7 @@ abstract class Configuration extends DataFlow::Configuration { final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { isAdditionalTaintStep(node1, node2) or - localAdditionalTaintStep(node1, node2) or - globalAdditionalTaintStep(node1, node2) + defaultAdditionalTaintStep(node1, node2) } /** diff --git a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll index 116251330ebf..0e7751152966 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll @@ -42,9 +42,11 @@ predicate localAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) { /** * Holds if the additional step from `src` to `sink` should be included in all - * global taint flow configurations but not in local taint. + * global taint flow configurations. */ -predicate globalAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) { none() } +predicate defaultAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) { + localAdditionalTaintStep(src, sink) +} /** * Holds if `node` should be a barrier in all global taint flow configurations diff --git a/java/ql/src/semmle/code/java/dataflow/internal/tainttracking1/TaintTrackingImpl.qll b/java/ql/src/semmle/code/java/dataflow/internal/tainttracking1/TaintTrackingImpl.qll index bd5b40a2df76..c05de75bfab0 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/tainttracking1/TaintTrackingImpl.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/tainttracking1/TaintTrackingImpl.qll @@ -99,8 +99,7 @@ abstract class Configuration extends DataFlow::Configuration { final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { isAdditionalTaintStep(node1, node2) or - localAdditionalTaintStep(node1, node2) or - globalAdditionalTaintStep(node1, node2) + defaultAdditionalTaintStep(node1, node2) } /** diff --git a/java/ql/src/semmle/code/java/dataflow/internal/tainttracking2/TaintTrackingImpl.qll b/java/ql/src/semmle/code/java/dataflow/internal/tainttracking2/TaintTrackingImpl.qll index bd5b40a2df76..c05de75bfab0 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/tainttracking2/TaintTrackingImpl.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/tainttracking2/TaintTrackingImpl.qll @@ -99,8 +99,7 @@ abstract class Configuration extends DataFlow::Configuration { final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { isAdditionalTaintStep(node1, node2) or - localAdditionalTaintStep(node1, node2) or - globalAdditionalTaintStep(node1, node2) + defaultAdditionalTaintStep(node1, node2) } /**