Java: Add pruning for local taint flow.

Author: aschackmull

java/ql/lib/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll

@@ -33,6 +33,57 @@ predicate localExprTaint(Expr src, Expr sink) {
   localTaint(DataFlow::exprNode(src), DataFlow::exprNode(sink))
 }
 
+/** Holds if `node` is an endpoint for local taint flow. */
+signature predicate nodeSig(DataFlow::Node node);
+
+/** Provides local taint flow restricted to a given set of sources and sinks. */
+module LocalTaintFlow<nodeSig/1 source, nodeSig/1 sink> {
+  private predicate reachRev(DataFlow::Node n) {
+    sink(n)
+    or
+    exists(DataFlow::Node mid |
+      localTaintStep(n, mid) and
+      reachRev(mid)
+    )
+  }
+
+  private predicate reachFwd(DataFlow::Node n) {
+    reachRev(n) and
+    (
+      source(n)
+      or
+      exists(DataFlow::Node mid |
+        localTaintStep(mid, n) and
+        reachFwd(mid)
+      )
+    )
+  }
+
+  private predicate step(DataFlow::Node n1, DataFlow::Node n2) {
+    localTaintStep(n1, n2) and
+    reachFwd(n1) and
+    reachFwd(n2)
+  }
+
+  /**
+   * Holds if taint can flow from `n1` to `n2` in zero or more local
+   * (intra-procedural) steps that are restricted to be part of a path between
+   * `source` and `sink`.
+   */
+  pragma[inline]
+  predicate hasFlow(DataFlow::Node n1, DataFlow::Node n2) { step*(n1, n2) }
+
+  /**
+   * Holds if taint can flow from `n1` to `n2` in zero or more local
+   * (intra-procedural) steps that are restricted to be part of a path between
+   * `source` and `sink`.
+   */
+  pragma[inline]
+  predicate hasExprFlow(Expr n1, Expr n2) {
+    hasFlow(DataFlow::exprNode(n1), DataFlow::exprNode(n2))
+  }
+}
+
 cached
 private module Cached {
   private import DataFlowImplCommon as DataFlowImplCommon

java/ql/lib/semmle/code/java/security/PathSanitizer.qll

@@ -69,13 +69,25 @@ private class ExactPathMatchSanitizer extends PathInjectionSanitizer {
   }
 }
 
-private class AllowedPrefixGuard extends Guard instanceof MethodAccess {
+abstract private class PathGuard extends Guard {
+  abstract Expr getCheckedExpr();
+}
+
+private predicate anyNode(DataFlow::Node n) { any() }
+
+private predicate pathGuardNode(DataFlow::Node n) { n.asExpr() = any(PathGuard g).getCheckedExpr() }
+
+private predicate localTaintFlowToPathGuard(Expr e, PathGuard g) {
+  TaintTracking::LocalTaintFlow<anyNode/1, pathGuardNode/1>::hasExprFlow(e, g.getCheckedExpr())
+}
+
+private class AllowedPrefixGuard extends PathGuard instanceof MethodAccess {
   AllowedPrefixGuard() {
     (isStringPrefixMatch(this) or isPathPrefixMatch(this)) and
     not isDisallowedWord(super.getAnArgument())
   }
 
-  Expr getCheckedExpr() { result = super.getQualifier() }
+  override Expr getCheckedExpr() { result = super.getQualifier() }
 }
 
 /**
@@ -91,10 +103,10 @@ private predicate allowedPrefixGuard(Guard g, Expr e, boolean branch) {
   //  String strPath = file.getCanonicalPath();
   //  if (strPath.startsWith("/safe/dir"))
   //    sink(file);
-  TaintTracking::localExprTaint(e, g.(AllowedPrefixGuard).getCheckedExpr()) and
+  g instanceof AllowedPrefixGuard and
+  localTaintFlowToPathGuard(e, g) and
   exists(Expr previousGuard |
-    TaintTracking::localExprTaint(previousGuard.(PathNormalizeSanitizer),
-      g.(AllowedPrefixGuard).getCheckedExpr())
+    localTaintFlowToPathGuard(previousGuard.(PathNormalizeSanitizer), g)
     or
     previousGuard
         .(PathTraversalGuard)
@@ -121,7 +133,7 @@ private predicate dotDotCheckGuard(Guard g, Expr e, boolean branch) {
   //  if (!strPath.contains("..") && strPath.startsWith("/safe/dir"))
   //    sink(path);
   branch = g.(PathTraversalGuard).getBranch() and
-  TaintTracking::localExprTaint(e, g.(PathTraversalGuard).getCheckedExpr()) and
+  localTaintFlowToPathGuard(e, g) and
   exists(Guard previousGuard |
     previousGuard.(AllowedPrefixGuard).controls(g.getBasicBlock(), true)
     or
@@ -136,13 +148,13 @@ private class DotDotCheckSanitizer extends PathInjectionSanitizer {
   }
 }
 
-private class BlockListGuard extends Guard instanceof MethodAccess {
+private class BlockListGuard extends PathGuard instanceof MethodAccess {
   BlockListGuard() {
     (isStringPartialMatch(this) or isPathPrefixMatch(this)) and
     isDisallowedWord(super.getAnArgument())
   }
 
-  Expr getCheckedExpr() { result = super.getQualifier() }
+  override Expr getCheckedExpr() { result = super.getQualifier() }
 }
 
 /**
@@ -158,10 +170,10 @@ private predicate blockListGuard(Guard g, Expr e, boolean branch) {
   //  String strPath = file.getCanonicalPath();
   //  if (!strPath.contains("..") && !strPath.startsWith("/dangerous/dir"))
   //    sink(file);
-  TaintTracking::localExprTaint(e, g.(BlockListGuard).getCheckedExpr()) and
+  g instanceof BlockListGuard and
+  localTaintFlowToPathGuard(e, g) and
   exists(Expr previousGuard |
-    TaintTracking::localExprTaint(previousGuard.(PathNormalizeSanitizer),
-      g.(BlockListGuard).getCheckedExpr())
+    localTaintFlowToPathGuard(previousGuard.(PathNormalizeSanitizer), g)
     or
     previousGuard
         .(PathTraversalGuard)
@@ -217,7 +229,7 @@ private predicate isDisallowedWord(CompileTimeConstantExpr word) {
 }
 
 /** A complementary guard that protects against path traversal, by looking for the literal `..`. */
-private class PathTraversalGuard extends Guard {
+private class PathTraversalGuard extends PathGuard {
   PathTraversalGuard() {
     exists(MethodAccess ma |
       ma.getMethod().getDeclaringType() instanceof TypeString and
@@ -235,7 +247,7 @@ private class PathTraversalGuard extends Guard {
     )
   }
 
-  Expr getCheckedExpr() {
+  override Expr getCheckedExpr() {
     exists(MethodAccess ma | ma = this.(EqualityTest).getAnOperand() or ma = this |
       result = ma.getQualifier()
     )