use a barrier directly instead of a barrier guard

Author: erik-krogh

javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql

@@ -185,6 +185,12 @@ class PropNameTracking extends DataFlow::Configuration {
     )
   }
 
+  override predicate isBarrier(DataFlow::Node node) {
+    super.isBarrier(node)
+    or
+    node instanceof DataFlow::VarAccessBarrier
+  }
+
   override predicate isBarrierGuard(DataFlow::BarrierGuardNode node) {
     node instanceof BlacklistEqualityGuard or
     node instanceof WhitelistEqualityGuard or
@@ -193,8 +199,7 @@ class PropNameTracking extends DataFlow::Configuration {
     node instanceof InstanceOfGuard or
     node instanceof TypeofGuard or
     node instanceof BlacklistInclusionGuard or
-    node instanceof WhitelistInclusionGuard or
-    node instanceof DataFlow::VarAccessBarrierGuard
+    node instanceof WhitelistInclusionGuard
   }
 }
 

javascript/ql/src/semmle/javascript/dataflow/Configuration.qll

@@ -1483,16 +1483,15 @@ private class AdditionalBarrierGuardCall extends AdditionalBarrierGuardNode, Dat
 
 /**
   * A check of the form `if(x)`, which sanitizes `x` in its "else" branch. 
-  * Can be added to a `isBarrierGuard` in a configuration to add the sanitization. 
+  * Can be added to a `isBarrier` in a configuration to add the sanitization.
   */
-class VarAccessBarrierGuard extends BarrierGuardNode, DataFlow::Node {
-  VarAccess var;
-
-  VarAccessBarrierGuard() {
-    var = this.getEnclosingExpr()
-  }
-
-  override predicate blocks(boolean outcome, Expr e) {
-    var = e and outcome = false
+class VarAccessBarrier extends DataFlow::Node {
+  VarAccessBarrier() {
+    exists(ConditionGuardNode guard, SsaRefinementNode refinement |
+      this = DataFlow::ssaDefinitionNode(refinement) and
+      refinement.getGuard() = guard and
+      guard.getTest() instanceof VarAccess and
+      guard.getOutcome() = false
+    )
   }
 }

javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll

@@ -89,7 +89,8 @@ module TaintTracking {
 
     final override predicate isBarrier(DataFlow::Node node) {
       super.isBarrier(node) or
-      isSanitizer(node)
+      isSanitizer(node) or
+      node instanceof DataFlow::VarAccessBarrier
     }
 
     final override predicate isBarrierEdge(DataFlow::Node source, DataFlow::Node sink) {
@@ -914,19 +915,4 @@ module TaintTracking {
     DataFlow::localFlowStep(pred, succ) or
     any(AdditionalTaintStep s).step(pred, succ)
   }
-
-  /** A check of the form `if(x)`, which sanitizes `x` in its "else" branch. */
-  private class VarAccessBarrierGuard extends AdditionalSanitizerGuardNode, DataFlow::Node {
-    DataFlow::VarAccessBarrierGuard guard;
-
-    VarAccessBarrierGuard() {
-      this = guard
-    }
-
-    override predicate sanitizes(boolean outcome, Expr e) {
-      guard.blocks(outcome, e)
-    }
-
-    override predicate appliesTo(Configuration cfg) { any() }
-  }
 }

javascript/ql/src/semmle/javascript/security/dataflow/TaintedPath.qll

@@ -35,8 +35,7 @@ module TaintedPath {
       guard instanceof StartsWithDotDotSanitizer or
       guard instanceof StartsWithDirSanitizer or
       guard instanceof IsAbsoluteSanitizer or
-      guard instanceof ContainsDotDotSanitizer or
-      guard instanceof DataFlow::VarAccessBarrierGuard
+      guard instanceof ContainsDotDotSanitizer
     }
 
     override predicate isAdditionalFlowStep(

javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll

@@ -355,6 +355,11 @@ module TaintedPath {
     }
   }
 
+  /**
+   * A check of the form `if(x)`, which sanitizes `x` in its "else" branch.
+   */
+  class VarAccessBarrier extends Sanitizer, DataFlow::VarAccessBarrier { }
+
   /**
    * A source of remote user input, considered as a flow source for
    * tainted-path vulnerabilities.