Merge pull request #9618 from aschackmull/dataflow/deprecate-barrierguard-class

Dataflow: Deprecate BarrierGuard class

csharp/ql/lib/change-notes/2022-06-21-barrierguard-deprecation.md

@@ -0,0 +1,4 @@
+---
+category: deprecated
+---
+* The `BarrierGuard` class has been deprecated. Such barriers and sanitizers can now instead be created using the new `BarrierGuard` parameterized module.

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl.qll

@@ -90,14 +90,20 @@ 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() }
+  /**
+   * DEPRECATED: Use `isBarrier` and `BarrierGuard` module instead.
+   *
+   * Holds if data flow through nodes guarded by `guard` is prohibited.
+   */
+  deprecated predicate isBarrierGuard(BarrierGuard guard) { none() }
 
   /**
+   * DEPRECATED: Use `isBarrier` and `BarrierGuard` module instead.
+   *
    * Holds if data flow through nodes guarded by `guard` is prohibited when
    * the flow state is `state`
    */
-  predicate isBarrierGuard(BarrierGuard guard, FlowState state) { none() }
+  deprecated predicate isBarrierGuard(BarrierGuard guard, FlowState state) { none() }
 
   /**
    * Holds if data may flow from `node1` to `node2` in addition to the normal data-flow steps.
@@ -335,6 +341,29 @@ private predicate outBarrier(NodeEx node, Configuration config) {
   )
 }
 
+/** A bridge class to access the deprecated `isBarrierGuard`. */
+private class BarrierGuardGuardedNodeBridge extends Unit {
+  abstract predicate guardedNode(Node n, Configuration config);
+
+  abstract predicate guardedNode(Node n, FlowState state, Configuration config);
+}
+
+private class BarrierGuardGuardedNode extends BarrierGuardGuardedNodeBridge {
+  deprecated override predicate guardedNode(Node n, Configuration config) {
+    exists(BarrierGuard g |
+      config.isBarrierGuard(g) and
+      n = g.getAGuardedNode()
+    )
+  }
+
+  deprecated override predicate guardedNode(Node n, FlowState state, Configuration config) {
+    exists(BarrierGuard g |
+      config.isBarrierGuard(g, state) and
+      n = g.getAGuardedNode()
+    )
+  }
+}
+
 pragma[nomagic]
 private predicate fullBarrier(NodeEx node, Configuration config) {
   exists(Node n | node.asNode() = n |
@@ -348,10 +377,7 @@ private predicate fullBarrier(NodeEx node, Configuration config) {
     not config.isSink(n) and
     not config.isSink(n, _)
     or
-    exists(BarrierGuard g |
-      config.isBarrierGuard(g) and
-      n = g.getAGuardedNode()
-    )
+    any(BarrierGuardGuardedNodeBridge b).guardedNode(n, config)
   )
 }
 
@@ -360,10 +386,7 @@ private predicate stateBarrier(NodeEx node, FlowState state, Configuration confi
   exists(Node n | node.asNode() = n |
     config.isBarrier(n, state)
     or
-    exists(BarrierGuard g |
-      config.isBarrierGuard(g, state) and
-      n = g.getAGuardedNode()
-    )
+    any(BarrierGuardGuardedNodeBridge b).guardedNode(n, state, config)
   )
 }
 

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll

@@ -90,14 +90,20 @@ 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() }
+  /**
+   * DEPRECATED: Use `isBarrier` and `BarrierGuard` module instead.
+   *
+   * Holds if data flow through nodes guarded by `guard` is prohibited.
+   */
+  deprecated predicate isBarrierGuard(BarrierGuard guard) { none() }
 
   /**
+   * DEPRECATED: Use `isBarrier` and `BarrierGuard` module instead.
+   *
    * Holds if data flow through nodes guarded by `guard` is prohibited when
    * the flow state is `state`
    */
