From 4550175b16928eceb39aa42a9b024bd03c3bd8cf Mon Sep 17 00:00:00 2001
From: Anders Schack-Mulligen <schack@semmle.com>
Date: Fri, 9 Aug 2019 11:12:00 +0200
Subject: [PATCH 1/4] Java/C++/C#: Add support for BarrierGuards.

---
 .../code/cpp/dataflow/internal/DataFlowImpl.qll |  8 ++++++++
 .../cpp/dataflow/internal/DataFlowImpl2.qll     |  8 ++++++++
 .../cpp/dataflow/internal/DataFlowImpl3.qll     |  8 ++++++++
 .../cpp/dataflow/internal/DataFlowImpl4.qll     |  8 ++++++++
 .../code/cpp/dataflow/internal/DataFlowUtil.qll | 11 +++++++++++
 .../cpp/ir/dataflow/internal/DataFlowImpl.qll   |  8 ++++++++
 .../cpp/ir/dataflow/internal/DataFlowImpl2.qll  |  8 ++++++++
 .../cpp/ir/dataflow/internal/DataFlowImpl3.qll  |  8 ++++++++
 .../cpp/ir/dataflow/internal/DataFlowImpl4.qll  |  8 ++++++++
 .../cpp/ir/dataflow/internal/DataFlowUtil.qll   | 12 ++++++++++++
 .../csharp/dataflow/internal/DataFlowImpl.qll   |  8 ++++++++
 .../csharp/dataflow/internal/DataFlowImpl2.qll  |  8 ++++++++
 .../csharp/dataflow/internal/DataFlowImpl3.qll  |  8 ++++++++
 .../csharp/dataflow/internal/DataFlowImpl4.qll  |  8 ++++++++
 .../csharp/dataflow/internal/DataFlowImpl5.qll  |  8 ++++++++
 .../csharp/dataflow/internal/DataFlowPublic.qll | 12 ++++++++++++
 java/ql/src/Security/CWE/CWE-022/TaintedPath.ql | 15 +++++++++++++++
 .../semmle/code/java/dataflow/TaintTracking.qll | 10 ++++++++++
 .../java/dataflow/internal/DataFlowImpl.qll     |  8 ++++++++
 .../java/dataflow/internal/DataFlowImpl2.qll    |  8 ++++++++
 .../java/dataflow/internal/DataFlowImpl3.qll    |  8 ++++++++
 .../java/dataflow/internal/DataFlowImpl4.qll    |  8 ++++++++
 .../java/dataflow/internal/DataFlowImpl5.qll    |  8 ++++++++
 .../java/dataflow/internal/DataFlowUtil.qll     | 17 +++++++++++++++++
 24 files changed, 221 insertions(+)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll
