make appliesTo recursive

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

@@ -161,7 +161,7 @@ abstract class Configuration extends string {
    */
   predicate isBarrier(DataFlow::Node node) {
     exists(BarrierGuardNode guard |
-      isBarrierGuardInternal(guard) and
+      isBarrierGuardInternal(this, guard) and
       barrierGuardBlocksNode(guard, node, "")
     )
   }
@@ -181,7 +181,7 @@ abstract class Configuration extends string {
    */
   predicate isLabeledBarrier(DataFlow::Node node, FlowLabel lbl) {
     exists(BarrierGuardNode guard |
-      isBarrierGuardInternal(guard) and
+      isBarrierGuardInternal(this, guard) and
       barrierGuardBlocksNode(guard, node, lbl)
     )
     or
@@ -198,17 +198,6 @@ abstract class Configuration extends string {
    */
   predicate isBarrierGuard(BarrierGuardNode guard) { none() }
 
-  /**
-   * Holds if `guard` is a barrier guard for this configuration, added through
-   * `isBarrierGuard` or `AdditionalBarrierGuardNode`.
-   */
-  pragma[nomagic]
-  private predicate isBarrierGuardInternal(BarrierGuardNode guard) {
-    isBarrierGuard(guard)
-    or
-    guard.(AdditionalBarrierGuardNode).appliesTo(this)
-  }
-
   /**
    * Holds if data may flow from `source` to `sink` for this configuration.
    */
@@ -267,6 +256,17 @@ abstract class Configuration extends string {
   }
 }
 
+/**
+ * Holds if `guard` is a barrier guard for this configuration, added through
+ * `isBarrierGuard` or `AdditionalBarrierGuardNode`.
+ */
+pragma[nomagic]
+private predicate isBarrierGuardInternal(Configuration cfg, BarrierGuardNode guard) {
+  cfg.isBarrierGuard(guard)
+  or
+  guard.(AdditionalBarrierGuardNode).appliesTo(cfg)
+}
+
 /**
  * A label describing the kind of information tracked by a flow configuration.
  *
@@ -1981,7 +1981,7 @@ private class BarrierGuardFunction extends Function {
   /**
    * Holds if this function applies to the flow in `cfg`.
    */
-  predicate appliesTo(Configuration cfg) { cfg.isBarrierGuard(guard) }
+  predicate appliesTo(Configuration cfg) { isBarrierGuardInternal(cfg, guard) }
 }
 
 /**
@@ -2034,7 +2034,7 @@ private class CallAgainstEqualityCheck extends AdditionalBarrierGuardNode {
     )
   }
 
-  override predicate appliesTo(Configuration cfg) { cfg.isBarrierGuard(prev) }
+  override predicate appliesTo(Configuration cfg) { isBarrierGuardInternal(cfg, prev) }
 }
 
 /**

javascript/ql/test/query-tests/Security/CWE-078/UnsafeShellCommandConstruction/UnsafeShellCommandConstruction.expected

@@ -311,6 +311,14 @@ nodes
 | lib/lib.js:590:29:590:32 | name |
 | lib/lib.js:593:25:593:28 | name |
 | lib/lib.js:593:25:593:28 | name |
+| lib/lib.js:608:42:608:45 | name |
+| lib/lib.js:608:42:608:45 | name |
+| lib/lib.js:609:22:609:25 | name |
+| lib/lib.js:609:22:609:25 | name |
+| lib/lib.js:626:29:626:32 | name |
+| lib/lib.js:626:29:626:32 | name |
+| lib/lib.js:629:25:629:28 | name |
+| lib/lib.js:629:25:629:28 | name |
 | lib/subLib2/compiled-file.ts:3:26:3:29 | name |
 | lib/subLib2/compiled-file.ts:3:26:3:29 | name |
 | lib/subLib2/compiled-file.ts:4:25:4:28 | name |
@@ -729,6 +737,18 @@ edges
 | lib/lib.js:572:41:572:44 | name | lib/lib.js:593:25:593:28 | name |
 | lib/lib.js:572:41:572:44 | name | lib/lib.js:593:25:593:28 | name |
 | lib/lib.js:572:41:572:44 | name | lib/lib.js:593:25:593:28 | name |
+| lib/lib.js:608:42:608:45 | name | lib/lib.js:609:22:609:25 | name |
+| lib/lib.js:608:42:608:45 | name | lib/lib.js:609:22:609:25 | name |
+| lib/lib.js:608:42:608:45 | name | lib/lib.js:609:22:609:25 | name |
+| lib/lib.js:608:42:608:45 | name | lib/lib.js:609:22:609:25 | name |
+| lib/lib.js:608:42:608:45 | name | lib/lib.js:626:29:626:32 | name |
+| lib/lib.js:608:42:608:45 | name | lib/lib.js:626:29:626:32 | name |
+| lib/lib.js:608:42:608:45 | name | lib/lib.js:626:29:626:32 | name |
+| lib/lib.js:608:42:608:45 | name | lib/lib.js:626:29:626:32 | name |
+| lib/lib.js:608:42:608:45 | name | lib/lib.js:629:25:629:28 | name |
+| lib/lib.js:608:42:608:45 | name | lib/lib.js:629:25:629:28 | name |
+| lib/lib.js:608:42:608:45 | name | lib/lib.js:629:25:629:28 | name |
+| lib/lib.js:608:42:608:45 | name | lib/lib.js:629:25:629:28 | name |
 | lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name |
 | lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name |
 | lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name |
@@ -856,6 +876,9 @@ edges
 | lib/lib.js:579:13:579:28 | "rm -rf " + name | lib/lib.js:572:41:572:44 | name | lib/lib.js:579:25:579:28 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:572:41:572:44 | name | library input | lib/lib.js:579:5:579:29 | cp.exec ... + name) | shell command |
 | lib/lib.js:590:17:590:32 | "rm -rf " + name | lib/lib.js:572:41:572:44 | name | lib/lib.js:590:29:590:32 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:572:41:572:44 | name | library input | lib/lib.js:590:9:590:33 | cp.exec ... + name) | shell command |
 | lib/lib.js:593:13:593:28 | "rm -rf " + name | lib/lib.js:572:41:572:44 | name | lib/lib.js:593:25:593:28 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:572:41:572:44 | name | library input | lib/lib.js:593:5:593:29 | cp.exec ... + name) | shell command |
+| lib/lib.js:609:10:609:25 | "rm -rf " + name | lib/lib.js:608:42:608:45 | name | lib/lib.js:609:22:609:25 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:608:42:608:45 | name | library input | lib/lib.js:609:2:609:26 | cp.exec ... + name) | shell command |
+| lib/lib.js:626:17:626:32 | "rm -rf " + name | lib/lib.js:608:42:608:45 | name | lib/lib.js:626:29:626:32 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:608:42:608:45 | name | library input | lib/lib.js:626:9:626:33 | cp.exec ... + name) | shell command |
+| lib/lib.js:629:13:629:28 | "rm -rf " + name | lib/lib.js:608:42:608:45 | name | lib/lib.js:629:25:629:28 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:608:42:608:45 | name | library input | lib/lib.js:629:5:629:29 | cp.exec ... + name) | shell command |
 | lib/subLib2/compiled-file.ts:4:13:4:28 | "rm -rf " + name | lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name | This string concatenation which depends on $@ is later used in a $@. | lib/subLib2/compiled-file.ts:3:26:3:29 | name | library input | lib/subLib2/compiled-file.ts:4:5:4:29 | cp.exec ... + name) | shell command |
 | lib/subLib2/special-file.js:4:10:4:25 | "rm -rf " + name | lib/subLib2/special-file.js:3:28:3:31 | name | lib/subLib2/special-file.js:4:22:4:25 | name | This string concatenation which depends on $@ is later used in a $@. | lib/subLib2/special-file.js:3:28:3:31 | name | library input | lib/subLib2/special-file.js:4:2:4:26 | cp.exec ... + name) | shell command |
 | lib/subLib3/my-file.ts:4:10:4:25 | "rm -rf " + name | lib/subLib3/my-file.ts:3:28:3:31 | name | lib/subLib3/my-file.ts:4:22:4:25 | name | This string concatenation which depends on $@ is later used in a $@. | lib/subLib3/my-file.ts:3:28:3:31 | name | library input | lib/subLib3/my-file.ts:4:2:4:26 | cp.exec ... + name) | shell command |

javascript/ql/test/query-tests/Security/CWE-078/UnsafeShellCommandConstruction/lib/lib.js

@@ -592,3 +592,39 @@ module.exports.safeWithBool = function (name) {
 
     cp.exec("rm -rf " + name); // NOT OK
 }
+
+function indirectThing(name) {
+    return isSafeName(name);
+}
+
+function indirectThing2(name) {
+    return isSafeName(name) === true;
+}
+
+function moreIndirect(name) {
+    return indirectThing2(name) !== false;
+}
+
+module.exports.veryIndeirect = function (name) {
+	cp.exec("rm -rf " + name); // NOT OK
+
+	if (indirectThing(name)) {
+        cp.exec("rm -rf " + name); // OK
+    }
+
+    if (indirectThing2(name)) {
+        cp.exec("rm -rf " + name); // OK
+    }
+
+    if (moreIndirect(name)) {
+        cp.exec("rm -rf " + name); // OK
+    }
+
+    if (moreIndirect(name) !== false) {
+        cp.exec("rm -rf " + name); // OK
+    } else {
+        cp.exec("rm -rf " + name); // NOT OK
+    }
+
+    cp.exec("rm -rf " + name); // NOT OK
+}