-  predicate isBarrierGuard(BarrierGuard guard, FlowState state) { none() }
+  deprecated predicate isBarrierGuard(BarrierGuard guard, FlowState state) { none() }
 
   /**
    * Holds if data may flow from `node1` to `node2` in addition to the normal data-flow steps.
@@ -335,6 +341,29 @@ private predicate outBarrier(NodeEx node, Configuration config) {
   )
 }
 
+/** A bridge class to access the deprecated `isBarrierGuard`. */
+private class BarrierGuardGuardedNodeBridge extends Unit {
+  abstract predicate guardedNode(Node n, Configuration config);
+
+  abstract predicate guardedNode(Node n, FlowState state, Configuration config);
+}
+
+private class BarrierGuardGuardedNode extends BarrierGuardGuardedNodeBridge {
+  deprecated override predicate guardedNode(Node n, Configuration config) {
+    exists(BarrierGuard g |
+      config.isBarrierGuard(g) and
+      n = g.getAGuardedNode()
+    )
+  }
+
+  deprecated override predicate guardedNode(Node n, FlowState state, Configuration config) {
+    exists(BarrierGuard g |
+      config.isBarrierGuard(g, state) and
+      n = g.getAGuardedNode()
+    )
+  }
+}
+
 pragma[nomagic]
 private predicate fullBarrier(NodeEx node, Configuration config) {
   exists(Node n | node.asNode() = n |
@@ -348,10 +377,7 @@ private predicate fullBarrier(NodeEx node, Configuration config) {
     not config.isSink(n) and
     not config.isSink(n, _)
     or
-    exists(BarrierGuard g |
-      config.isBarrierGuard(g) and
-      n = g.getAGuardedNode()
-    )
+    any(BarrierGuardGuardedNodeBridge b).guardedNode(n, config)
   )
 }
 
@@ -360,10 +386,7 @@ private predicate stateBarrier(NodeEx node, FlowState state, Configuration confi
   exists(Node n | node.asNode() = n |
     config.isBarrier(n, state)
     or
-    exists(BarrierGuard g |
-      config.isBarrierGuard(g, state) and
-      n = g.getAGuardedNode()
-    )
+    any(BarrierGuardGuardedNodeBridge b).guardedNode(n, state, config)
   )
 }
 

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll

@@ -90,14 +90,20 @@ 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() }
+  /**
+   * DEPRECATED: Use `isBarrier` and `BarrierGuard` module instead.
+   *
+   * Holds if data flow through nodes guarded by `guard` is prohibited.
+   */
+  deprecated predicate isBarrierGuard(BarrierGuard guard) { none() }
 
   /**
+   * DEPRECATED: Use `isBarrier` and `BarrierGuard` module instead.
+   *
    * Holds if data flow through nodes guarded by `guard` is prohibited when
    * the flow state is `state`
    */
-  predicate isBarrierGuard(BarrierGuard guard, FlowState state) { none() }
+  deprecated predicate isBarrierGuard(BarrierGuard guard, FlowState state) { none() }
 
   /**
    * Holds if data may flow from `node1` to `node2` in addition to the normal data-flow steps.
@@ -335,6 +341,29 @@ private predicate outBarrier(NodeEx node, Configuration config) {
   )
 }
 
+/** A bridge class to access the deprecated `isBarrierGuard`. */
+private class BarrierGuardGuardedNodeBridge extends Unit {
+  abstract predicate guardedNode(Node n, Configuration config);
+
+  abstract predicate guardedNode(Node n, FlowState state, Configuration config);
+}
+
+private class BarrierGuardGuardedNode extends BarrierGuardGuardedNodeBridge {
+  deprecated override predicate guardedNode(Node n, Configuration config) {
+    exists(BarrierGuard g |
+      config.isBarrierGuard(g) and
+      n = g.getAGuardedNode()
+    )
+  }
+
+  deprecated override predicate guardedNode(Node n, FlowState state, Configuration config) {
+    exists(BarrierGuard g |
+      config.isBarrierGuard(g, state) and
+      n = g.getAGuardedNode()
+    )
+  }
+}
+
 pragma[nomagic]
 private predicate fullBarrier(NodeEx node, Configuration config) {
   exists(Node n | node.asNode() = n |
@@ -348,10 +377,7 @@ private predicate fullBarrier(NodeEx node, Configuration config) {
     not config.isSink(n) and
     not config.isSink(n, _)
     or
-    exists(BarrierGuard g |
-      config.isBarrierGuard(g) and
-      n = g.getAGuardedNode()
-    )
+    any(BarrierGuardGuardedNodeBridge b).guardedNode(n, config)
   )
 }
 
@@ -360,10 +386,7 @@ private predicate stateBarrier(NodeEx node, FlowState state, Configuration confi
   exists(Node n | node.asNode() = n |
     config.isBarrier(n, state)
     or
-    exists(BarrierGuard g |
-      config.isBarrierGuard(g, state) and
-      n = g.getAGuardedNode()
-    )
+    any(BarrierGuardGuardedNodeBridge b).guardedNode(n, state, config)
   )
 }
 

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll

@@ -90,14 +90,20 @@ 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() }
+  /**
+   * DEPRECATED: Use `isBarrier` and `BarrierGuard` module instead.
+   *
+   * Holds if data flow through nodes guarded by `guard` is prohibited.
+   */
+  deprecated predicate isBarrierGuard(BarrierGuard guard) { none() }
 
   /**
+   * DEPRECATED: Use `isBarrier` and `BarrierGuard` module instead.
+   *
    * Holds if data flow through nodes guarded by `guard` is prohibited when
    * the flow state is `state`
    */
