Java: Deprecate BarrierGuard class.

java/ql/src/experimental/Security/CWE/CWE-073/FilePathInjection.ql

@@ -30,10 +30,8 @@ class InjectFilePathConfig 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 PathTraversalBarrierGuard
+    or
+    node instanceof PathTraversalSanitizer
   }
 }
 

java/ql/src/experimental/Security/CWE/CWE-200/InsecureWebResourceResponse.ql

@@ -24,9 +24,7 @@ class InsecureWebResourceResponseConfig extends TaintTracking::Configuration {
 
   override predicate isSink(DataFlow::Node sink) { sink instanceof WebResourceResponseSink }
 
-  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
-    guard instanceof PathTraversalBarrierGuard
-  }
+  override predicate isSanitizer(DataFlow::Node node) { node instanceof PathTraversalSanitizer }
 }
 
 from DataFlow::PathNode source, DataFlow::PathNode sink, InsecureWebResourceResponseConfig conf

java/ql/src/experimental/Security/CWE/CWE-200/SensitiveAndroidFileLeak.ql

@@ -15,17 +15,13 @@ import AndroidFileIntentSink
 import AndroidFileIntentSource
 import DataFlow::PathGraph
 
-private class StartsWithSanitizer extends DataFlow::BarrierGuard {
-  StartsWithSanitizer() { this.(MethodAccess).getMethod().hasName("startsWith") }
-
-  override predicate checks(Expr e, boolean branch) {
-    e =
-      [
-        this.(MethodAccess).getQualifier(),
-        this.(MethodAccess).getQualifier().(MethodAccess).getQualifier()
-      ] and
+private predicate startsWithSanitizer(Guard g, Expr e, boolean branch) {
+  exists(MethodAccess ma |
+    g = ma and
+    ma.getMethod().hasName("startsWith") and
+    e = [ma.getQualifier(), ma.getQualifier().(MethodAccess).getQualifier()] and
     branch = false
-  }
+  )
 }
 
 class AndroidFileLeakConfig extends TaintTracking::Configuration {
@@ -75,8 +71,8 @@ class AndroidFileLeakConfig extends TaintTracking::Configuration {
     )
   }
 
-  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
-    guard instanceof StartsWithSanitizer
+  override predicate isSanitizer(DataFlow::Node node) {
+    node = DataFlow::BarrierGuard<startsWithSanitizer/3>::getABarrierNode()
   }
 }
 

java/ql/src/experimental/Security/CWE/CWE-400/LocalThreadResourceAbuse.ql

@@ -59,10 +59,8 @@ class ThreadResourceAbuse extends TaintTracking::Configuration {
       ma.getMethod().hasQualifiedName("java.lang", "Math", "min") and
       node.asExpr() = ma.getAnArgument()
     )
-  }
-
-  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
-    guard instanceof LessThanSanitizer // if (sleepTime > 0 && sleepTime < 5000) { ... }
+    or
+    node instanceof LessThanSanitizer // if (sleepTime > 0 && sleepTime < 5000) { ... }
   }
 }
 

java/ql/src/experimental/Security/CWE/CWE-400/ThreadResourceAbuse.ql

@@ -33,10 +33,8 @@ class ThreadResourceAbuse extends TaintTracking::Configuration {
       ma.getMethod().hasQualifiedName("java.lang", "Math", "min") and
       node.asExpr() = ma.getAnArgument()
     )
-  }
-
-  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
-    guard instanceof LessThanSanitizer // if (sleepTime > 0 && sleepTime < 5000) { ... }
+    or
+    node instanceof LessThanSanitizer // if (sleepTime > 0 && sleepTime < 5000) { ... }
   }
 }
 

java/ql/src/experimental/Security/CWE/CWE-400/ThreadResourceAbuse.qll

@@ -4,6 +4,7 @@ import java
 import semmle.code.java.dataflow.DataFlow
 private import semmle.code.java.dataflow.ExternalFlow
 import semmle.code.java.dataflow.FlowSteps
