From a3177368f019bbcb8bd49baae4bc87cf41162fb2 Mon Sep 17 00:00:00 2001
From: Anders Schack-Mulligen <aschackmull@github.com>
Date: Tue, 24 May 2022 16:36:03 +0200
Subject: [PATCH] Java: Add support for BarrierGuards as parameterised modules.

---
 .../java/dataflow/internal/DataFlowUtil.qll   | 21 ++++++++++++++++++
 .../src/Security/CWE/CWE-022/TaintedPath.ql   | 22 ++++++++-----------
 2 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll
index 50abd905698..a387765d3a0 100644
--- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll
+++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll
@@ -304,6 +304,27 @@ class ContentSet instanceof Content {
   }
 }
 
+/**
+ * Holds if `g` validates the `e` upon evaluating to `branch`.
+ *
+ * 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, boolean branch);
+
+module BarrierGuard<guardChecksSig/3 guardChecks> {
+  /** Gets a node that is safely guarded by the given guard. */
+  Node getABarrierNode() {
+    exists(Guard g, SsaVariable v, boolean branch, RValue use |
+      guardChecks(g, v.getAUse(), branch) and
+      use = v.getAUse() and
+      g.controls(use.getBasicBlock(), branch) and
+      result.asExpr() = use
+    )
+  }
+}
+
 /**
  * A guard that validates some expression.
  *
diff --git a/java/ql/src/Security/CWE/CWE-022/TaintedPath.ql b/java/ql/src/Security/CWE/CWE-022/TaintedPath.ql
index adb51f751b4..306b835b98b 100644
--- a/java/ql/src/Security/CWE/CWE-022/TaintedPath.ql
+++ b/java/ql/src/Security/CWE/CWE-022/TaintedPath.ql
@@ -19,15 +19,13 @@ import semmle.code.java.security.PathCreation
 import DataFlow::PathGraph
 import TaintedPathCommon
 
-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
-  }
+predicate containsDotDotSanitizer(Guard g, Expr e, boolean branch) {
+  exists(MethodAccess contains | g = contains |
+    contains.getMethod().hasName("contains") and
+    contains.getAnArgument().(StringLiteral).getValue() = ".." and
+    e = contains.getQualifier() and
+    branch = false
+  )
 }
 
 class TaintedPathConfig extends TaintTracking::Configuration {
@@ -41,10 +39,8 @@ 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
+    or
+    node = DataFlow::BarrierGuard<containsDotDotSanitizer/3>::getABarrierNode()
   }
 }