-  predicate isBarrierGuard(BarrierGuard guard, FlowState state) { none() }
+  deprecated predicate isBarrierGuard(BarrierGuard guard, FlowState state) { none() }
 
   /**
    * Holds if data may flow from `node1` to `node2` in addition to the normal data-flow steps.
@@ -335,6 +341,29 @@ private predicate outBarrier(NodeEx node, Configuration config) {
   )
 }
 
+/** A bridge class to access the deprecated `isBarrierGuard`. */
+private class BarrierGuardGuardedNodeBridge extends Unit {
+  abstract predicate guardedNode(Node n, Configuration config);
+
+  abstract predicate guardedNode(Node n, FlowState state, Configuration config);
+}
+
+private class BarrierGuardGuardedNode extends BarrierGuardGuardedNodeBridge {
+  deprecated override predicate guardedNode(Node n, Configuration config) {
+    exists(BarrierGuard g |
+      config.isBarrierGuard(g) and
+      n = g.getAGuardedNode()
+    )
+  }
+
+  deprecated override predicate guardedNode(Node n, FlowState state, Configuration config) {
+    exists(BarrierGuard g |
+      config.isBarrierGuard(g, state) and
+      n = g.getAGuardedNode()
+    )
+  }
+}
+
 pragma[nomagic]
 private predicate fullBarrier(NodeEx node, Configuration config) {
   exists(Node n | node.asNode() = n |
@@ -348,10 +377,7 @@ private predicate fullBarrier(NodeEx node, Configuration config) {
     not config.isSink(n) and
     not config.isSink(n, _)
     or
-    exists(BarrierGuard g |
-      config.isBarrierGuard(g) and
-      n = g.getAGuardedNode()
-    )
+    any(BarrierGuardGuardedNodeBridge b).guardedNode(n, config)
   )
 }
 
@@ -360,10 +386,7 @@ private predicate stateBarrier(NodeEx node, FlowState state, Configuration confi
   exists(Node n | node.asNode() = n |
     config.isBarrier(n, state)
     or
-    exists(BarrierGuard g |
-      config.isBarrierGuard(g, state) and
-      n = g.getAGuardedNode()
-    )
+    any(BarrierGuardGuardedNodeBridge b).guardedNode(n, state, config)
   )
 }
 

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll

@@ -90,14 +90,20 @@ 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() }
+  /**
+   * DEPRECATED: Use `isBarrier` and `BarrierGuard` module instead.
+   *
+   * Holds if data flow through nodes guarded by `guard` is prohibited.
+   */
+  deprecated predicate isBarrierGuard(BarrierGuard guard) { none() }
 
   /**
+   * DEPRECATED: Use `isBarrier` and `BarrierGuard` module instead.
+   *
    * Holds if data flow through nodes guarded by `guard` is prohibited when
    * the flow state is `state`
    */
-  predicate isBarrierGuard(BarrierGuard guard, FlowState state) { none() }
+  deprecated predicate isBarrierGuard(BarrierGuard guard, FlowState state) { none() }
 
   /**
    * Holds if data may flow from `node1` to `node2` in addition to the normal data-flow steps.
@@ -335,6 +341,29 @@ private predicate outBarrier(NodeEx node, Configuration config) {
   )
 }
 
+/** A bridge class to access the deprecated `isBarrierGuard`. */
+private class BarrierGuardGuardedNodeBridge extends Unit {
+  abstract predicate guardedNode(Node n, Configuration config);
+
+  abstract predicate guardedNode(Node n, FlowState state, Configuration config);
+}
+
+private class BarrierGuardGuardedNode extends BarrierGuardGuardedNodeBridge {
+  deprecated override predicate guardedNode(Node n, Configuration config) {
+    exists(BarrierGuard g |
+      config.isBarrierGuard(g) and
+      n = g.getAGuardedNode()
+    )
+  }
+
+  deprecated override predicate guardedNode(Node n, FlowState state, Configuration config) {
+    exists(BarrierGuard g |
+      config.isBarrierGuard(g, state) and
+      n = g.getAGuardedNode()
+    )
+  }
+}
+
 pragma[nomagic]
 private predicate fullBarrier(NodeEx node, Configuration config) {
   exists(Node n | node.asNode() = n |
@@ -348,10 +377,7 @@ private predicate fullBarrier(NodeEx node, Configuration config) {
     not config.isSink(n) and
     not config.isSink(n, _)
     or
-    exists(BarrierGuard g |
-      config.isBarrierGuard(g) and
-      n = g.getAGuardedNode()
-    )
+    any(BarrierGuardGuardedNodeBridge b).guardedNode(n, config)
   )
 }
 
@@ -360,10 +386,7 @@ private predicate stateBarrier(NodeEx node, FlowState state, Configuration confi
   exists(Node n | node.asNode() = n |
     config.isBarrier(n, state)
     or
-    exists(BarrierGuard g |
-      config.isBarrierGuard(g, state) and
-      n = g.getAGuardedNode()
-    )
+    any(BarrierGuardGuardedNodeBridge b).guardedNode(n, state, config)
   )
 }
 

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImplForContentDataFlow.qll

@@ -90,14 +90,20 @@ 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() }
+  /**
+   * DEPRECATED: Use `isBarrier` and `BarrierGuard` module instead.
+   *
+   * Holds if data flow through nodes guarded by `guard` is prohibited.
+   */
+  deprecated predicate isBarrierGuard(BarrierGuard guard) { none() }
 
   /**
+   * DEPRECATED: Use `isBarrier` and `BarrierGuard` module instead.
+   *
    * Holds if data flow through nodes guarded by `guard` is prohibited when
    * the flow state is `state`
    */