+import semmle.code.java.controlflow.Guards
 
 /** `java.lang.Math` data model for value comparison in the new CSV format. */
 private class MathCompDataModel extends SummaryModelCsv {
@@ -32,15 +33,17 @@ class PauseThreadSink extends DataFlow::Node {
   PauseThreadSink() { sinkNode(this, "thread-pause") }
 }
 
+private predicate lessThanGuard(Guard g, Expr e, boolean branch) {
+  e = g.(ComparisonExpr).getLesserOperand() and
+  branch = true
+  or
+  e = g.(ComparisonExpr).getGreaterOperand() and
+  branch = false
+}
+
 /** A sanitizer for lessThan check. */
-class LessThanSanitizer extends DataFlow::BarrierGuard instanceof ComparisonExpr {
-  override predicate checks(Expr e, boolean branch) {
-    e = super.getLesserOperand() and
-    branch = true
-    or
-    e = super.getGreaterOperand() and
-    branch = false
-  }
+class LessThanSanitizer extends DataFlow::Node {
+  LessThanSanitizer() { this = DataFlow::BarrierGuard<lessThanGuard/3>::getABarrierNode() }
 }
 
 /** Value step from the constructor call of a `Runnable` to the instance parameter (this) of `run`. */

java/ql/src/experimental/Security/CWE/CWE-470/UnsafeReflection.ql

@@ -15,23 +15,19 @@ import DataFlow
 import UnsafeReflectionLib
 import semmle.code.java.dataflow.DataFlow
 import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.controlflow.Guards
 import DataFlow::PathGraph
 
-private class ContainsSanitizer extends DataFlow::BarrierGuard {
-  ContainsSanitizer() { this.(MethodAccess).getMethod().hasName("contains") }
-
-  override predicate checks(Expr e, boolean branch) {
-    e = this.(MethodAccess).getArgument(0) and branch = true
-  }
+private predicate containsSanitizer(Guard g, Expr e, boolean branch) {
+  g.(MethodAccess).getMethod().hasName("contains") and
+  e = g.(MethodAccess).getArgument(0) and
+  branch = true
 }
 
-private class EqualsSanitizer extends DataFlow::BarrierGuard {
-  EqualsSanitizer() { this.(MethodAccess).getMethod().hasName("equals") }
-
-  override predicate checks(Expr e, boolean branch) {
-    e = [this.(MethodAccess).getArgument(0), this.(MethodAccess).getQualifier()] and
-    branch = true
-  }
+private predicate equalsSanitizer(Guard g, Expr e, boolean branch) {
+  g.(MethodAccess).getMethod().hasName("equals") and
+  e = [g.(MethodAccess).getArgument(0), g.(MethodAccess).getQualifier()] and
+  branch = true
 }
 
 class UnsafeReflectionConfig extends TaintTracking::Configuration {
@@ -78,8 +74,9 @@ class UnsafeReflectionConfig extends TaintTracking::Configuration {
     )
   }
 
-  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
-    guard instanceof ContainsSanitizer or guard instanceof EqualsSanitizer
+  override predicate isSanitizer(DataFlow::Node node) {
+    node = DataFlow::BarrierGuard<containsSanitizer/3>::getABarrierNode() or
+    node = DataFlow::BarrierGuard<equalsSanitizer/3>::getABarrierNode()
   }
 }
 

java/ql/src/experimental/Security/CWE/CWE-552/UnsafeUrlForward.ql

@@ -35,10 +35,9 @@ class UnsafeUrlForwardFlowConfig extends TaintTracking::Configuration {
 
   override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeUrlForwardSink }
 
-  override predicate isSanitizer(DataFlow::Node node) { node instanceof UnsafeUrlForwardSanitizer }
-
-  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
-    guard instanceof PathTraversalBarrierGuard
+  override predicate isSanitizer(DataFlow::Node node) {
+    node instanceof UnsafeUrlForwardSanitizer or
+    node instanceof PathTraversalSanitizer
   }
 
   override DataFlow::FlowFeature getAFeature() {

java/ql/src/experimental/Security/CWE/CWE-601/SpringUrlRedirect.ql

@@ -13,18 +13,15 @@
 import java
 import SpringUrlRedirect
 import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.controlflow.Guards
 import DataFlow::PathGraph
 
-private class StartsWithSanitizer extends DataFlow::BarrierGuard {
-  StartsWithSanitizer() {
-    this.(MethodAccess).getMethod().hasName("startsWith") and
-    this.(MethodAccess).getMethod().getDeclaringType() instanceof TypeString and
-    this.(MethodAccess).getMethod().getNumberOfParameters() = 1
-  }
-
-  override predicate checks(Expr e, boolean branch) {
-    e = this.(MethodAccess).getQualifier() and branch = true
-  }
+private predicate startsWithSanitizer(Guard g, Expr e, boolean branch) {
+  g.(MethodAccess).getMethod().hasName("startsWith") and
+  g.(MethodAccess).getMethod().getDeclaringType() instanceof TypeString and
+  g.(MethodAccess).getMethod().getNumberOfParameters() = 1 and
+  e = g.(MethodAccess).getQualifier() and
+  branch = true
 }
 
 class SpringUrlRedirectFlowConfig extends TaintTracking::Configuration {
@@ -38,10 +35,6 @@ class SpringUrlRedirectFlowConfig extends TaintTracking::Configuration {
     springUrlRedirectTaintStep(fromNode, toNode)
   }
 
-  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
-    guard instanceof StartsWithSanitizer
-  }
-
   override predicate isSanitizer(DataFlow::Node node) {
     // Exclude the case where the left side of the concatenated string is not `redirect:`.
     // E.g: `String url = "/path?token=" + request.getParameter("token");`
@@ -63,6 +56,8 @@ class SpringUrlRedirectFlowConfig extends TaintTracking::Configuration {
     )
     or
     nonLocationHeaderSanitizer(node)
+    or
+    node = DataFlow::BarrierGuard<startsWithSanitizer/3>::getABarrierNode()
   }
 }
 

java/ql/src/experimental/semmle/code/java/PathSanitizer.qll

@@ -3,21 +3,32 @@ private import semmle.code.java.controlflow.Guards
 private import semmle.code.java.dataflow.FlowSources
 private import semmle.code.java.dataflow.ExternalFlow
 
-/** A barrier guard that protects against path traversal vulnerabilities. */
-abstract class PathTraversalBarrierGuard extends DataFlow::BarrierGuard { }
+/**
+ * DEPRECATED: Use `PathTraversalSanitizer` instead.
+ *
+ * A barrier guard that protects against path traversal vulnerabilities.
+ */
+abstract deprecated class PathTraversalBarrierGuard extends DataFlow::BarrierGuard { }
+
+/** A sanitizer that protects against path traversal vulnerabilities. */
+abstract class PathTraversalSanitizer extends DataFlow::Node { }
 
 /**
- * A guard that considers safe a string being exactly compared to a trusted value.
+ * Holds if `g` is guard that compares a string to a trusted value.
  */
-private class ExactStringPathMatchGuard extends PathTraversalBarrierGuard instanceof MethodAccess {
-  ExactStringPathMatchGuard() {
-    super.getMethod().getDeclaringType() instanceof TypeString and
-    super.getMethod().getName() = ["equals", "equalsIgnoreCase"]
-  }
-
-  override predicate checks(Expr e, boolean branch) {
-    e = super.getQualifier() and
+private predicate exactStringPathMatchGuard(Guard g, Expr e, boolean branch) {
+  exists(MethodAccess ma |
+    ma = g and
+    ma.getMethod().getDeclaringType() instanceof TypeString and
+    ma.getMethod().getName() = ["equals", "equalsIgnoreCase"] and
+    e = ma.getQualifier() and
     branch = true
+  )
+}
+
+private class ExactStringPathMatchSanitizer extends PathTraversalSanitizer {
+  ExactStringPathMatchSanitizer() {
+    this = DataFlow::BarrierGuard<exactStringPathMatchGuard/3>::getABarrierNode()
   }
 }
 
@@ -42,41 +53,45 @@ private class AllowListGuard extends Guard instanceof MethodAccess {
 }
 
 /**
- * A guard that considers a path safe because it is checked against an allowlist of partial trusted values.
+ * Holds if `g` is a guard that considers a path safe because it is checked against an allowlist of partial trusted values.
  * This requires additional protection against path traversal, either another guard (`PathTraversalGuard`)
  * or a sanitizer (`PathNormalizeSanitizer`), to ensure any internal `..` components are removed from the path.
  */
-private class AllowListBarrierGuard extends PathTraversalBarrierGuard instanceof AllowListGuard {
-  override predicate checks(Expr e, boolean branch) {
-    e = super.getCheckedExpr() and
-    branch = true and
-    (
-      // Either a path normalization sanitizer comes before the guard,
-      exists(PathNormalizeSanitizer sanitizer | DataFlow::localExprFlow(sanitizer, e))
-      or
-      // or a check like `!path.contains("..")` comes before the guard
-      exists(PathTraversalGuard previousGuard |
-        DataFlow::localExprFlow(previousGuard.getCheckedExpr(), e) and
-        previousGuard.controls(this.getBasicBlock().(ConditionBlock), false)
-      )
+private predicate allowListGuard(Guard g, Expr e, boolean branch) {
+  e = g.(AllowListGuard).getCheckedExpr() and
+  branch = true and
+  (
+    // Either a path normalization sanitizer comes before the guard,
+    exists(PathNormalizeSanitizer sanitizer | DataFlow::localExprFlow(sanitizer, e))
+    or
+    // or a check like `!path.contains("..")` comes before the guard
+    exists(PathTraversalGuard previousGuard |
+      DataFlow::localExprFlow(previousGuard.getCheckedExpr(), e) and
+      previousGuard.controls(g.getBasicBlock().(ConditionBlock), false)
     )
-  }
+  )
+}
+
+private class AllowListSanitizer extends PathTraversalSanitizer {
+  AllowListSanitizer() { this = DataFlow::BarrierGuard<allowListGuard/3>::getABarrierNode() }
 }
 
 /**
- * A guard that considers a path safe because it is checked for `..` components, having previously
+ * Holds if `g` is a guard that considers a path safe because it is checked for `..` components, having previously
  * been checked for a trusted prefix.
  */
-private class DotDotCheckBarrierGuard extends PathTraversalBarrierGuard instanceof PathTraversalGuard {
-  override predicate checks(Expr e, boolean branch) {
-    e = super.getCheckedExpr() and
-    branch = false and
-    // The same value has previously been checked against a list of allowed prefixes:
-    exists(AllowListGuard previousGuard |
-      DataFlow::localExprFlow(previousGuard.getCheckedExpr(), e) and
-      previousGuard.controls(this.getBasicBlock().(ConditionBlock), true)
-    )
-  }
+private predicate dotDotCheckGuard(Guard g, Expr e, boolean branch) {
+  e = g.(PathTraversalGuard).getCheckedExpr() and
+  branch = false and
+  // The same value has previously been checked against a list of allowed prefixes:
+  exists(AllowListGuard previousGuard |
+    DataFlow::localExprFlow(previousGuard.getCheckedExpr(), e) and
+    previousGuard.controls(g.getBasicBlock().(ConditionBlock), true)
+  )
+}
+
+private class DotDotCheckSanitizer extends PathTraversalSanitizer {
+  DotDotCheckSanitizer() { this = DataFlow::BarrierGuard<dotDotCheckGuard/3>::getABarrierNode() }
 }
 
 private class BlockListGuard extends Guard instanceof MethodAccess {
@@ -89,40 +104,44 @@ private class BlockListGuard extends Guard instanceof MethodAccess {
 }
 
 /**
- * A guard that considers a string safe because it is checked against a blocklist of known dangerous values.
+ * Holds if `g` is a guard that considers a string safe because it is checked against a blocklist of known dangerous values.
  * This requires a prior check for URL encoding concealing a forbidden value, either a guard (`UrlEncodingGuard`)
  * or a sanitizer (`UrlDecodeSanitizer`).
  */
-private class BlockListBarrierGuard extends PathTraversalBarrierGuard instanceof BlockListGuard {
-  override predicate checks(Expr e, boolean branch) {
-    e = super.getCheckedExpr() and
-    branch = false and
-    (
-      // Either `e` has been URL decoded:
-      exists(UrlDecodeSanitizer sanitizer | DataFlow::localExprFlow(sanitizer, e))
-      or
-      // or `e` has previously been checked for URL encoding sequences:
-      exists(UrlEncodingGuard previousGuard |
-        DataFlow::localExprFlow(previousGuard.getCheckedExpr(), e) and
-        previousGuard.controls(this.getBasicBlock(), false)
-      )
+private predicate blockListGuard(Guard g, Expr e, boolean branch) {
+  e = g.(BlockListGuard).getCheckedExpr() and
+  branch = false and
+  (
+    // Either `e` has been URL decoded:
+    exists(UrlDecodeSanitizer sanitizer | DataFlow::localExprFlow(sanitizer, e))
+    or
+    // or `e` has previously been checked for URL encoding sequences:
+    exists(UrlEncodingGuard previousGuard |
+      DataFlow::localExprFlow(previousGuard.getCheckedExpr(), e) and
+      previousGuard.controls(g.getBasicBlock(), false)
     )
-  }
+  )
+}
+
+private class BlockListSanitizer extends PathTraversalSanitizer {
+  BlockListSanitizer() { this = DataFlow::BarrierGuard<blockListGuard/3>::getABarrierNode() }
 }
 
 /**
- * A guard that considers a string safe because it is checked for URL encoding sequences,
+ * Holds if `g` is a guard that considers a string safe because it is checked for URL encoding sequences,
  * having previously been checked against a block-list of forbidden values.
  */
-private class UrlEncodingBarrierGuard extends PathTraversalBarrierGuard instanceof UrlEncodingGuard {
-  override predicate checks(Expr e, boolean branch) {
-    e = super.getCheckedExpr() and
-    branch = false and
-    exists(BlockListGuard previousGuard |
-      DataFlow::localExprFlow(previousGuard.getCheckedExpr(), e) and
-      previousGuard.controls(this.getBasicBlock(), false)
-    )
-  }
+private predicate urlEncodingGuard(Guard g, Expr e, boolean branch) {
+  e = g.(UrlEncodingGuard).getCheckedExpr() and
+  branch = false and
+  exists(BlockListGuard previousGuard |
+    DataFlow::localExprFlow(previousGuard.getCheckedExpr(), e) and
+    previousGuard.controls(g.getBasicBlock(), false)
+  )
+}
+
+private class UrlEncodingSanitizer extends PathTraversalSanitizer {
+  UrlEncodingSanitizer() { this = DataFlow::BarrierGuard<urlEncodingGuard/3>::getABarrierNode() }
 }
 
 /**