From ade93e66e1b5a650cdb49527c33a29dbcdfa2a60 Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Thu, 6 Feb 2020 15:44:22 +0100
Subject: [PATCH] move the if(!x) from DataFLow to TaintTracking

---
 .../Security/CWE-400/PrototypePollutionUtility.ql |  3 ++-
 .../semmle/javascript/dataflow/Configuration.qll  |  9 +++++----
 .../semmle/javascript/dataflow/TaintTracking.qll  | 15 +++++++++++++++
 .../javascript/security/dataflow/TaintedPath.qll  |  3 ++-
 4 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql b/javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql
index f27036c9369..6923b78d9d8 100644
--- a/javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql
+++ b/javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql
@@ -356,7 +356,8 @@ class PropNameTracking extends DataFlow::Configuration {
     node instanceof InstanceOfGuard or
     node instanceof TypeofGuard or
     node instanceof BlacklistInclusionGuard or
-    node instanceof WhitelistInclusionGuard
+    node instanceof WhitelistInclusionGuard or
+    node instanceof DataFlow::VarAccessBarrierGuard
   }
 }
 
diff --git a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
index b35285d936d..6f9b78780f7 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
@@ -1481,8 +1481,11 @@ private class AdditionalBarrierGuardCall extends AdditionalBarrierGuardNode, Dat
   override predicate appliesTo(Configuration cfg) { f.appliesTo(cfg) }
 }
 
-/** A check of the `if(x)`, which sanitizes `x` in its "else" branch. */
-private class VarAccessBarrierGuard extends AdditionalBarrierGuardNode, DataFlow::Node {
+/**
+  * A check of the `if(x)`, which sanitizes `x` in its "else" branch. 
+  * Can be added to a `isBarrierGuard` in a configuration to add the sanitization. 
+  */
+class VarAccessBarrierGuard extends BarrierGuardNode, DataFlow::Node {
   VarAccess var;
 
   VarAccessBarrierGuard() {
@@ -1492,6 +1495,4 @@ private class VarAccessBarrierGuard extends AdditionalBarrierGuardNode, DataFlow
   override predicate blocks(boolean outcome, Expr e) {
     var = e and outcome = false
   }
-
-  override predicate appliesTo(Configuration cfg) { any() }
 }
\ No newline at end of file
diff --git a/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll b/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll
index be1df8bc7c2..fa573cf908a 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll
@@ -914,4 +914,19 @@ 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() }
+  }
 }
diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPath.qll b/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPath.qll
index b46f7d508f7..5e888b6e768 100644
--- a/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPath.qll
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPath.qll
@@ -35,7 +35,8 @@ module TaintedPath {
       guard instanceof StartsWithDotDotSanitizer or
       guard instanceof StartsWithDirSanitizer or
       guard instanceof IsAbsoluteSanitizer or
-      guard instanceof ContainsDotDotSanitizer
+      guard instanceof ContainsDotDotSanitizer or
+      guard instanceof DataFlow::VarAccessBarrierGuard
     }
 
     override predicate isAdditionalFlowStep(