-  predicate isBarrierGuard(BarrierGuard guard, FlowState state) { none() }
+  deprecated predicate isBarrierGuard(BarrierGuard guard, FlowState state) { none() }
 
   /**
    * Holds if data may flow from `node1` to `node2` in addition to the normal data-flow steps.
@@ -335,6 +341,29 @@ private predicate outBarrier(NodeEx node, Configuration config) {
   )
 }
 
+/** A bridge class to access the deprecated `isBarrierGuard`. */
+private class BarrierGuardGuardedNodeBridge extends Unit {
+  abstract predicate guardedNode(Node n, Configuration config);
+
+  abstract predicate guardedNode(Node n, FlowState state, Configuration config);
+}
+
+private class BarrierGuardGuardedNode extends BarrierGuardGuardedNodeBridge {
+  deprecated override predicate guardedNode(Node n, Configuration config) {
+    exists(BarrierGuard g |
+      config.isBarrierGuard(g) and
+      n = g.getAGuardedNode()
+    )
+  }
+
+  deprecated override predicate guardedNode(Node n, FlowState state, Configuration config) {
+    exists(BarrierGuard g |
+      config.isBarrierGuard(g, state) and
+      n = g.getAGuardedNode()
+    )
+  }
+}
+
 pragma[nomagic]
 private predicate fullBarrier(NodeEx node, Configuration config) {
   exists(Node n | node.asNode() = n |
@@ -348,10 +377,7 @@ private predicate fullBarrier(NodeEx node, Configuration config) {
     not config.isSink(n) and
     not config.isSink(n, _)
     or
-    exists(BarrierGuard g |
-      config.isBarrierGuard(g) and
-      n = g.getAGuardedNode()
-    )
+    any(BarrierGuardGuardedNodeBridge b).guardedNode(n, config)
   )
 }
 
@@ -360,10 +386,7 @@ private predicate stateBarrier(NodeEx node, FlowState state, Configuration confi
   exists(Node n | node.asNode() = n |
     config.isBarrier(n, state)
     or
-    exists(BarrierGuard g |
-      config.isBarrierGuard(g, state) and
-      n = g.getAGuardedNode()
-    )
+    any(BarrierGuardGuardedNodeBridge b).guardedNode(n, state, config)
   )
 }
 

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll

@@ -173,6 +173,33 @@ abstract class NonLocalJumpNode extends Node {
 }
 
 /**
+ * Holds if the guard `g` validates the expression `e` upon evaluating to `v`.
+ *
+ * The expression `e` is expected to be a syntactic part of the guard `g`.
+ * For example, the guard `g` might be a call `isSafe(x)` and the expression `e`
+ * the argument `x`.
+ */
+signature predicate guardChecksSig(Guard g, Expr e, AbstractValue v);
+
+/**
+ * Provides a set of barrier nodes for a guard that validates an expression.
+ *
+ * This is expected to be used in `isBarrier`/`isSanitizer` definitions
+ * in data flow and taint tracking.
+ */
+module BarrierGuard<guardChecksSig/3 guardChecks> {
+  /** Gets a node that is safely guarded by the given guard check. */
+  ExprNode getABarrierNode() {
+    exists(Guard g, Expr e, AbstractValue v |
+      guardChecks(g, e, v) and
+      g.controlsNode(result.getControlFlowNode(), e, v)
+    )
+  }
+}
+
+/**
+ * DEPRECATED: Use `BarrierGuard` module instead.
+ *
  * A guard that validates some expression.
  *
  * To use this in a configuration, extend the class and provide a
@@ -181,7 +208,7 @@ abstract class NonLocalJumpNode extends Node {
  *
  * It is important that all extending classes in scope are disjoint.
  */
-class BarrierGuard extends Guard {
+deprecated class BarrierGuard extends Guard {
   /** Holds if this guard validates `e` upon evaluating to `v`. */
   abstract predicate checks(Expr e, AbstractValue v);
 

csharp/ql/lib/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll

@@ -19,12 +19,6 @@ private import semmle.code.csharp.frameworks.WCF
  */
 predicate defaultTaintSanitizer(DataFlow::Node node) { none() }
 
-/**
- * Holds if `guard` should be a sanitizer guard in all global taint flow configurations
- * but not in local taint.
- */
-predicate defaultTaintSanitizerGuard(DataFlow::BarrierGuard guard) { none() }
-
 /**
  * Holds if default `TaintTracking::Configuration`s should allow implicit reads
  * of `c` at sinks and inputs to additional taint steps.

csharp/ql/lib/semmle/code/csharp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll

@@ -116,20 +116,30 @@ abstract class Configuration extends DataFlow::Configuration {
 
   final override predicate isBarrierOut(DataFlow::Node node) { this.isSanitizerOut(node) }
 
-  /** Holds if taint propagation through nodes guarded by `guard` is prohibited. */
-  predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { none() }
+  /**
+   * DEPRECATED: Use `isSanitizer` and `BarrierGuard` module instead.
+   *
+   * Holds if taint propagation through nodes guarded by `guard` is prohibited.
+   */
+  deprecated predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { none() }
 
-  final override predicate isBarrierGuard(DataFlow::BarrierGuard guard) {
-    this.isSanitizerGuard(guard) or defaultTaintSanitizerGuard(guard)
+  deprecated final override predicate isBarrierGuard(DataFlow::BarrierGuard guard) {
+    this.isSanitizerGuard(guard)
   }
 
   /**
+   * DEPRECATED: Use `isSanitizer` and `BarrierGuard` module instead.
+   *
    * Holds if taint propagation through nodes guarded by `guard` is prohibited
    * when the flow state is `state`.
    */
-  predicate isSanitizerGuard(DataFlow::BarrierGuard guard, DataFlow::FlowState state) { none() }
+  deprecated predicate isSanitizerGuard(DataFlow::BarrierGuard guard, DataFlow::FlowState state) {
+    none()
+  }
 
-  final override predicate isBarrierGuard(DataFlow::BarrierGuard guard, DataFlow::FlowState state) {
+  deprecated final override predicate isBarrierGuard(
+    DataFlow::BarrierGuard guard, DataFlow::FlowState state
+  ) {
     this.isSanitizerGuard(guard, state)
   }
 

csharp/ql/lib/semmle/code/csharp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll

@@ -116,20 +116,30 @@ abstract class Configuration extends DataFlow::Configuration {
 
   final override predicate isBarrierOut(DataFlow::Node node) { this.isSanitizerOut(node) }
 
-  /** Holds if taint propagation through nodes guarded by `guard` is prohibited. */
-  predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { none() }
+  /**
+   * DEPRECATED: Use `isSanitizer` and `BarrierGuard` module instead.
+   *
+   * Holds if taint propagation through nodes guarded by `guard` is prohibited.
+   */
+  deprecated predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { none() }
 
-  final override predicate isBarrierGuard(DataFlow::BarrierGuard guard) {
-    this.isSanitizerGuard(guard) or defaultTaintSanitizerGuard(guard)
+  deprecated final override predicate isBarrierGuard(DataFlow::BarrierGuard guard) {
+    this.isSanitizerGuard(guard)
   }
 
   /**
+   * DEPRECATED: Use `isSanitizer` and `BarrierGuard` module instead.
+   *
    * Holds if taint propagation through nodes guarded by `guard` is prohibited
    * when the flow state is `state`.
    */
-  predicate isSanitizerGuard(DataFlow::BarrierGuard guard, DataFlow::FlowState state) { none() }
+  deprecated predicate isSanitizerGuard(DataFlow::BarrierGuard guard, DataFlow::FlowState state) {
+    none()
+  }
 
-  final override predicate isBarrierGuard(DataFlow::BarrierGuard guard, DataFlow::FlowState state) {
+  deprecated final override predicate isBarrierGuard(
+    DataFlow::BarrierGuard guard, DataFlow::FlowState state
+  ) {
     this.isSanitizerGuard(guard, state)
   }
 

csharp/ql/lib/semmle/code/csharp/dataflow/internal/tainttracking3/TaintTrackingImpl.qll

@@ -116,20 +116,30 @@ abstract class Configuration extends DataFlow::Configuration {
 
   final override predicate isBarrierOut(DataFlow::Node node) { this.isSanitizerOut(node) }
 
-  /** Holds if taint propagation through nodes guarded by `guard` is prohibited. */
-  predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { none() }
+  /**
+   * DEPRECATED: Use `isSanitizer` and `BarrierGuard` module instead.
+   *
+   * Holds if taint propagation through nodes guarded by `guard` is prohibited.
+   */
+  deprecated predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { none() }
 
-  final override predicate isBarrierGuard(DataFlow::BarrierGuard guard) {
-    this.isSanitizerGuard(guard) or defaultTaintSanitizerGuard(guard)
+  deprecated final override predicate isBarrierGuard(DataFlow::BarrierGuard guard) {
+    this.isSanitizerGuard(guard)
   }
 
   /**
+   * DEPRECATED: Use `isSanitizer` and `BarrierGuard` module instead.
+   *
    * Holds if taint propagation through nodes guarded by `guard` is prohibited
    * when the flow state is `state`.
    */
-  predicate isSanitizerGuard(DataFlow::BarrierGuard guard, DataFlow::FlowState state) { none() }
+  deprecated predicate isSanitizerGuard(DataFlow::BarrierGuard guard, DataFlow::FlowState state) {
+    none()
+  }
 
-  final override predicate isBarrierGuard(DataFlow::BarrierGuard guard, DataFlow::FlowState state) {
+  deprecated final override predicate isBarrierGuard(
+    DataFlow::BarrierGuard guard, DataFlow::FlowState state
+  ) {
     this.isSanitizerGuard(guard, state)
   }
 

csharp/ql/lib/semmle/code/csharp/dataflow/internal/tainttracking4/TaintTrackingImpl.qll

@@ -116,20 +116,30 @@ abstract class Configuration extends DataFlow::Configuration {
 
   final override predicate isBarrierOut(DataFlow::Node node) { this.isSanitizerOut(node) }
 
-  /** Holds if taint propagation through nodes guarded by `guard` is prohibited. */
-  predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { none() }
+  /**
+   * DEPRECATED: Use `isSanitizer` and `BarrierGuard` module instead.
+   *
+   * Holds if taint propagation through nodes guarded by `guard` is prohibited.
+   */
+  deprecated predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { none() }
 
-  final override predicate isBarrierGuard(DataFlow::BarrierGuard guard) {
-    this.isSanitizerGuard(guard) or defaultTaintSanitizerGuard(guard)
+  deprecated final override predicate isBarrierGuard(DataFlow::BarrierGuard guard) {
+    this.isSanitizerGuard(guard)
   }
 
   /**
+   * DEPRECATED: Use `isSanitizer` and `BarrierGuard` module instead.
+   *
    * Holds if taint propagation through nodes guarded by `guard` is prohibited
    * when the flow state is `state`.
    */
-  predicate isSanitizerGuard(DataFlow::BarrierGuard guard, DataFlow::FlowState state) { none() }
+  deprecated predicate isSanitizerGuard(DataFlow::BarrierGuard guard, DataFlow::FlowState state) {
+    none()
+  }
 
-  final override predicate isBarrierGuard(DataFlow::BarrierGuard guard, DataFlow::FlowState state) {
+  deprecated final override predicate isBarrierGuard(
+    DataFlow::BarrierGuard guard, DataFlow::FlowState state
+  ) {
     this.isSanitizerGuard(guard, state)
   }
 

csharp/ql/lib/semmle/code/csharp/dataflow/internal/tainttracking5/TaintTrackingImpl.qll

@@ -116,20 +116,30 @@ abstract class Configuration extends DataFlow::Configuration {
 
   final override predicate isBarrierOut(DataFlow::Node node) { this.isSanitizerOut(node) }
 
-  /** Holds if taint propagation through nodes guarded by `guard` is prohibited. */
-  predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { none() }
+  /**
+   * DEPRECATED: Use `isSanitizer` and `BarrierGuard` module instead.
+   *
+   * Holds if taint propagation through nodes guarded by `guard` is prohibited.
+   */
+  deprecated predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { none() }
 
-  final override predicate isBarrierGuard(DataFlow::BarrierGuard guard) {
-    this.isSanitizerGuard(guard) or defaultTaintSanitizerGuard(guard)
+  deprecated final override predicate isBarrierGuard(DataFlow::BarrierGuard guard) {
+    this.isSanitizerGuard(guard)
   }
 
   /**
+   * DEPRECATED: Use `isSanitizer` and `BarrierGuard` module instead.
+   *
    * Holds if taint propagation through nodes guarded by `guard` is prohibited
    * when the flow state is `state`.
    */
-  predicate isSanitizerGuard(DataFlow::BarrierGuard guard, DataFlow::FlowState state) { none() }
+  deprecated predicate isSanitizerGuard(DataFlow::BarrierGuard guard, DataFlow::FlowState state) {
+    none()
+  }
 
-  final override predicate isBarrierGuard(DataFlow::BarrierGuard guard, DataFlow::FlowState state) {
+  deprecated final override predicate isBarrierGuard(
+    DataFlow::BarrierGuard guard, DataFlow::FlowState state
+  ) {
     this.isSanitizerGuard(guard, state)
   }
 

csharp/ql/lib/semmle/code/csharp/security/dataflow/UrlRedirectQuery.qll

@@ -26,9 +26,11 @@ abstract class Sink extends DataFlow::ExprNode { }
 abstract class Sanitizer extends DataFlow::ExprNode { }
 
 /**
+ * DEPRECATED: Use `Sanitizer` instead.
+ *
  * A guard for unvalidated URL redirect vulnerabilities.
  */
-abstract class SanitizerGuard extends DataFlow::BarrierGuard { }
+abstract deprecated class SanitizerGuard extends DataFlow::BarrierGuard { }
 
 /**
  * A taint-tracking configuration for reasoning about unvalidated URL redirect vulnerabilities.
@@ -42,7 +44,7 @@ class TaintTrackingConfiguration extends TaintTracking::Configuration {
 
   override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
 
-  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+  deprecated override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
     guard instanceof SanitizerGuard
   }
 }
@@ -102,16 +104,17 @@ class HttpServerTransferSink extends Sink {
   }
 }
 
+private predicate isLocalUrlSanitizer(Guard g, Expr e, AbstractValue v) {
+  g.(MethodCall).getTarget().hasName("IsLocalUrl") and
+  e = g.(MethodCall).getArgument(0) and
+  v.(AbstractValues::BooleanValue).getValue() = true
+}
+
 /**
  * A URL argument to a call to `UrlHelper.isLocalUrl()` that is a sanitizer for URL redirects.
  */
-class IsLocalUrlSanitizer extends SanitizerGuard, MethodCall {
-  IsLocalUrlSanitizer() { this.getTarget().hasName("IsLocalUrl") }
-
-  override predicate checks(Expr e, AbstractValue v) {
-    e = this.getArgument(0) and
-    v.(AbstractValues::BooleanValue).getValue() = true
-  }
+class LocalUrlSanitizer extends Sanitizer {
+  LocalUrlSanitizer() { this = DataFlow::BarrierGuard<isLocalUrlSanitizer/3>::getABarrierNode() }
 }
 
 /**

csharp/ql/lib/semmle/code/csharp/security/dataflow/ZipSlipQuery.qll

@@ -21,9 +21,11 @@ abstract class Sink extends DataFlow::ExprNode { }
 abstract class Sanitizer extends DataFlow::ExprNode { }
 
 /**
+ * DEPRECATED: Use `Sanitizer` instead.
+ *
  * A guard for unsafe zip extraction.
  */
-abstract class SanitizerGuard extends DataFlow::BarrierGuard { }
+abstract deprecated class SanitizerGuard extends DataFlow::BarrierGuard { }
 
 /** A taint tracking configuration for Zip Slip */
 class TaintTrackingConfiguration extends TaintTracking::Configuration {
@@ -35,7 +37,7 @@ class TaintTrackingConfiguration extends TaintTracking::Configuration {
 
   override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
 
-  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+  deprecated override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
     guard instanceof SanitizerGuard
   }
 }
@@ -126,26 +128,22 @@ class SubstringSanitizer extends Sanitizer {
   }
 }
 
+private predicate stringCheckGuard(Guard g, Expr e, AbstractValue v) {
+  g.(MethodCall).getTarget().hasQualifiedName("System.String", "StartsWith") and
+  g.(MethodCall).getQualifier() = e and
+  // A StartsWith check against Path.Combine is not sufficient, because the ".." elements have
+  // not yet been resolved.
+  not exists(MethodCall combineCall |
+    combineCall.getTarget().hasQualifiedName("System.IO.Path", "Combine") and
+    DataFlow::localExprFlow(combineCall, e)
+  ) and
+  v.(AbstractValues::BooleanValue).getValue() = true
+}
+
 /**
  * A call to `String.StartsWith()` that indicates that the tainted path value is being
  * validated to ensure that it occurs within a permitted output path.
  */
-class StringCheckGuard extends SanitizerGuard, MethodCall {
-  private Expr q;
-
-  StringCheckGuard() {
-    this.getTarget().hasQualifiedName("System.String", "StartsWith") and
-    this.getQualifier() = q and
-    // A StartsWith check against Path.Combine is not sufficient, because the ".." elements have
-    // not yet been resolved.
-    not exists(MethodCall combineCall |
-      combineCall.getTarget().hasQualifiedName("System.IO.Path", "Combine") and
-      DataFlow::localExprFlow(combineCall, q)
-    )
-  }
-
-  override predicate checks(Expr e, AbstractValue v) {
-    e = q and
-    v.(AbstractValues::BooleanValue).getValue() = true
-  }
+class StringCheckSanitizer extends Sanitizer {
+  StringCheckSanitizer() { this = DataFlow::BarrierGuard<stringCheckGuard/3>::getABarrierNode() }
 }

csharp/ql/src/experimental/CWE-918/RequestForgery.qll

@@ -18,10 +18,10 @@ module RequestForgery {
   abstract private class Sink extends DataFlow::ExprNode { }
 
   /**
-   * A data flow BarrierGuard which blocks the flow of taint for
+   * A data flow Barrier that blocks the flow of taint for
    * server side request forgery vulnerabilities.
    */
-  abstract private class BarrierGuard extends DataFlow::BarrierGuard { }
+  abstract private class Barrier extends DataFlow::Node { }
 
   /**
    * A data flow configuration for detecting server side request forgery vulnerabilities.
@@ -51,9 +51,7 @@ module RequestForgery {
       pathCombineStep(prev, succ)
     }
 
-    override predicate isBarrierGuard(DataFlow::BarrierGuard guard) {
-      guard instanceof BarrierGuard
-    }
+    override predicate isBarrier(DataFlow::Node node) { node instanceof Barrier }
   }
 
   /**
@@ -129,17 +127,18 @@ module RequestForgery {
    * to be a guard for Server Side Request Forgery(SSRF) Vulnerabilities.
    * This guard considers all checks as valid.
    */
-  private class BaseUriGuard extends BarrierGuard, MethodCall {
-    BaseUriGuard() { this.getTarget().hasQualifiedName("System.Uri", "IsBaseOf") }
+  private predicate baseUriGuard(Guard g, Expr e, AbstractValue v) {
+    g.(MethodCall).getTarget().hasQualifiedName("System.Uri", "IsBaseOf") and
+    // we consider any checks against the tainted value to sainitize the taint.
+    // This implies any check such as shown below block the taint flow.
+    // Uri url = new Uri("whitelist.com")
+    // if (url.isBaseOf(`taint1))
+    (e = g.(MethodCall).getArgument(0) or e = g.(MethodCall).getQualifier()) and
+    v.(AbstractValues::BooleanValue).getValue() = true
+  }
 
-    override predicate checks(Expr e, AbstractValue v) {
-      // we consider any checks against the tainted value to sainitize the taint.
-      // This implies any check such as shown below block the taint flow.
-      // Uri url = new Uri("whitelist.com")
-      // if (url.isBaseOf(`taint1))
-      (e = this.getArgument(0) or e = this.getQualifier()) and
-      v.(AbstractValues::BooleanValue).getValue() = true
-    }
+  private class BaseUriBarrier extends Barrier {
+    BaseUriBarrier() { this = DataFlow::BarrierGuard<baseUriGuard/3>::getABarrierNode() }
   }
 
   /**
@@ -147,18 +146,19 @@ module RequestForgery {
    * to be a guard for Server Side Request Forgery(SSRF) Vulnerabilities.
    * This guard considers all checks as valid.
    */
-  private class StringStartsWithBarrierGuard extends BarrierGuard, MethodCall {
-    StringStartsWithBarrierGuard() {
-      this.getTarget().hasQualifiedName("System.String", "StartsWith")
-    }
+  private predicate stringStartsWithGuard(Guard g, Expr e, AbstractValue v) {
+    g.(MethodCall).getTarget().hasQualifiedName("System.String", "StartsWith") and
+    // Any check such as the ones shown below
+    // "https://myurl.com/".startsWith(`taint`)
+    // `taint`.startsWith("https://myurl.com/")
+    // are assumed to sainitize the taint
+    (e = g.(MethodCall).getQualifier() or g.(MethodCall).getArgument(0) = e) and
+    v.(AbstractValues::BooleanValue).getValue() = true
+  }
 
-    override predicate checks(Expr e, AbstractValue v) {
-      // Any check such as the ones shown below
-      // "https://myurl.com/".startsWith(`taint`)
-      // `taint`.startsWith("https://myurl.com/")
-      // are assumed to sainitize the taint
-      (e = this.getQualifier() or this.getArgument(0) = e) and
-      v.(AbstractValues::BooleanValue).getValue() = true
+  private class StringStartsWithBarrier extends Barrier {
+    StringStartsWithBarrier() {
+      this = DataFlow::BarrierGuard<stringStartsWithGuard/3>::getABarrierNode()
     }
   }