index 1d51a88d13de..094a2530c319 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll
@@ -75,6 +75,9 @@ abstract class Configuration extends string {
   /** Holds if data flow out of `node` is prohibited. */
   predicate isBarrierOut(Node node) { none() }
 
+  /** Holds if data flow through nodes guarded by `guard` is prohibited. */
+  predicate isBarrierGuard(BarrierGuard guard) { none() }
+
   /**
    * Holds if the additional flow step from `node1` to `node2` must be taken
    * into account in the analysis.
@@ -136,6 +139,11 @@ private predicate fullBarrier(Node node, Configuration config) {
   or
   config.isBarrierOut(node) and
   not config.isSink(node)
+  or
+  exists(BarrierGuard g |
+    config.isBarrierGuard(g) and
+    node = g.getAGuardedNode()
+  )
 }
 
 private class AdditionalFlowStepSource extends Node {
diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll
index 1d51a88d13de..094a2530c319 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll
@@ -75,6 +75,9 @@ abstract class Configuration extends string {
   /** Holds if data flow out of `node` is prohibited. */
   predicate isBarrierOut(Node node) { none() }
 
+  /** Holds if data flow through nodes guarded by `guard` is prohibited. */
+  predicate isBarrierGuard(BarrierGuard guard) { none() }
+
   /**
    * Holds if the additional flow step from `node1` to `node2` must be taken
    * into account in the analysis.
@@ -136,6 +139,11 @@ private predicate fullBarrier(Node node, Configuration config) {
   or
   config.isBarrierOut(node) and
   not config.isSink(node)
+  or
+  exists(BarrierGuard g |
+    config.isBarrierGuard(g) and
+    node = g.getAGuardedNode()
+  )
 }
 
 private class AdditionalFlowStepSource extends Node {
diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll
index 1d51a88d13de..094a2530c319 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll
@@ -75,6 +75,9 @@ abstract class Configuration extends string {
   /** Holds if data flow out of `node` is prohibited. */
   predicate isBarrierOut(Node node) { none() }
 
+  /** Holds if data flow through nodes guarded by `guard` is prohibited. */
+  predicate isBarrierGuard(BarrierGuard guard) { none() }
+
   /**
    * Holds if the additional flow step from `node1` to `node2` must be taken
    * into account in the analysis.
@@ -136,6 +139,11 @@ private predicate fullBarrier(Node node, Configuration config) {
   or
   config.isBarrierOut(node) and
   not config.isSink(node)
+  or
+  exists(BarrierGuard g |
+    config.isBarrierGuard(g) and
+    node = g.getAGuardedNode()
+  )
 }
 
 private class AdditionalFlowStepSource extends Node {
diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll
index 1d51a88d13de..094a2530c319 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll
@@ -75,6 +75,9 @@ abstract class Configuration extends string {
   /** Holds if data flow out of `node` is prohibited. */
   predicate isBarrierOut(Node node) { none() }
 
+  /** Holds if data flow through nodes guarded by `guard` is prohibited. */
+  predicate isBarrierGuard(BarrierGuard guard) { none() }
+
   /**
    * Holds if the additional flow step from `node1` to `node2` must be taken
    * into account in the analysis.
@@ -136,6 +139,11 @@ private predicate fullBarrier(Node node, Configuration config) {
   or
   config.isBarrierOut(node) and
   not config.isSink(node)
+  or
+  exists(BarrierGuard g |
+    config.isBarrierGuard(g) and
+    node = g.getAGuardedNode()
+  )
 }
 
 private class AdditionalFlowStepSource extends Node {
diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index e9b82a54e431..fe5518828ce4 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -331,3 +331,14 @@ VariableAccess getAnAccessToAssignedVariable(Expr assign) {
     result = var.getAnAccess()
   )
 }
+
+/** A guard that validates some expression. */
+class BarrierGuard extends Expr {
+  /** Holds if this guard validates `e` upon evaluating to `branch`. */
+  abstract predicate checks(Expr e, boolean branch);
+
+  /** Gets a node guarded by this. */
+  final Node getAGuardedNode() {
+    none() // stub
+  }
+}
diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll
index 1d51a88d13de..094a2530c319 100644
--- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll
+++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll
@@ -75,6 +75,9 @@ abstract class Configuration extends string {
   /** Holds if data flow out of `node` is prohibited. */
   predicate isBarrierOut(Node node) { none() }
 
+  /** Holds if data flow through nodes guarded by `guard` is prohibited. */
+  predicate isBarrierGuard(BarrierGuard guard) { none() }
+
   /**
    * Holds if the additional flow step from `node1` to `node2` must be taken
    * into account in the analysis.
@@ -136,6 +139,11 @@ private predicate fullBarrier(Node node, Configuration config) {
   or
   config.isBarrierOut(node) and
   not config.isSink(node)
+  or
+  exists(BarrierGuard g |
+    config.isBarrierGuard(g) and
+    node = g.getAGuardedNode()
+  )
 }
 
 private class AdditionalFlowStepSource extends Node {
diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll
index 1d51a88d13de..094a2530c319 100644
--- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll
+++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll
@@ -75,6 +75,9 @@ abstract class Configuration extends string {
   /** Holds if data flow out of `node` is prohibited. */
   predicate isBarrierOut(Node node) { none() }
 
+  /** Holds if data flow through nodes guarded by `guard` is prohibited. */
+  predicate isBarrierGuard(BarrierGuard guard) { none() }
+
   /**
    * Holds if the additional flow step from `node1` to `node2` must be taken
    * into account in the analysis.
@@ -136,6 +139,11 @@ private predicate fullBarrier(Node node, Configuration config) {
   or
   config.isBarrierOut(node) and
   not config.isSink(node)
+  or
+  exists(BarrierGuard g |
+    config.isBarrierGuard(g) and
+    node = g.getAGuardedNode()
+  )
 }
 
 private class AdditionalFlowStepSource extends Node {
diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll
index 1d51a88d13de..094a2530c319 100644
--- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll
+++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll
@@ -75,6 +75,9 @@ abstract class Configuration extends string {
   /** Holds if data flow out of `node` is prohibited. */
   predicate isBarrierOut(Node node) { none() }
 
+  /** Holds if data flow through nodes guarded by `guard` is prohibited. */
+  predicate isBarrierGuard(BarrierGuard guard) { none() }
+
   /**
    * Holds if the additional flow step from `node1` to `node2` must be taken
    * into account in the analysis.
@@ -136,6 +139,11 @@ private predicate fullBarrier(Node node, Configuration config) {
   or
   config.isBarrierOut(node) and
   not config.isSink(node)
+  or
+  exists(BarrierGuard g |
+    config.isBarrierGuard(g) and
+    node = g.getAGuardedNode()
+  )
 }
 
 private class AdditionalFlowStepSource extends Node {
diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll
index 1d51a88d13de..094a2530c319 100644
--- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll
+++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll
@@ -75,6 +75,9 @@ abstract class Configuration extends string {
   /** Holds if data flow out of `node` is prohibited. */
   predicate isBarrierOut(Node node) { none() }
 
+  /** Holds if data flow through nodes guarded by `guard` is prohibited. */
+  predicate isBarrierGuard(BarrierGuard guard) { none() }
+
   /**
    * Holds if the additional flow step from `node1` to `node2` must be taken
    * into account in the analysis.
@@ -136,6 +139,11 @@ private predicate fullBarrier(Node node, Configuration config) {
   or
   config.isBarrierOut(node) and
   not config.isSink(node)
+  or
+  exists(BarrierGuard g |
+    config.isBarrierGuard(g) and
+    node = g.getAGuardedNode()
+  )
 }
 
 private class AdditionalFlowStepSource extends Node {
diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
index d2101d09ab4d..965d4c14b249 100644
--- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
@@ -4,6 +4,7 @@
 
 private import cpp
 private import semmle.code.cpp.ir.IR
+private import semmle.code.cpp.controlflow.IRGuards
 
 /**
  * A node in a data flow graph.
@@ -166,3 +167,14 @@ predicate localFlowStep(Node nodeFrom, Node nodeTo) {
  * (intra-procedural) steps.
  */
 predicate localFlow(Node source, Node sink) { localFlowStep*(source, sink) }
+
+/** A guard that validates some expression. */
+class BarrierGuard extends IRGuardCondition {
+  /** Holds if this guard validates `e` upon evaluating to `b`. */
+  abstract predicate checks(Instruction e, boolean b);
+
+  /** Gets a node guarded by this. */
+  final Node getAGuardedNode() {
+    none() // stub
+  }
+}
diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl.qll
index 1d51a88d13de..094a2530c319 100644
--- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl.qll
+++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl.qll
@@ -75,6 +75,9 @@ abstract class Configuration extends string {
   /** Holds if data flow out of `node` is prohibited. */
   predicate isBarrierOut(Node node) { none() }
 
+  /** Holds if data flow through nodes guarded by `guard` is prohibited. */
+  predicate isBarrierGuard(BarrierGuard guard) { none() }
+
   /**
    * Holds if the additional flow step from `node1` to `node2` must be taken
    * into account in the analysis.
@@ -136,6 +139,11 @@ private predicate fullBarrier(Node node, Configuration config) {
   or
   config.isBarrierOut(node) and
   not config.isSink(node)
+  or
+  exists(BarrierGuard g |
+    config.isBarrierGuard(g) and
+    node = g.getAGuardedNode()
+  )
 }
 
 private class AdditionalFlowStepSource extends Node {
diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll
index 1d51a88d13de..094a2530c319 100644
--- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll
+++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll
@@ -75,6 +75,9 @@ abstract class Configuration extends string {
   /** Holds if data flow out of `node` is prohibited. */
   predicate isBarrierOut(Node node) { none() }
 
+  /** Holds if data flow through nodes guarded by `guard` is prohibited. */
+  predicate isBarrierGuard(BarrierGuard guard) { none() }
+
   /**
    * Holds if the additional flow step from `node1` to `node2` must be taken
    * into account in the analysis.
@@ -136,6 +139,11 @@ private predicate fullBarrier(Node node, Configuration config) {
   or
   config.isBarrierOut(node) and
   not config.isSink(node)
+  or
+  exists(BarrierGuard g |
+    config.isBarrierGuard(g) and
+    node = g.getAGuardedNode()
+  )
 }
 
 private class AdditionalFlowStepSource extends Node {
diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll
index 1d51a88d13de..094a2530c319 100644
--- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll
+++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll
@@ -75,6 +75,9 @@ abstract class Configuration extends string {
   /** Holds if data flow out of `node` is prohibited. */
   predicate isBarrierOut(Node node) { none() }
 
+  /** Holds if data flow through nodes guarded by `guard` is prohibited. */
+  predicate isBarrierGuard(BarrierGuard guard) { none() }
+
   /**
    * Holds if the additional flow step from `node1` to `node2` must be taken
    * into account in the analysis.
@@ -136,6 +139,11 @@ private predicate fullBarrier(Node node, Configuration config) {
   or
   config.isBarrierOut(node) and
   not config.isSink(node)
+  or
+  exists(BarrierGuard g |
+    config.isBarrierGuard(g) and
+    node = g.getAGuardedNode()
+  )
 }
 
 private class AdditionalFlowStepSource extends Node {
diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll
index 1d51a88d13de..094a2530c319 100644
--- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll
+++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll
@@ -75,6 +75,9 @@ abstract class Configuration extends string {
   /** Holds if data flow out of `node` is prohibited. */
   predicate isBarrierOut(Node node) { none() }
 
+  /** Holds if data flow through nodes guarded by `guard` is prohibited. */
+  predicate isBarrierGuard(BarrierGuard guard) { none() }
+
   /**
    * Holds if the additional flow step from `node1` to `node2` must be taken
    * into account in the analysis.
@@ -136,6 +139,11 @@ private predicate fullBarrier(Node node, Configuration config) {
   or
   config.isBarrierOut(node) and
   not config.isSink(node)
+  or
+  exists(BarrierGuard g |
+    config.isBarrierGuard(g) and
+    node = g.getAGuardedNode()
+  )
 }
 
 private class AdditionalFlowStepSource extends Node {
diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll
index 1d51a88d13de..094a2530c319 100644
--- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll
+++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll
@@ -75,6 +75,9 @@ abstract class Configuration extends string {
   /** Holds if data flow out of `node` is prohibited. */
   predicate isBarrierOut(Node node) { none() }
 
+  /** Holds if data flow through nodes guarded by `guard` is prohibited. */
+  predicate isBarrierGuard(BarrierGuard guard) { none() }
+
   /**
    * Holds if the additional flow step from `node1` to `node2` must be taken
    * into account in the analysis.
@@ -136,6 +139,11 @@ private predicate fullBarrier(Node node, Configuration config) {
   or
   config.isBarrierOut(node) and
   not config.isSink(node)
+  or
+  exists(BarrierGuard g |
+    config.isBarrierGuard(g) and
+    node = g.getAGuardedNode()
+  )
 }
 
 private class AdditionalFlowStepSource extends Node {
diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll
index 13fcb586395f..545b0174269e 100644
--- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll
+++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll
@@ -3,6 +3,7 @@ private import cil
 private import dotnet
 private import DataFlowPrivate
 private import semmle.code.csharp.Caching
+private import semmle.code.csharp.controlflow.Guards
 
 /**
  * An element, viewed as a node in a data flow graph. Either an expression
@@ -162,3 +163,14 @@ abstract class NonLocalJumpNode extends Node {
   /** Gets a successor node that is potentially in another callable. */
   abstract Node getAJumpSuccessor(boolean preservesValue);
 }
+
+/** A guard that validates some expression. */
+class BarrierGuard extends Internal::Guard {
+  /** Holds if this guard validates `e` upon evaluating to `v`. */
+  abstract predicate checks(Expr e, AbstractValue v);
+
+  /** Gets a node guarded by this. */
+  final Node getAGuardedNode() {
+    none() // stub
+  }
+}
diff --git a/java/ql/src/Security/CWE/CWE-022/TaintedPath.ql b/java/ql/src/Security/CWE/CWE-022/TaintedPath.ql
index 31fb0c5035cd..2094207dc92b 100644
--- a/java/ql/src/Security/CWE/CWE-022/TaintedPath.ql
+++ b/java/ql/src/Security/CWE/CWE-022/TaintedPath.ql
@@ -17,6 +17,17 @@ import semmle.code.java.dataflow.FlowSources
 import PathsCommon
 import DataFlow::PathGraph
 
+class ContainsDotDotSanitizer extends DataFlow::BarrierGuard {
+  ContainsDotDotSanitizer() {
+    this.(MethodAccess).getMethod().hasName("contains") and
+    this.(MethodAccess).getAnArgument().(StringLiteral).getValue() = ".."
+  }
+
+  override predicate checks(Expr e, boolean branch) {
+    e = this.(MethodAccess).getQualifier() and branch = false
+  }
+}
+
 class TaintedPathConfig extends TaintTracking::Configuration {
   TaintedPathConfig() { this = "TaintedPathConfig" }
 
@@ -29,6 +40,10 @@ class TaintedPathConfig extends TaintTracking::Configuration {
   override predicate isSanitizer(DataFlow::Node node) {
     exists(Type t | t = node.getType() | t instanceof BoxedType or t instanceof PrimitiveType)
   }
+
+  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+    guard instanceof ContainsDotDotSanitizer
+  }
 }
 
 from DataFlow::PathNode source, DataFlow::PathNode sink, PathCreation p, TaintedPathConfig conf
diff --git a/java/ql/src/semmle/code/java/dataflow/TaintTracking.qll b/java/ql/src/semmle/code/java/dataflow/TaintTracking.qll
index c6dfc4e88acb..6d79359bb362 100644
--- a/java/ql/src/semmle/code/java/dataflow/TaintTracking.qll
+++ b/java/ql/src/semmle/code/java/dataflow/TaintTracking.qll
@@ -80,6 +80,11 @@ module TaintTracking {
 
     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 `node1` to `node2`
      * must be taken into account in the analysis.
@@ -162,6 +167,11 @@ module TaintTracking {
 
     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 `node1` to `node2`
      * must be taken into account in the analysis.
diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll
index 1d51a88d13de..094a2530c319 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll
@@ -75,6 +75,9 @@ abstract class Configuration extends string {
   /** Holds if data flow out of `node` is prohibited. */
   predicate isBarrierOut(Node node) { none() }
 
+  /** Holds if data flow through nodes guarded by `guard` is prohibited. */
+  predicate isBarrierGuard(BarrierGuard guard) { none() }
+
   /**
    * Holds if the additional flow step from `node1` to `node2` must be taken
    * into account in the analysis.
@@ -136,6 +139,11 @@ private predicate fullBarrier(Node node, Configuration config) {
   or
   config.isBarrierOut(node) and
   not config.isSink(node)
+  or
+  exists(BarrierGuard g |
+    config.isBarrierGuard(g) and
+    node = g.getAGuardedNode()
+  )
 }
 
 private class AdditionalFlowStepSource extends Node {
diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl2.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl2.qll
index 1d51a88d13de..094a2530c319 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl2.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl2.qll
@@ -75,6 +75,9 @@ abstract class Configuration extends string {
   /** Holds if data flow out of `node` is prohibited. */
   predicate isBarrierOut(Node node) { none() }
 
+  /** Holds if data flow through nodes guarded by `guard` is prohibited. */
+  predicate isBarrierGuard(BarrierGuard guard) { none() }
+
   /**
    * Holds if the additional flow step from `node1` to `node2` must be taken
    * into account in the analysis.
@@ -136,6 +139,11 @@ private predicate fullBarrier(Node node, Configuration config) {
   or
   config.isBarrierOut(node) and
   not config.isSink(node)
+  or
+  exists(BarrierGuard g |
+    config.isBarrierGuard(g) and
+    node = g.getAGuardedNode()
+  )
 }
 
 private class AdditionalFlowStepSource extends Node {
diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl3.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl3.qll
index 1d51a88d13de..094a2530c319 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl3.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl3.qll
@@ -75,6 +75,9 @@ abstract class Configuration extends string {
   /** Holds if data flow out of `node` is prohibited. */
   predicate isBarrierOut(Node node) { none() }
 
+  /** Holds if data flow through nodes guarded by `guard` is prohibited. */
+  predicate isBarrierGuard(BarrierGuard guard) { none() }
+
   /**
    * Holds if the additional flow step from `node1` to `node2` must be taken
    * into account in the analysis.
@@ -136,6 +139,11 @@ private predicate fullBarrier(Node node, Configuration config) {
   or
   config.isBarrierOut(node) and
   not config.isSink(node)
+  or
+  exists(BarrierGuard g |
+    config.isBarrierGuard(g) and
+    node = g.getAGuardedNode()
+  )
 }
 
 private class AdditionalFlowStepSource extends Node {
diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl4.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl4.qll
index 1d51a88d13de..094a2530c319 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl4.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl4.qll
@@ -75,6 +75,9 @@ abstract class Configuration extends string {
   /** Holds if data flow out of `node` is prohibited. */
   predicate isBarrierOut(Node node) { none() }
 
+  /** Holds if data flow through nodes guarded by `guard` is prohibited. */
+  predicate isBarrierGuard(BarrierGuard guard) { none() }
+
   /**
    * Holds if the additional flow step from `node1` to `node2` must be taken
    * into account in the analysis.
@@ -136,6 +139,11 @@ private predicate fullBarrier(Node node, Configuration config) {
   or
   config.isBarrierOut(node) and
   not config.isSink(node)
+  or
+  exists(BarrierGuard g |
+    config.isBarrierGuard(g) and
+    node = g.getAGuardedNode()
+  )
 }
 
 private class AdditionalFlowStepSource extends Node {
diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl5.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl5.qll
index 1d51a88d13de..094a2530c319 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl5.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl5.qll
@@ -75,6 +75,9 @@ abstract class Configuration extends string {
   /** Holds if data flow out of `node` is prohibited. */
   predicate isBarrierOut(Node node) { none() }
 
+  /** Holds if data flow through nodes guarded by `guard` is prohibited. */
+  predicate isBarrierGuard(BarrierGuard guard) { none() }
+
   /**
    * Holds if the additional flow step from `node1` to `node2` must be taken
    * into account in the analysis.
@@ -136,6 +139,11 @@ private predicate fullBarrier(Node node, Configuration config) {
   or
   config.isBarrierOut(node) and
   not config.isSink(node)
+  or
+  exists(BarrierGuard g |
+    config.isBarrierGuard(g) and
+    node = g.getAGuardedNode()
+  )
 }
 
 private class AdditionalFlowStepSource extends Node {
diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll
index aae6f2ae2df9..7657fea48eb7 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll
@@ -6,6 +6,7 @@ private import java
 private import DataFlowPrivate
 private import semmle.code.java.dataflow.SSA
 private import semmle.code.java.dataflow.TypeFlow
+private import semmle.code.java.controlflow.Guards
 import semmle.code.java.dataflow.InstanceAccess
 
 cached
@@ -416,3 +417,19 @@ Node getInstanceArgument(Call call) {
   explicitInstanceArgument(call, result.asExpr()) or
   implicitInstanceArgument(call, result.(ImplicitInstanceAccess).getInstanceAccess())
 }
+
+/** A guard that validates some expression. */
+class BarrierGuard extends Guard {
+  /** Holds if this guard validates `e` upon evaluating to `branch`. */
+  abstract predicate checks(Expr e, boolean branch);
+
+  /** Gets a node guarded by this. */
+  final Node getAGuardedNode() {
+    exists(SsaVariable v, boolean branch, RValue use |
+      this.checks(v.getAUse(), branch) and
+      use = v.getAUse() and
+      this.controls(use.getBasicBlock(), branch) and
+      result.asExpr() = use
+    )
+  }
+}

From 9e902066add987f4f426152660c3a1ccc1f6693d Mon Sep 17 00:00:00 2001
From: Anders Schack-Mulligen <schack@semmle.com>
Date: Fri, 9 Aug 2019 14:13:26 +0200
Subject: [PATCH 2/4] Java/C++/C#: Elaborate qldoc.

---
 .../code/cpp/dataflow/internal/DataFlowUtil.qll    | 14 +++++++++++---
 .../code/cpp/ir/dataflow/internal/DataFlowUtil.qll | 14 +++++++++++---
 .../csharp/dataflow/internal/DataFlowPublic.qll    | 14 +++++++++++---
 .../code/java/dataflow/internal/DataFlowUtil.qll   | 10 +++++++++-
 4 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index fe5518828ce4..b2a54b01e42c 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -332,10 +332,18 @@ VariableAccess getAnAccessToAssignedVariable(Expr assign) {
   )
 }
 
-/** A guard that validates some expression. */
+/**
+ * A guard that validates some expression.
+ *
+ * To use this in a configuration, extend the class and provide a
+ * characteristic predicate precisely specifying the guard, and override
+ * `checks` to specify what is being validated and in which branch.
+ *
+ * It is important that all extending classes in scope are disjoint.
+ */
 class BarrierGuard extends Expr {
-  /** Holds if this guard validates `e` upon evaluating to `branch`. */
-  abstract predicate checks(Expr e, boolean branch);
+  /** NOT YET SUPPORTED. Holds if this guard validates `e` upon evaluating to `branch`. */
+  abstract deprecated predicate checks(Expr e, boolean branch);
 
   /** Gets a node guarded by this. */
   final Node getAGuardedNode() {
diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
index 965d4c14b249..46ae1dcb28cf 100644
--- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
@@ -168,10 +168,18 @@ predicate localFlowStep(Node nodeFrom, Node nodeTo) {
  */
 predicate localFlow(Node source, Node sink) { localFlowStep*(source, sink) }
 
-/** A guard that validates some expression. */
+/**
+ * A guard that validates some expression.
+ *
+ * To use this in a configuration, extend the class and provide a
+ * characteristic predicate precisely specifying the guard, and override
+ * `checks` to specify what is being validated and in which branch.
+ *
+ * It is important that all extending classes in scope are disjoint.
+ */
 class BarrierGuard extends IRGuardCondition {
-  /** Holds if this guard validates `e` upon evaluating to `b`. */
-  abstract predicate checks(Instruction e, boolean b);
+  /** NOT YET SUPPORTED. Holds if this guard validates `e` upon evaluating to `b`. */
+  abstract deprecated predicate checks(Instruction e, boolean b);
 
   /** Gets a node guarded by this. */
   final Node getAGuardedNode() {
diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll
index 545b0174269e..42f1dbf3903c 100644
--- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll
+++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll
@@ -164,10 +164,18 @@ abstract class NonLocalJumpNode extends Node {
   abstract Node getAJumpSuccessor(boolean preservesValue);
 }
 
-/** A guard that validates some expression. */
+/**
+ * A guard that validates some expression.
+ *
+ * To use this in a configuration, extend the class and provide a
+ * characteristic predicate precisely specifying the guard, and override
+ * `checks` to specify what is being validated and in which branch.
+ *
+ * It is important that all extending classes in scope are disjoint.
+ */
 class BarrierGuard extends Internal::Guard {
-  /** Holds if this guard validates `e` upon evaluating to `v`. */
-  abstract predicate checks(Expr e, AbstractValue v);
+  /** NOT YET SUPPORTED. Holds if this guard validates `e` upon evaluating to `v`. */
+  abstract deprecated predicate checks(Expr e, AbstractValue v);
 
   /** Gets a node guarded by this. */
   final Node getAGuardedNode() {
diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll
index 7657fea48eb7..0d8756d7cc75 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll
@@ -418,7 +418,15 @@ Node getInstanceArgument(Call call) {
   implicitInstanceArgument(call, result.(ImplicitInstanceAccess).getInstanceAccess())
 }
 
-/** A guard that validates some expression. */
+/**
+ * A guard that validates some expression.
+ *
+ * To use this in a configuration, extend the class and provide a
+ * characteristic predicate precisely specifying the guard, and override
+ * `checks` to specify what is being validated and in which branch.
+ *
+ * It is important that all extending classes in scope are disjoint.
+ */
 class BarrierGuard extends Guard {
   /** Holds if this guard validates `e` upon evaluating to `branch`. */
   abstract predicate checks(Expr e, boolean branch);

From 411bc16f44cafd05642988651b95ff569a703230 Mon Sep 17 00:00:00 2001
From: Anders Schack-Mulligen <schack@semmle.com>
Date: Mon, 12 Aug 2019 10:37:41 +0200
Subject: [PATCH 3/4] Java/C++/C#: Address review comment.

---
 cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll   | 2 +-
 .../src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll   | 2 +-
 .../src/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll | 2 +-
 java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index b2a54b01e42c..17bf13d9e39d 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -345,7 +345,7 @@ class BarrierGuard extends Expr {
   /** NOT YET SUPPORTED. Holds if this guard validates `e` upon evaluating to `branch`. */
   abstract deprecated predicate checks(Expr e, boolean branch);
 
-  /** Gets a node guarded by this. */
+  /** Gets a node guarded by this guard. */
   final Node getAGuardedNode() {
     none() // stub
   }
diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
index 46ae1dcb28cf..5ceb4e79682f 100644
--- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
@@ -181,7 +181,7 @@ class BarrierGuard extends IRGuardCondition {
   /** NOT YET SUPPORTED. Holds if this guard validates `e` upon evaluating to `b`. */
   abstract deprecated predicate checks(Instruction e, boolean b);
 
-  /** Gets a node guarded by this. */
+  /** Gets a node guarded by this guard. */
   final Node getAGuardedNode() {
     none() // stub
   }
diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll
index 42f1dbf3903c..dafb7a0c0300 100644
--- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll
+++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll
@@ -177,7 +177,7 @@ class BarrierGuard extends Internal::Guard {
   /** NOT YET SUPPORTED. Holds if this guard validates `e` upon evaluating to `v`. */
   abstract deprecated predicate checks(Expr e, AbstractValue v);
 
-  /** Gets a node guarded by this. */
+  /** Gets a node guarded by this guard. */
   final Node getAGuardedNode() {
     none() // stub
   }
diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll
index 0d8756d7cc75..454f30ad5f18 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll
@@ -431,7 +431,7 @@ class BarrierGuard extends Guard {
   /** Holds if this guard validates `e` upon evaluating to `branch`. */
   abstract predicate checks(Expr e, boolean branch);
 
-  /** Gets a node guarded by this. */
+  /** Gets a node guarded by this guard. */
   final Node getAGuardedNode() {
     exists(SsaVariable v, boolean branch, RValue use |
       this.checks(v.getAUse(), branch) and

From c99d0e7bd5d01bb5fa1512b14a42a3e0e04e48c5 Mon Sep 17 00:00:00 2001
From: Anders Schack-Mulligen <schack@semmle.com>
Date: Tue, 13 Aug 2019 16:59:59 +0200
Subject: [PATCH 4/4] Java: Add change note.

---
 change-notes/1.22/analysis-java.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/change-notes/1.22/analysis-java.md b/change-notes/1.22/analysis-java.md
index ad6447e5bc1a..6cbb13106430 100644
--- a/change-notes/1.22/analysis-java.md
+++ b/change-notes/1.22/analysis-java.md
@@ -6,6 +6,7 @@
 |----------------------------|------------------------|------------------------------------------------------------------|
 | Equals method does not inspect argument type (`java/unchecked-cast-in-equals`) | Fewer false positive and more true positive results | Precision has been improved by doing a bit of inter-procedural analysis and relying less on ad-hoc method names. |
 | Uncontrolled data in arithmetic expression (`java/uncontrolled-arithmetic`) | Fewer false positive results | Precision has been improved in several ways, in particular, by better detection of guards along the data-flow path. |
+| Uncontrolled data used in path expression (`java/path-injection`) | Fewer false positive results | The query no longer reports results guarded by `!var.contains("..")`. |
 | User-controlled data in arithmetic expression (`java/tainted-arithmetic`) | Fewer false positive results | Precision has been improved in several ways, in particular, by better detection of guards along the data-flow path. |
 
 ## Changes to QL libraries